1.0-dev: fix interval(::Interval) and some ambiguities

[OlivierHnt]

• Fix ambiguities with +, -, * and / when the operands are Intervals with different numtype. The mechanism relies on promote, so promote_rule is defined, as a side-effect [Interval{Float32}(pi), Interval{Float64}(pi)]::Vector{Interval{Float64}} with guaranteed error bounds.
• Fix typos in dosctrings

Showcase:

Before this PR:

julia> Interval{Float64}(1) + Interval{Float32}(1)
ERROR: MethodError: +(::Interval{Float64}, ::Interval{Float32}) is ambiguous.

Candidates:
  +(b::Real, a::Interval)
    @ IntervalArithmetic ~/.julia/dev/dev_IA/IntervalArithmetic/src/intervals/arithmetic/basic.jl:31
  +(a::Interval{T}, b::S) where {T, S<:Real}
    @ IntervalArithmetic ~/.julia/dev/dev_IA/IntervalArithmetic/src/intervals/arithmetic/basic.jl:30

Possible fix, define
  +(::Interval{T}, ::S) where {T, S<:Interval}

Stacktrace:
 [1] top-level scope
   @ REPL[6]:1

julia> promote_type(Interval{Float32}, Interval{Float64})
Interval

Julia> showfull( interval(Interval(pi)) )
Interval([3.14159, 3.1416], [3.14159, 3.1416])

After this PR

julia> Interval{Float64}(1) + Interval{Float32}(1)
   [2, 2]

julia> promote_type(Interval{Float32}, Interval{Float64})
Interval{Float64}

julia> showfull( interval(Interval(pi)) )
Interval(3.141592653589793, 3.1415926535897936)

[lucaferranti]

sorry for the delay! I'll try to have a look tomorrow

[codecov-commenter]

Codecov Report

Base: 85.22% // Head: 85.47% // Increases project coverage by +0.25% 🎉

Coverage data is based on head (28dd065) compared to base (1d2e6e4).
Patch coverage: 89.47% of modified lines in pull request are covered.

Additional details and impacted files

@@             Coverage Diff             @@
##           1.0-dev     #554      +/-   ##
===========================================
+ Coverage    85.22%   85.47%   +0.25%     
===========================================
  Files           34       34              
  Lines         1793     1804      +11     
===========================================
+ Hits          1528     1542      +14     
+ Misses         265      262       -3     

Impacted Files	Coverage Δ
src/intervals/construction.jl	93.44% <75.00%> (-3.11%)	⬇️
src/intervals/arithmetic/basic.jl	91.80% <100.00%> (+0.27%)	⬆️
src/intervals/rounding.jl	60.00% <0.00%> (-7.15%)	⬇️
src/display.jl	83.01% <0.00%> (+0.10%)	⬆️
src/IntervalArithmetic.jl	60.00% <0.00%> (+10.00%)	⬆️
src/intervals/rounding_macros.jl	92.30% <0.00%> (+92.30%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[OlivierHnt]

No worries, thank you for taking the time 🙂

[OlivierHnt]

When I initially made this PR, I did not look at the changelog.md file. I see that conversions and promotions are purposely disallowed to guarantee correctness.

However, doing this still does not guarantee that generic methods for Real or Number will error. The first example that comes to mind is norm from LinearAlgebra because it relies on < and float which are implemented for Interval. To fully guarantee correctness, one probably needs to not subtype Interval to anything (right now: Interval <: Real). I think there is a branch which experimented with this, how did it go?

On the other hand, the IntervalArithmetic.jl package should not have to guarantee correctness for functions defined outside of its scope. Arguably, anyone performing rigorous computations should make sure that the functions they call are safe for Interval.
Personally, I lean more towards this point of view: have a module which defines correct functions for Interval. The user can then be warned that using any other functions may result in silent failure of correctness.

cc @lucaferranti @Kolaru @lbenet

[Kolaru]

Ho ho ho you found our Pandora's box already ^^'

First some reference of past related discussions #2 #165 #168 #237 #309 and #271 (on which the branch 1.0-dev is based).

And now for a brief summary:

To fully guarantee correctness, one probably needs to not subtype Interval to anything (right now: Interval <: Real). I think there is a branch which experimented with this, how did it go?

You are right. That's the only way to guarantee correctness. However in order to have interop we need to subtype Real so the idea of the experimentation was to have two kind of interval, one subtype of Real and the other not. We gave up on that, as along the way I started to refactor a lot to separate what is from the standard and what isn't, which lead to the 1.0-dev branch. We decided it was more important to first have a full stable implementation, well refactored and well documented.

The first example that comes to mind is norm from LinearAlgebra because it relies on < and float which are implemented for Interval.

For interop in principle we only need to be sure we override all Base function with their proper interval extension. It sounds easy enough, but the IEEE interval standard makes it tricky, because it has its own definition of some operations and symbols. For example, it defines < for interval, but not as the interval extension of pointwise <, making direct translation of the standard and correct interop with Base incompatible.

That's where the concept of flavors comes in. Flavors would allow to tune the behavior of interval for that kind of operations, and all corner cases that may be misinterpreted. This is also one of the thing that are slowly going into 1.0-dev.

Arguably, anyone performing rigorous computations should make sure that the functions they call are safe for Interval.

This is a fair point, but we had greater ambitions ^^. My idea of disallowing promotion is that we should provide as much guarantee as possible out of the box. Promotion however is an easy way to shoot ourselves in the foot, as it makes it quite tricky to actually make sure all functions you call are safe for Interval. Since some function could promote, it is hard to know which functions you are actually using.

Now I haven't review this PR yet (will try to do it soon), if your implementation is trustworthy enough to avoid the problem we had in the past, I'm personnally fine bringing promotion back in 1.0-dev.

[lbenet]

Let me provide an example of what Benoit mentions about interoperability, where I am involved: TaylorSeries.jl defines some parametric types (e.g., Taylor1{T}), whose parameter T are subtypes of Number. Among other, this allows us to have Taylor series defined aver intervals, since Interval <: Real (!); notice that, while IntervalArithmetic.jl is not a dependency, there is specific code loaded (through Requires) in case of necessity. All this is (hopefully!) rigorously exploited in TaylorModels.jl, where the Taylor approximation of a function is rigorously bounded (with an interval) on a domain (which is also an interval). The point is: to have this, we explicitly need Interval <: Real. The point of this comment is that, currently, we need such subtyping.

[OlivierHnt]

Thanks for the detailed answer! 🙂

Indeed there were issues in the past with conversion/promotion, see also #539.
At first glance at what you linked @Kolaru (but interval arithmetic is full of subtlety, and I may have missed a counter-example) I do not think this PR will suffer from these issues because it relies only on T(x, RoundDown) and T(x, RoundUp) where T <: AbstractFloat to construct the bounds of the Interval. There is nothing clever about the mechanism, in contrast with the current mechanism on master.

The downside is that perhaps someone will have a slightly wider interval than necessary 🤷‍♂️.
However I think this is acceptable as it is generally what generic code in Julia is: working (meaning guaranteed correctness) but maybe not optimal (meaning maybe not the tightest interval).

This is a fair point, but we had greater ambitions ^^. My idea of disallowing promotion is that we should provide as much guarantee as possible out of the box. Promotion however is an easy way to shoot ourselves in the foot, as it makes it quite tricky to actually make sure all functions you call are safe for Interval. Since some function could promote, it is hard to know which functions you are actually using.

Indeed not "mixing" Real and Interval through conversion/promotion is a guardrail. On the other hand, it comes with a cost on interoperability as it hinders writing efficient generic code. E.g. not being able to construct a type-stable vector [x, y] where x is an Interval.
Also, while incompatibilities with Base may be resolved by introducing the flavour mechanism, there are still no guarantee that the generic function called by the user does not conflate distinct flavours... At the end of the day, one still needs to take a look at the code. Hence I think whether the conversion/promotion is permitted or not, the warning is the same: IntervalArithmetic.jl does not guarantee that any function call (beyond the set of operations defined in the module) perform rigorous computations.

Of course I am very biased on this topic 😅 since I am in a similar position as @lbenet (thanks for the work on TaylorSeries.jl and TaylorModels.jl, these are awesome libraries!). I am the developer of RadiiPolynomial.jl a library for computer-assisted proofs in dynamical systems (which includes Taylor but also Fourier and Chebyshev series). So I really appreciate having a flexible Interval type which combines well with the Julia ecosystem. In particular, having Interval an instance of a Real makes life a lot easier and I definitely would not suggest to remove this feature.

[lbenet]

Aside from the comment I left, which require a small change, LGTM, so I'm in favor of merging it.

[lbenet]

I didn't know about RadiiPolynomial.jl; looks very interesting! Thanks for sharing it!

[OlivierHnt]

I have made the suggested change: allow promotion only between Interval types.
I have also added a few tests.

[lbenet]

LGTM!

[lbenet]

This PR is ok from my point of view; and thus I'm in favor of merging it. Any comments @lucaferranti, @Kolaru or @dpsanders ?

[Kolaru]

All look good to me.

I think that regarding the discussion, your opinion is actually in line with the consensus here :).

As per promotion, I should have read the PR first. The problematic promotion was the promotion between number and interval[1][2]. Promoting interval of different types causes no problem as far as I can see.

[1] Promotion between non interval Number and Interval really, since our intervals here are Number themselves
[2] That way we didn't have to define all combinations of operations mixing number and intervals ourselves. It brought quite hard to trace bug though, so was abandoned.

[lbenet]

Great! So let's go ahead and merge this!

Thanks a lo @OlivierHnt !