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

convert can fail to return a valid interval #539

Closed
OlivierHnt opened this issue Jul 21, 2022 · 3 comments · Fixed by #567
Closed

convert can fail to return a valid interval #539

OlivierHnt opened this issue Jul 21, 2022 · 3 comments · Fixed by #567
Labels

Comments

@OlivierHnt
Copy link
Member

The following conversion fails to return a valid interval:

julia> x = Float64(π)
3.141592653589793

julia> convert(Interval{BigFloat}, x).lo  x  convert(Interval{BigFloat}, x).hi
false

The problem seems to stem from using rationalize; the returned interval is obtained by computingInterval(BigFloat(rationalize(x), RoundDown), BigFloat(rationalize(x), RoundUp)).

However, if x is wrapped as an Interval, then the returned interval is valid:

julia> x = Float64(π)
3.141592653589793

julia> y = Interval(x)
[3.14159, 3.1416]

julia> convert(Interval{BigFloat}, y).lo  x  convert(Interval{BigFloat}, y).hi
true

where here the interval is obtained by computing Interval(BigFloat(x, RoundDown), BigFloat(x, RoundUp)).

@Kolaru Kolaru added the bug label Jul 22, 2022
@Kolaru
Copy link
Collaborator

Kolaru commented Jul 22, 2022

Same problem when creating the interval with Interval{BigFloat}(x) and also exist on 1.0-dev.

@cbilz
Copy link

cbilz commented Sep 28, 2022

Same problem when creating the interval with Interval{BigFloat}(x) and also exist on 1.0-dev.

This seems to work fine on master:

julia> x = Float64(π)
3.141592653589793

julia> x  Interval{BigFloat}(x)
true

julia> x  Interval{BigFloat}(Interval(x))
true

@cbilz
Copy link

cbilz commented Sep 28, 2022

My understanding of the discussion in #51 is that rationalize is used to get this behavior:

julia> 1//3  convert(Interval{BigFloat}, 1/3)
true

Which works because rationalize(1/3) == 1//3.

On the other hand, in almost all cases, x != rationalize(x) and then x ∉ convert(Interval{BigFloat}, x), assuming sufficient precision. This is the case in @OlivierHnt's example. But there is another problem that arises even when decreasing precision: If rationalize(x) is exactly representable in the target type, then the resulting interval is thin and does not contain x:

julia> x = nextfloat(1.0)
1.0000000000000002

julia> y = convert(Interval{Float32}, x)
[1f0, 1f0]

julia> x  y
false

julia> y.lo == y.hi == rationalize(x) == 1
true

From the Julia docs:

since convert can be called implicitly, its methods are restricted to cases that are considered "safe" or "unsurprising"

I would argue that the use of rationalize in convert is surprising because, when called implicitly, it is impossible to know whether a float like 1/3 was intended to represent a rational number like 1//3 or whether it just happened to be the result of some previous float calculations. So it seems to make sense to default to working with the float passed to convert, not rationalizing it.

I think it is also surprising in interactive use, because a user can convert 1//3 instead of 1/3 if they mean the rational number rather than the float. OTOH, with the current behavior it is not straightforward to convert 1/3 if that is the intention.

Something like

convert(::Type{Interval{T}}, x::Real) where T = Interval{T}(T(x, RoundDown), T(x, RoundUp))

would have the advantage of returning an interval that contains x regardless of whether that is a float or something else.

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

Successfully merging a pull request may close this issue.

3 participants