msg225305 - (view) |
Author: Mark Dickinson (mark.dickinson) *  |
Date: 2014-08-14 16:47 |
I'm not sure it's worth fixing this, but it seems worth recording:
>>> -0.5 // float('inf')
-1.0
I was expecting a value of `-0.0`, and while IEEE 754 doesn't cover the floor division operation, I'm reasonably confident that that's the value it would have recommended if it had. :-)
However, it's difficult to come up with a situation where the difference matters: there aren't any obvious invariants I can think of that are broken by this special case. So unless anyone thinks it should be changed, I'll settle for recording the oddity in this issue, and closing as won't fix after a short period.
|
msg225309 - (view) |
Author: Steven D'Aprano (steven.daprano) *  |
Date: 2014-08-14 19:19 |
On Thu, Aug 14, 2014 at 04:47:41PM +0000, Mark Dickinson wrote:
> I'm not sure it's worth fixing this, but it seems worth recording:
>
> >>> -0.5 // float('inf')
> -1.0
>
> I was expecting a value of `-0.0`, and while IEEE 754 doesn't cover
> the floor division operation, I'm reasonably confident that that's the
> value it would have recommended if it had. :-)
Hmmm. I'm not so sure. -0.5 // something_really_big gives -1:
py> -0.5//1e200
-1.0
Consider something_really_big as it gets bigger and bigger and
approaches infinity, if we *informally* take the limit -> inf I think it
makes sense for it to return -1. Another way of looking at it is that
-0.5/inf returns a negative infinitesimal quantity, and then taking the
floor returns -1. So I think the current behaviour is "correct", for
some definition of correct.
The alternative is a discontinuity, where -0.5//x = -1 for all finite
but huge x and then suddenly 0 when x overflows to infinity. That's
probably a bad idea.
|
msg225313 - (view) |
Author: Tim Peters (tim.peters) *  |
Date: 2014-08-14 19:40 |
I'm OK with -1, but I don't get that or -0.0 on 32-bit Windows Py 3.4.1:
Python 3.4.1 (v3.4.1:c0e311e010fc, May 18 2014, 10:38:22) [MSC v.1600 32 bit (Intel)] on win32
Type "copyright", "credits" or "license()" for more information.
>>> -0.5 // float('inf')
nan
So maybe NaN is the best answer ;-)
In favor of -1.0: that _is_ the limit of the mathematical floor(-0.5 / x) as x approaches +infinity.
In favor of -0.0: it "should be" mathematically that floor_division(x/y) = floor(x / y), and floor(-0.5 / inf) = floor(-0.0) = ... well, not -0.0! floor() in Py3 is defined to return an integer, and there is no -0 integer:
>>> floor(-0.0)
0
That's +0. So I see no justification at all for -0.0 in Py3. -1 seems the best that can be done. The NaN I actually get doesn't make sense.
|
msg225314 - (view) |
Author: Mark Dickinson (mark.dickinson) *  |
Date: 2014-08-14 19:42 |
Steven: there's a set of (unwritten) rules for how the IEEE 754 operations work. (I think they actually *were* articulated explicitly in some of the 754r drafts, but didn't make it into the final version.) One of them is that ideally, a floating-point operations works as though the corresponding mathematical operation were performed exactly on the inputs (considered as real numbers), followed by a rounding step that takes the resulting real number and rounds it to the nearest floating-point number. This is how essentially *all* the operations prescribed in IEEE 754 behave, with a greater or lesser amount of hand-waving when it comes to specifying results for special cases like infinities and nans. In this case, the underlying mathematical operation is `x, y -> floor(x / y)`. The only tricky point is the extension to infinity, but we've got the existing behaviour of regular division to guide us there - the result of dividing a finite value by an infinity is an appropriately signed zero. So there's really not a lot of room for manoeuvre in an IEEE 754-like operation.
> The alternative is a discontinuity, where -0.5//x = -1 for all finite
> but huge x and then suddenly 0 when x overflows to infinity. That's
> probably a bad idea.
Shrug: the underlying mathematical operation is discontinuous; I really don't see a problem here. In any case, if you're worried about discontinuities, what about the one that occurs between positive values and negative values of x in the current implementation (a jump from 0 to -1)? Continuity takes second place to correctness here.
|
msg225315 - (view) |
Author: Mark Dickinson (mark.dickinson) *  |
Date: 2014-08-14 19:48 |
[Tim]
>>> -0.5 // float('inf')
nan
Urk! I wonder what's going on there. I think I like that answer even less than -1.0.
IEEE 754's floor does indeed take -0.0 to -0.0.
|
msg225340 - (view) |
Author: Raymond Hettinger (rhettinger) *  |
Date: 2014-08-15 05:27 |
> ideally, a floating-point operations works as though the
> corresponding mathematical operation were performed exactly
>on the inputs (considered as real numbers), followed by a rounding
> step that takes the resulting real number and rounds it to the
> nearest floating-point number.
FWIW, the Decimal Arithmetic Specification was created around the same principle. Accordingly, it gets the answer that Mark expected:
>>> from decimal import Decimal
>>> Decimal('-0.5') // Decimal('Inf')
Decimal('-0')
|
msg225359 - (view) |
Author: Stefan Krah (skrah) *  |
Date: 2014-08-15 18:48 |
I think the intention of the standard is pretty much as Mark
said in msg225314. The fact that decimal behaves that way is
another indicator, since Cowlishaw really tried to mirror the
2008 standard as closely as possible.
|
msg225360 - (view) |
Author: Tim Peters (tim.peters) *  |
Date: 2014-08-15 18:59 |
To be clear, I agree -0.0 is "the correct" answer, and -1.0 is at best defensible via a mostly-inappropriate limit argument. But in Py3 floor division of floats returns an integer, and there is no integer -0. Nor, God willing, will there ever be ;-)
Looks to me like what (Py3's, at least) floatobject.c's floor_divmod() returns (the source of float floor division's result) when the 2nd argument is infinite is largely an accident, depending on what the platform C fmod() and floor() happen to return. So it would require special-casing an infinite denominator in that function to force any specific cross-platform result.
|
msg225365 - (view) |
Author: Eryk Sun (eryksun) *  |
Date: 2014-08-15 21:20 |
decimal.Decimal 'floor division' is integer division that truncates toward 0 (see 9.4.2).
>>> Decimal('-0.5').__floor__()
-1
>>> Decimal('-0.5').__floordiv__(1)
Decimal('-0')
Numpy 1.8.1:
>>> np.float32(-0.5) // 1
-1.0
>>> np.float32(-0.5) // float('inf')
-0.0
>>> np.array([-0.5]) // 1
array([-1.])
>>> np.array([-0.5]) // float('inf')
array([-0.])
|
msg225386 - (view) |
Author: Mark Dickinson (mark.dickinson) *  |
Date: 2014-08-16 08:16 |
> But in Py3 floor division of floats returns an integer.
Not in my version!
Python 3.4.1 (default, May 21 2014, 01:39:38)
[GCC 4.2.1 Compatible Apple LLVM 5.1 (clang-503.0.40)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> -3.0 // 5.0
-1.0
Maybe I'm using the wrong time machine.
|
msg225401 - (view) |
Author: Tim Peters (tim.peters) *  |
Date: 2014-08-16 16:02 |
Sorry, Mark - I took a true thing and careleslly turned it into a false thing ;-)
It's math.floor(a_float) that returns an int in Py3, not floor division of floats. So, yup, no real problem with returning -0.0 after all; it's just that it can't be _explained_ via
x // y means math.floor(x / y)
is Py3 for float x and y, since the latter returns an int bur the former a float.
But looks like it can be "explained" via
x // y means divmod(x, y)[0]
|
msg226541 - (view) |
Author: Petr Viktorin (petr.viktorin) *  |
Date: 2014-09-07 19:06 |
I tried my hand at writing a patch. I hope it is helpful.
The message of the 2001 commit that introduces this says that "there's no platform-independent way to write a test case for this". I assume with @support.requires_IEEE_754 that is no longer true (at least for non-exotic platforms), or was there another issue?
I noticed there is no test suite for float floordiv, so I attempted writing a fuller one, but when I saw that
>>> float('inf') // 1.0
nan
I decided to keep my first CPython patch small and focused, so I can learn the ropes. I'll file more issues later.
|
msg226542 - (view) |
Author: Petr Viktorin (petr.viktorin) *  |
Date: 2014-09-07 19:11 |
Note: I signed the contributor agreement form recently, I should have a * soon.
|
msg227307 - (view) |
Author: Alexander Belopolsky (belopolsky) *  |
Date: 2014-09-22 20:44 |
I wonder if it would make sense to rewrite float_divmod using the newer POSIX/C99 remquo function. I believe it is designed to compute the exact value of round(x/y), but getting floor instead should not be hard. Its behavior on special values is fully specified.
From the Linux man-page (I believe POSIX/C99 only guarantees 3 bits in quo):
NAME
remquo -- floating-point remainder and quotient function
SYNOPSIS
#include <math.h>
double
remquo(double x, double y, int *quo);
long double
remquol(long double x, long double y, int *quo);
float
remquof(float x, float y, int *quo);
DESCRIPTION
The remquo() functions compute the value r such that r = x - n*y, where n is
the integer nearest the exact value of x/y.
If there are two integers closest to x/y, n shall be the even one. If r is
zero, it is given the same sign as x. This is the same value that is
returned by the remainder() function. remquo() also calculates the lower
seven bits of the integral quotient x/y, and gives that value the same sign
as x/y. It stores this signed value in the object pointed to by quo.
SPECIAL VALUES
remquo(x, y, quo) returns a NaN and raises the "invalid" floating-point
exception if x is infinite or y is 0.
|
msg228865 - (view) |
Author: Petr Viktorin (petr.viktorin) *  |
Date: 2014-10-09 12:24 |
Apologies for the delay; I missed/did not get a notification.
Alexander, I don't disagree, but I'd like my first patch to Python to not be a refactoring. As I said, I'd like to keep this patch focused. After that I'd like to provide tests the rest of float_divmod; and then perhaps use an entirely different implementation.
If that's not a good course of action, and you suggest a different one or just tell me to improve everything at once, I will certainly try. But, I think that this patch is an improvement, and that it does fix this bug.
|
msg230958 - (view) |
Author: Petr Viktorin (petr.viktorin) *  |
Date: 2014-11-10 12:35 |
ping, could someone please review the patch?
|
msg234068 - (view) |
Author: Petr Viktorin (petr.viktorin) *  |
Date: 2015-01-15 08:56 |
ping, is there anything I can do to help push the patch forward?
|
msg234069 - (view) |
Author: Mark Dickinson (mark.dickinson) *  |
Date: 2015-01-15 09:18 |
The patch is fine; I just need to find time to look at it properly. That might take a week or two. Sorry for the delay.
|
msg241088 - (view) |
Author: Petr Viktorin (petr.viktorin) *  |
Date: 2015-04-15 08:18 |
ping?
|
msg241342 - (view) |
Author: Mark Dickinson (mark.dickinson) *  |
Date: 2015-04-17 16:15 |
Thanks for the ping, and sorry for forgetting about this.
I'm -1 on applying this patch. I agree that floor division has some corner case issues (of which this is only one). But there's no clear agreement on what the right answer is, and I don't think making a tiny change to one corner case is worth it in terms of code churn. And making several such tiny changes over the course of different Python releases is something we'd definitely want to avoid.
Ideally, there'd be a once-and-for-all agreement on exactly what should happen with *all* the corner cases; we'd fix the code to implement exactly that, and then we could forget about it. But without a standard to guide us, I don't think that's going to happen.
So my vote is to close as "wont fix".
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:06 | admin | set | github: 66394 |
2016-07-07 15:31:52 | serhiy.storchaka | set | status: open -> closed resolution: wont fix stage: resolved |
2015-04-17 16:15:51 | mark.dickinson | set | assignee: mark.dickinson -> |
2015-04-17 16:15:42 | mark.dickinson | set | messages:
+ msg241342 |
2015-04-15 08:18:01 | petr.viktorin | set | messages:
+ msg241088 |
2015-01-15 09:18:52 | mark.dickinson | set | messages:
+ msg234069 |
2015-01-15 08:56:12 | petr.viktorin | set | messages:
+ msg234068 |
2014-11-10 12:35:29 | petr.viktorin | set | messages:
+ msg230958 |
2014-10-09 12:24:55 | petr.viktorin | set | messages:
+ msg228865 |
2014-09-23 01:58:27 | casevh | set | nosy:
+ casevh
|
2014-09-22 20:52:40 | Arfrever | set | nosy:
+ Arfrever
|
2014-09-22 20:44:13 | belopolsky | set | messages:
+ msg227307 |
2014-09-22 20:27:03 | belopolsky | set | nosy:
+ belopolsky
|
2014-09-07 19:11:56 | petr.viktorin | set | messages:
+ msg226542 |
2014-09-07 19:06:27 | petr.viktorin | set | files:
+ issue22198.patch
nosy:
+ petr.viktorin messages:
+ msg226541
keywords:
+ patch |
2014-08-16 16:02:44 | tim.peters | set | messages:
+ msg225401 |
2014-08-16 08:16:51 | mark.dickinson | set | messages:
+ msg225386 |
2014-08-15 21:20:28 | eryksun | set | nosy:
+ eryksun messages:
+ msg225365
|
2014-08-15 18:59:08 | tim.peters | set | messages:
+ msg225360 |
2014-08-15 18:48:58 | skrah | set | messages:
+ msg225359 |
2014-08-15 05:27:16 | rhettinger | set | nosy:
+ rhettinger messages:
+ msg225340
|
2014-08-14 19:48:44 | mark.dickinson | set | messages:
+ msg225315 |
2014-08-14 19:42:08 | mark.dickinson | set | messages:
+ msg225314 |
2014-08-14 19:40:49 | tim.peters | set | messages:
+ msg225313 |
2014-08-14 19:19:14 | steven.daprano | set | nosy:
+ steven.daprano messages:
+ msg225309
|
2014-08-14 16:49:48 | mark.dickinson | set | nosy:
+ tim.peters
|
2014-08-14 16:48:21 | mark.dickinson | set | nosy:
+ skrah
|
2014-08-14 16:47:41 | mark.dickinson | create | |