Skip to content
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

float-rational == is not transitive #3102

Closed
StefanKarpinski opened this issue May 14, 2013 · 5 comments · Fixed by #3110
Closed

float-rational == is not transitive #3102

StefanKarpinski opened this issue May 14, 2013 · 5 comments · Fixed by #3110
Labels
breaking This change will break code needs decision A decision on this change is needed

Comments

@StefanKarpinski
Copy link
Member

Example:

julia> (x,y,z) = (1//10, 0.1, 3602879701896397//36028797018963968);

julia> x == y
true

julia> y == z
true

julia> x == z
false

Originally raised here. To fix this, we would have to make comparisons between rationals and floats do something more conservative, such as converting the float to the exact rational value that it represents. For example, this conversion would do it for many Float64s:

function float2rat(x::Float64)
  ux = reinterpret(Uint64,x)
  int(sign(x))*(1+int(ux & (2^52-1))//2^52)*(2//1)^int((((ux>>52)&(2^11-1))-1023))
end

However, this does have some problems in that it overflows for floats with large exponents, so something slightly cleverer has to be done to make this work. One also doesn't actually need to construct the rational number to do this comparison – cross multiplication and an equality check will also work.

@mlubin
Copy link
Member

mlubin commented May 14, 2013

I think it's reasonable to restrict == to mean exact equality. isapprox should be used in other cases.

@StefanKarpinski
Copy link
Member Author

I agree. We should ensure that == is transitive for all types in Base or we may end up like JavaScript and PHP (where equality isn't even reflexive). It's kind of amazing how difficult it is to make == transitive. There are several intrinsics for int-float comparisons that were fiendishly difficult to concoct.

@StefanKarpinski
Copy link
Member Author

I also kind of like the idea of being able to demonstrate that 1/10 cannot be exactly represented as a float like this:

julia> 1/10
0.1

julia> ans == 1//10
false

StefanKarpinski added a commit that referenced this issue May 15, 2013
This is a fairly insane way to do this, but it works robustly. Even
works for Rational{BigInt}, which I really need to add checks for,
along with all the other functionality.

Temporarily disable some tests in test/numbers.jl until I can sort
out what they ought to check, if anything.
@StefanKarpinski
Copy link
Member Author

Still todo: inequalities. These are always even more of a pain.

StefanKarpinski added a commit that referenced this issue Jun 11, 2013
This is a fairly major change, but it's in line with the recent
change to make comparisons of floats and rationals strict [#3102].
There is still some work to be done, namely:

  - handle large values correctly
  - rational-float inequalities
@stevengj
Copy link
Member

As I mentioned on the mailing list, I think promotion of (float,rational) to rational is a mistake; it makes the Rational type positively dangerous in a floating-point/numerical context, because it means any rational will propagate through the computation, destroying accuracy and performance of whatever it touches. Transitivity of == in mixed rational/float comparison is a small price to pay to avoid this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking This change will break code needs decision A decision on this change is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants