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

transcendental functions type-unstable for Interval{Rational} #466

Closed
lucaferranti opened this issue May 27, 2021 · 5 comments
Closed

transcendental functions type-unstable for Interval{Rational} #466

lucaferranti opened this issue May 27, 2021 · 5 comments

Comments

@lucaferranti
Copy link
Member

e.g.

julia> @code_warntype sqrt(1//2..3//4)
Variables
  #self#::Core.Const(sqrt)
  a@_2::Interval{Rational{Int64}}
  domain::Interval{Rational{Int64}}
  a@_4::Interval{Rational{Int64}}

Body::Union{Interval{Rational{Int64}}, Interval{Float64}}
1 ─       (a@_4 = a@_2)
│   %2  = Core.apply_type(IntervalArithmetic.Interval, $(Expr(:static_parameter, 1)))::Core.Const(Interval{Rational{Int64}})
│         (domain = (%2)(0, IntervalArithmetic.Inf))
│         (a@_4 = a@_4 ∩ domain::Core.Const([0//1, 1//0]))
│   %5  = IntervalArithmetic.isempty(a@_4)::Bool
└──       goto #3 if not %5
2 ─       return a@_4
3 ─ %8  = Base.getproperty(a@_4, :lo)::Rational{Int64}
│   %9  = IntervalArithmetic.sqrt(%8, RoundingMode{:Down}())::Float64
│   %10 = Base.getproperty(a@_4, :hi)::Rational{Int64}
│   %11 = IntervalArithmetic.sqrt(%10, RoundingMode{:Up}())::Float64
│   %12 = IntervalArithmetic.Interval(%9, %11)::Interval{Float64}
└──       return %12
@lucaferranti
Copy link
Member Author

lucaferranti commented May 28, 2021

what should e.g. sqrt(1//2..3//4) return anyway? empty interval? change to Interval{Float64}? to Interval{BigFloat}? I think the 1788 way would probably be to print a warning and return an empty interval?

@dpsanders
Copy link
Member

I think it should call sqrt(float(x))

@lbenet
Copy link
Member

lbenet commented May 28, 2021

I fully agree with David: If the user is working with Interval{Rational{Int}}, it is because it makes sense, so I guess the user deals at most with rational functions with integer or rational coefficients. If by some reason this is not the case, promotion has to set in, which in this case means return Interval{Float64} (or if the original interval is Interval{Rational{BigInt}}, then returning Interval{BigFloat}), and then it is the responsibility of the user to stick to this type or convert later to Interval{Rational{Int}}.

The reason to have Interval{Rational{T}} is, if I recall correctly, that some arithmetic operations are exact for rationals (e.g. rational functions with integer or rational coefficients), so no rounding needs to be done.

@Kolaru
Copy link
Collaborator

Kolaru commented May 31, 2021

I don't remember the standard ever mentioning bounds that are not floats, so I think that we are free to promote here.

@OlivierHnt
Copy link
Member

This is fixed on master, note that the return type is Interval{Rational{Int64}}. While returning Interval{Float64} gives a sharper answer, we should avoid type promotion when a valid Interval{Rational{Int64}} can be returned. After all,Rational{Int64} is a supported type for the interval bounds.
Also, there is no loss here, the sharpest result can always be retrieved via sqrt(float(x)) which makes the bound type promotion explicit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants