Codecov Report

Attention: 196 lines in your changes are missing coverage. Please review.

Comparison is base (742a26f) 84.06% compared to head (10c348e) 83.01%.

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master     #593      +/-   ##
==========================================
- Coverage   84.06%   83.01%   -1.05%     
==========================================
  Files          26       23       -3     
  Lines        1970     2072     +102     
==========================================
+ Hits         1656     1720      +64     
- Misses        314      352      +38     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

It seems that there is still more work to be done to have better ForwardDiff.jl compatibility. I have added a few broken tests to keep track of this a little.

For instance, ForwardDiff.derivative(x -> x^interval(2), interval(-1, 1)) throws a MethodError due to method ambiguity; which is probably not so bad to fix.

On the other hand, ForwardDiff seems to rely on T(::Real), where T<:Real, for conversion. This is something we have discontinued in favour of explicit convert call; but perhaps we can re-allow it (with the extra isguarranteed == false of course). Perhaps simply

Interval{T}(x) where {T<:NumTypes} = convert(Interval{T}, x)

Looks again like a giant PR is coming 😄 (or 🥲 ? )... Thanks again for the enourmous effort.

Few comments (note that apriori I do not opose to the changes):

• changed behaviour of functions in numeric.jl for numbers; e.g. sup(Inf) = NaN (Inf is first converted into an Interval), previous behaviour sup(Inf) == Inf

  Can you explain the motivation? It seems that this is some default method, to be appied to Reals. Note in pass that sup(5.0) returns 5.0 (not an interval), so I think returning Inf is consistent...

• change default bisection to be at 0.5 by default

  If I recall correctly, the value (127//256) was considered to "break slightly the symmetry" (for symmetric intervals) which in somecases complicates applying IntervalRootFinding methods; in other words, breaking such symmetry permitted faster convergence. (I think this was inspired by a comment in the book by Tucker.)

• remove atomic since it is an unused internal constructor (see also Make decorated interval the default #590 (comment))

  We could also consider keeping atomic if it indeed returns an atomic interval (either a thin interval of a one-eps wide interval, except perhaps for unbounded intervals). I'm thinking about the possibility to use it to construct tight intervals, that do contain the actual real number, as atomic(Float64, "0.1") does. Issues about performance can be stated in the docs. This is, at the end, also related to rounding.

• A side comment, related to the ITF1788 test suite: I am in favor of having the rev functions within IntervalArithmetic; I truly do not see any scenario of using such functions separated from IA...

• I don't understand the to-do:

  rework rounding to stop doing type piracy (currently we overload sqrt, *, /, etc from Base)

  My point is, rather than type-piracy, we do method-overloading exploiting multiple dispatch. I am in favor of keeping those functions.

• Perhaps the to-do and to-do list should be addressed in a separate PR... makes life easier to review. The question is if the compatibility with ForwardDiff should be also addressed in another PR. Once said this, if T(x) is needed by ForwardDiff, your solution seems ok with me. The question is if we can use ForwardDiff in a guaranteed form.

Mhm @interval has very poor performance; this is due to parse, e.g.:

julia> @time IntervalArithmetic.atomic(Float64, 1)
  0.000029 seconds (14 allocations: 644 bytes)
[1.0, 1.0]_com

EDIT: with the last commit, this is down to 8 allocations (300 bytes); there is still room for improvement.

Just noticed the following (which relies on Base methods!):

julia> a = interval(-1, 1)
Interval{Float64}(-1.0, 1.0, com)

julia> b = complex(a, a)
Interval{Float64}(-1.0, 1.0, com) + Interval{Float64}(-1.0, 1.0, com)im

julia> zero(a)  # OK
Interval{Float64}(0.0, 0.0, com)

julia> zero(b)  # Result is ok, except perhaps for the NG (due to using a method from Base)
Interval{Float64}(0.0, 0.0, com, NG) + Interval{Float64}(0.0, 0.0, com, NG)im

julia> @which zero(b)
zero(x::Number)
     @ Base number.jl:308

Maybe we should open a new issue with the most basic operations we want to have for ComplexI... (Truly, I adore that alias!)

I added a specialised method for zero and one.

We should perhaps merge this PR and release 0.22? This will allow us getting more feedback from downstream packages during December.

Please go ahead! I fully agree merging this PR and releasing v0.22 will allow us to check everything more reliably. Great work!