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

Failure of Interval{T}(x::AbstractIrrational) #588

Closed
mfherbst opened this issue Oct 11, 2023 · 7 comments
Closed

Failure of Interval{T}(x::AbstractIrrational) #588

mfherbst opened this issue Oct 11, 2023 · 7 comments

Comments

@mfherbst
Copy link
Contributor

The exported function Interval{T}(x::AbstractIrrational) currently fails:

julia> Interval(π)
ERROR: MethodError: no method matching atomic(::Type{Interval{Float64}}, ::Irrational{:π})

Closest candidates are:
  atomic(::Type{T}, ::Any) where T<:Union{AbstractFloat, Rational}
   @ IntervalArithmetic ~/.julia/packages/IntervalArithmetic/fEeQI/src/intervals/construction.jl:250

Stacktrace:
 [1] #s30#144
   @ ~/.julia/packages/IntervalArithmetic/fEeQI/src/IntervalArithmetic.jl:124 [inlined]
 [2] var"#s30#144"(T::Any, ::Any, x::Any)
   @ IntervalArithmetic ./none:0
 [3] (::Core.GeneratedFunctionStub)(::Any, ::Vararg{Any})
   @ Core ./boot.jl:602
 [4] Interval(x::Irrational{:π})
   @ IntervalArithmetic ~/.julia/packages/IntervalArithmetic/fEeQI/src/IntervalArithmetic.jl:129
 [5] top-level scope
   @ REPL[2]:1
@mfherbst
Copy link
Contributor Author

I should maybe add that interval(π) works, but I suppose it makes sense to have this one around as well.

@OlivierHnt
Copy link
Member

Thanks for reporting the issue. I presume you are using IntervalArithmetic.jl v0.21.1?

@mfherbst
Copy link
Contributor Author

yep, indeed.

@OlivierHnt
Copy link
Member

OlivierHnt commented Oct 11, 2023

Ah I see the issue, some lines of code were not properly removed when we released 0.21.

Let me say that Interval as a constructor was purposely removed since it was too close semantically to interval. However the latter is compliant with the standard, while the former is not. In the end we just removed everything for consistency (even though Interval(::AbstractIrrational) should also be a safe constructor).
If you need to recover the "unsafe construction" (I do not advise it) there is an unexported unsafe_interval function.

So the "bug" here is that the error message is not the right one.

@mfherbst
Copy link
Contributor Author

mfherbst commented Oct 11, 2023

(even though Interval(::AbstractIrrational) should also be a safe constructor).

I see, thanks for your quick response.

I understand the overall decision of the redesign of IntervalArithmetic, but let me just add my two cents: My use case of intervals is also a little more on the pragmatic side, such as #580. For me conversions like Interval(::AbstractIrrational) get called in my code during "normal operation" during Julia's normal type promotion / conversion. Given that this is a safe constructor, would it not make sense to keep it for convenience ?

@OlivierHnt
Copy link
Member

OlivierHnt commented Oct 11, 2023

Yes indeed. After discussions, we truly wanted an Interval object that could not be "accidentally" produced from generic code. This means fighting against Julia's (amazing) flexibility at the cost of interoperability and inconvenience.
In particular, we wanted to have users spelling out the constructor function, here interval, and not rely on a "type constructor"; e.g.

generic_function(::Type{T}, a) where {T} = T(log(a))
generic_function(Interval, 3) # danger zone

That being said, issue #580 (see also #568, #584) is being worked on currently (I should have a PR ready very soon 🙂). The idea is that Interval will always be decorated. This will allow us to bring back conversion/promotion and other "mixing" with Number by specifying an appropriate decoration. On the other hand, the current Interval struct will be renamed BareInterval.
Hopefully the next IntervalArithmetic.jl version will feel as convenient as 0.20 did, and at the same time be more reliable.

OlivierHnt added a commit that referenced this issue Oct 11, 2023
@mfherbst
Copy link
Contributor Author

Thanks for your quick response and detailed explanation, much appreciated!

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

2 participants