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

unwise new rational/float promotion rules #3378

Closed
stevengj opened this issue Jun 13, 2013 · 3 comments
Closed

unwise new rational/float promotion rules #3378

stevengj opened this issue Jun 13, 2013 · 3 comments
Labels
needs decision A decision on this change is needed

Comments

@stevengj
Copy link
Member

julia> e * (1//2) - e/2
ERROR: invalid rational: 0//0
 in Rational at rational.jl:7
 in - at promotion.jl:137

whereas float64(e * (1//2)) - e/2 works (and gives 0.0).

This seems like a bug to me. See also commit 24dbefc. @StefanKarpinski?

@stevengj
Copy link
Member Author

It looks like this is a disastrous overflow that occurs from the ill-advised promotion rules implemented for #3102. See also the mailing list.

Basically, any Rational value is now poison for numerical computation, propagating through arithmetic.

My feeling is that promote(float,rational) should give float. Transitivity be damned, this is way worse.

@GunnarFarneback
Copy link
Contributor

#3382 mitigates a subset of these problems, including the example above. It has no effect on other expressions though, like

julia> (0.1 * 1//1)^2
-Inf

so it doesn't substantially change the situation.

@tomasaschan
Copy link
Member

After reading through the comments on commit 6125749, I think this issue arises from confusing two different problems from a user perspective:

  1. deciding how to handle the operands in mathematical operations involving both Rationals and FloatingPoints
  2. finding a Rational that corresponds to a given float - either exactly or within some tolerance (and preferrably with "small" numerator and denominator).

I think the possibility to do 2. is a good goal, but not at the cost of predictability in 1. Rather than using the convert and promote methods for this, it is nice to have it as a separate method - rationalize seems to be a good candidate, or maybe a constructor for Rational that takes a number of some other type? Then, rationalize would maybe have to be (partially) re-implemented to be unaffected by promotion rules etc, and the promotion rules can be reverted to the previous, better, behavior.

StefanKarpinski added a commit that referenced this issue Jun 14, 2013
Revert promotion behavior of rationals with floating-point values
to behavior prior to 6125749 and 24dbefc, i.e. floating-point
wins. Also includes a couple of promotion tweaks for MathConsts.

See discussion at

https://groups.google.com/forum/?fromgroups#!searchin/julia-dev/rational/julia-dev/2JcZdFKisis/Ag9rBJrrqQQJ
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs decision A decision on this change is needed
Projects
None yet
Development

No branches or pull requests

4 participants