-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ranges terribly sensitive to floating point #2333
Comments
We are using the formula |
Ah yes: julia> .1 + 2*.1
0.30000000000000004 So our current formula is consistent with how we compute the elements. I suppose we'd have to change both. I don't suppose it would be ok to adjust the endpoint to |
More specifically, the issue is here: julia> a = (.3-.1)/.1
1.9999999999999998 Adding eps(x) works, but I'm not sure it's justified:
|
In this particular case, it turns out you can get the exact values you want (0.1, 0.2, 0.3) by adjusting the step size to |
I'm not sure that you can reasonably expect to avoid this kind of off-by-one errors for floating point ranges. The definition is intrinsically sensitive to roundoff errors. |
How about |
...or maybe you want to avoid the sqrt for performance reasons, in which case a better choice might be nf = (stop-start)/step + 1
nf += abs(nf)*eps(T)
n = ifloor(nf) |
I'll bet most of you didn't know you could almost write a book on colon alone. :-) There are two quesions for start:delta:stop
Regarding 2 a few comments Strategy a) [start+i_delta for i=0:n] <--- terrible strategy Regarding 1 You'll have to decide whether to preserve the user's stop without roundng,
and
I would declare stop as official and not to be modified. Otherwise, I would |
I think that dismissing |
Excellent, thank you. nf = (stop-start)/step + 1
n = round(nf)
if abs(n-nf) < eps(n)*3
step = (stop-start)/(n-1)
end |
In the case where you modify Also, shouldn't the rule use |
Ok, I pushed a new implementation that is probably no worse than the old one. I think the best thing to do now is find more test cases, and if the behavior for any of them seems bad enough I can try further improvements. |
Should be something like:
I think. Your current patch gives
|
Current patch still doesn't necessarily preserve the endpoint specified by the user:
Note that there is no |
I don't think it should be round, because you can get beyond the end point in On Sunday, February 17, 2013 12:35:42 PM Jeff Bezanson wrote:
|
Oh shoot, I should have checked for updates, @stevengj made the same point already. |
I do think all this
|
The step adjustment does help in cases where a good step value exists, though. When no good step value exists we're no worse than before. |
The end point is going to be I'm pretty sure the only role for the adjustment is to save you from mistakes due to the usage of |
If |
You're talking about
which happens to work for the original example:
However, even this doesn't solve @stevengj's second example:
So adjusting |
I understand that as long as we use |
This is crazy. We should not be changing the step that is given because we feel like it. The problem with the original example is that 1/10 cannot be represented exactly, but it is one of the real values that the binary number |
No, this is based on the idea that the end value should be taken more seriously than the step value. In general, the step value is not observed directly, but the start and end values are. We're not changing the step because we feel like it, but to try to hit the endpoint exactly:
I would argue that's better than what we did before. |
Regardless of the other issues, I would argue that 3 lines are better than 13. |
@timholy no argument there, certainly. Unfortunately we still need the special case for I think we need some more examples. I wouldn't want to change the step unconditionally, since in cases like |
Guido van Rossum asked for an algorithm for this a couple of years ago. It's telling that it was intended as a trick question, and he considers |
We should probably make a bigger deal about the fact that you can avoid these issues using Python seems to be making its life a bit harder by excluding the end point from ranges. If the last returned value is supposed to be |
You're not really late as we haven't implemented anything in response to this. I've been considering how best to do it. My current thought is to have a FloatRange type that stores function ref(r::FloatRange, k::Int)
k -= 1; (0 <= k <= r.n) || error(BoundsError)
((r.n-k)*r.A + k*r.B)/r.d
end However, four numbers is a lot of state to keep track of here, so I'm not entirely sure. That also leaves unclear how to handle the |
Possibly-related:
as reported on julia-users. |
Corner case. The lines if abs(n-nf) < eps(n)*3
# adjust step to try to hit stop exactly
step = (stop-start)/(n-1)
len = itrunc(n) make no sense when n = 1. Fixed by pull request #3961. |
Just to leave a record here, these are some good tricky test cases for frange(0.1, 0.3, 0.1)
frange(0.0, 0.3, 0.1)
frange(0.3, -0.1, -0.1)
frange(0.1, -0.3, -0.1)
frange(1.0, 27.0, 1/49)[end]
frange(0.0, 2.1, 0.7)
frange(0.0, 5.5, 1.0)
frange(prevfloat(0.1), 0.3, 0.1)
frange(nextfloat(0.1), 0.3, 0.1)
frange(prevfloat(0.0), 0.3, 0.1)
frange(nextfloat(0.0), 0.3, 0.1)
frange(0.1, prevfloat(0.3), 0.1)
frange(0.1, nextfloat(0.3), 0.1)
frange(0.0, prevfloat(0.3), 0.1)
frange(0.0, nextfloat(0.3), 0.1)
frange(0.1, 0.3, prevfloat(0.1))
frange(0.1, 0.3, nextfloat(0.1))
frange(0.0, 0.3, nextfloat(0.1))
frange(0.0, 0.3, prevfloat(0.1)) |
This is a fairly remarkable function frange(a,b,s)
@show m = 1/s
@show A = m * a
@show n = floor(m*b - A)
@show A = (A + n) - n
@show m = a + s == 0 ? A/a : (A+1)/(a+s)
@show A = m * a
[ (A + k)/m for k = 0:n ]
end
@assert frange(0.1, 0.3, 0.1) == [1:3]./10
@assert frange(0.0, 0.3, 0.1) == [0:3]./10
@assert frange(0.3, -0.1, -0.1) == [3:-1:-1]./10
@assert frange(0.1, -0.3, -0.1) == [1:-1:-3]./10
@assert frange(0.0, 1.0, 0.1) == [0:10]./10
@assert frange(0.0, 1.0, -0.1) == []
@assert frange(0.0, -1.0, 0.1) == []
@assert frange(0.0, -1.0, -0.1) == [0:-1:-10]./10
@assert frange(1.0, 27.0, 1/49) == [49:1323]./49
@assert frange(0.0, 2.1, 0.7) == [0:7:21]./10
@assert frange(0.0, 5.5, 1.0) == [0:10:55]./10
@assert frange(0.0, 0.5, -1.0) == []
@assert frange(0.0, 0.5, 1.0) == [0.0]
@assert frange(prevfloat(0.1), 0.3, 0.1) == [prevfloat(0.1), 0.2, 0.3]
@assert frange(nextfloat(0.1), 0.3, 0.1) == [nextfloat(0.1), 0.2]
@assert frange(prevfloat(0.0), 0.3, 0.1) == [prevfloat(0.0), 0.1, 0.2, 0.3]
@assert frange(nextfloat(0.0), 0.3, 0.1) == [nextfloat(0.0), 0.1, 0.2, 0.3]
@assert frange(0.1, prevfloat(0.3), 0.1) == [0.1, 0.2]
@assert frange(0.1, nextfloat(0.3), 0.1) == [0.1, 0.2, 0.3]
@assert frange(0.0, prevfloat(0.3), 0.1) == [0.0, 0.1, 0.2]
@assert frange(0.0, nextfloat(0.3), 0.1) == [0.0, 0.1, 0.2, 0.3]
@assert frange(0.1, 0.3, prevfloat(0.1)) == [0.1, 0.2, 0.3]
@assert frange(0.1, 0.3, nextfloat(0.1)) == [0.1, 0.2]
@assert frange(0.0, 0.3, prevfloat(0.1)) == [0.0, 0.1, 0.2, 0.3]
@assert frange(0.0, 0.3, nextfloat(0.1)) == [0.0, nextfloat(0.1), nextfloat(0.2)] It still has some issues with non-integral |
Ok, this version passes every test I can throw at it: function rat(s)
b, d, y = zero(s), one(s), abs(s)
while true
f = trunc(y); y -= f
d, b = b, f*b + d
a = round(b*s)
(y == 0 || a/b == s) && return a, b
y = inv(y)
end
end
function frange(a,b,s)
x = (b-a)/s
A, n, d, N, r = a, s, one(s), floor(x), round(x)
if prevfloat(r) <= x <= nextfloat(r)
T, D = rat(s)
if (D*b - D*a)/r/D == s
A, n, d, N = D*a, T, D, r
end
end
[ (A+k*n)/d for k = 0:N ]
end
@assert frange(0.1, 0.3, 0.1) == [1:3]./10
@assert frange(0.0, 0.3, 0.1) == [0:3]./10
@assert frange(0.3, -0.1, -0.1) == [3:-1:-1]./10
@assert frange(0.1, -0.3, -0.1) == [1:-1:-3]./10
@assert frange(0.0, 1.0, 0.1) == [0:10]./10
@assert frange(0.0, 1.0, -0.1) == []
@assert frange(0.0, -1.0, 0.1) == []
@assert frange(0.0, -1.0, -0.1) == [0:-1:-10]./10
@assert frange(1.0, 27.0, 1/49) == [49:1323]./49
@assert frange(0.0, 2.1, 0.7) == [0:7:21]./10
@assert frange(0.0, 3.3, 1.1) == [0:11:33]./10
@assert frange(0.1, 3.4, 1.1) == [1:11:34]./10
@assert frange(0.0, 3.9, 1.3) == [0:13:39]./10
@assert frange(0.1, 4.0, 1.3) == [1:13:40]./10
@assert frange(0.0, 5.5, 1.0) == [0:10:55]./10
@assert frange(0.0, 0.5, -1.0) == []
@assert frange(0.0, 0.5, 1.0) == [0.0]
@assert frange(prevfloat(0.1), 0.3, 0.1) == [prevfloat(0.1), 0.2, 0.3]
@assert frange(nextfloat(0.1), 0.3, 0.1) == [nextfloat(0.1), 0.2]
@assert frange(prevfloat(0.0), 0.3, 0.1) == [prevfloat(0.0), 0.1, 0.2, 0.3]
@assert frange(nextfloat(0.0), 0.3, 0.1) == [nextfloat(0.0), 0.1, 0.2, 0.3]
@assert frange(0.1, prevfloat(0.3), 0.1) == [0.1, 0.2]
@assert frange(0.1, nextfloat(0.3), 0.1) == [0.1, 0.2, nextfloat(0.3)]
@assert frange(0.0, prevfloat(0.3), 0.1) == [0.0, 0.1, 0.2]
@assert frange(0.0, nextfloat(0.3), 0.1) == [0.0, 0.1, 0.2, nextfloat(0.3)]
@assert frange(0.1, 0.3, prevfloat(0.1)) == [0.1, 0.2, 0.3]
@assert frange(0.1, 0.3, nextfloat(0.1)) == [0.1, 0.2]
@assert frange(0.0, 0.3, prevfloat(0.1)) == [0.0, prevfloat(0.1), prevfloat(0.2), 0.3]
@assert frange(0.0, 0.3, nextfloat(0.1)) == [0.0, nextfloat(0.1), nextfloat(0.2)] Some comments:
|
FloatRange: new type for accurate floating-point ranges [fix #2333]
This one is kind of funny
The text was updated successfully, but these errors were encountered: