1.0-dev: rework constructors

[OlivierHnt]

This PR reworks the mechanism to construct an Interval.

Minor changes:

• default_bound has been renamed default_numtype for consistency.
• it became more useful and more consistent for atomic to only accept the bound type; so atomic(Interval{T}, x) is now atomic(T, x).

Standard constructors:

• unsafe_interval(T, a, b) is introduced as a replacement for Interval{T}(a, b) which was unsafe, yet semantically very close to the safe constructor interval. The only remaining method for Interval{T} is Interval{T}(::Interval) which is safe since it relies on interval.
• checked_interval function is now removed for clarity (it was already just a mere alias of interval).
• ± can now be used to construct complex intervals when the midpoint is a complex number. EDIT: note that since ± is a midpoint-radius constructor, it is appropriate to define radius(::Complex) such that there is a consistency between x = m ± r and mid(x) ± radius(x). This is defined in the complex.jl file but this file is not yet included into IntervalArithmetic.

Macro constructors:

• @floatinterval and @biginterval are now removed and replaced with the more general macro @tinterval (where the letter "t" stands for "typed"). This new macro has the same capabilities as @interval except that the bound type can be given as the first argument. In particular, @floatinterval expr and @biginterval expr become @tinterval Float64 expr and @tinterval BigFloat expr respectively.
• string can now be parsed to return a rational interval. Unless default_numtype() is redefined to be a Rational, this is not used by default in @interval and @I_str. This feature comes in when calling @tinterval, e.g. @tinterval Rational{Int} "0.1" + pi * 2//3 returns an Interval{Rational{Int}}.

These type of changes required doing quite a lot of editing in many files; I also took this opportunity to do some minor cleanup and improvements in docstrings.

[OlivierHnt]

After looking at some issues, this PR fixes #127, #346.
EDIT: I found this issue #466, where there is a discussion on what bound type sqrt(::Interval{<:Rational}) should return. After this PR, sqrt(::Interval{<:Rational}) returns an Interval{<:Rational}.

[OlivierHnt]

@lbenet @Kolaru @lucaferranti

Any comments/feedback on this PR? Since it involved altering many files, let me know if some things need to be clarified/modified 🙂

A "limitation" of this PR is that @I_str is unable to predict the bound type from parsing and constructs an Interval{default_numtype()}. It could be a nice feature to have at some point, but the mechanism to make this work properly is still unclear to me.

EDIT: Also, I am not so found of introducing the new macro @tinterval to pass a specific bound type; yet it is nicer than having @floatinterval and @biginterval.
As an alternative approach: to only have @interval as a macro constructor we could pass the type via the syntax @interval (T=NumType,) lo hi or @interval (T=NumType,) x. What do you think? 🤔

[Kolaru]

I will have a deep look sometime this week.

About @tinterval and @interval : do we need the macro anyway ? What the macro is currently doing could be done by a function (number literals are parsed before the macro application anyway, we can't intercept the input before parsing), which solve the problem since it would allow multiple dispatch to deal with argument types.

[dpsanders]

@interval is designed so that you can write e.g.

@interval sin(0.1) + cos(0.2)

and it will do the right thing (i.e. wrap the literals in correctly-rounded intervals).

I think it's nice to keep that (and I don't think it can be replaced by a function), although I guess it's not really necessary.

Probably @interval should create a Float64 interval; I would personally prefer to keep that name.

[dpsanders]

Ah, I hadn't read @Kolaru 's comment properly. It's true that literals are parsed before macros; I guess we were doing some hacky workaround for that that should probably be removed! So maybe @interval is indeed not necessary at all.

[OlivierHnt]

Yes the macro @interval is not necessary; the tightest enclosure will be achieved through @I_str.
I guess we ought to also question the other two macro constructors: @decorated and @I_str.

Here is a proposal: we remove @interval and @decorated. Only @I_str remains which constructs, before run-time, either an Interval (current behaviour) or a DecoratedInterval (new feature to introduce). I do not think this introduces type instability since the type of the returned interval should be known at compile-time.

[Kolaru]

I only have small remarks for possible improvements. Nothing blocking it from being merge in my opinion (you are of course welcome to first remove @interval as we discussed if you'd like).

Thanks a lot for the huge work, and especially for the efforts put in the docstrings !

[Kolaru]

It can be documented as T <: NumTypes.

[OlivierHnt]

I thought about this but I did not know if we wanted to export NumTypes or just keep it as an internal constant.
Whether we export NumTypes or not, if we want to document it then I will add a docstring for NumTypes.
Let me know :)

[Kolaru]

Good point, I think then the wisest is to document this as T <: IntervalArithmetic.NumTypes. It is not exported (NumTypes should not I think), and it makes sure this docstring stay consistent if the definition of NumTypes change).

[codecov-commenter]

Codecov Report

Patch coverage: 90.82% and project coverage change: -1.04 ⚠️

Comparison is base (3e2e99e) 85.82% compared to head (5fb804e) 84.79%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@             Coverage Diff             @@
##           1.0-dev     #560      +/-   ##
===========================================
- Coverage    85.82%   84.79%   -1.04%     
===========================================
  Files           34       33       -1     
  Lines         1841     1828      -13     
===========================================
- Hits          1580     1550      -30     
- Misses         261      278      +17     

Impacted Files	Coverage Δ
src/IntervalArithmetic.jl	100.00% <ø> (ø)
src/decorations/functions.jl	81.76% <0.00%> (ø)
src/intervals/complex.jl	0.00% <0.00%> (ø)
src/intervals/interval_operations/overlap.jl	100.00% <ø> (ø)
...rc/intervals/interval_operations/set_operations.jl	61.53% <55.00%> (-14.66%)	⬇️
src/intervals/interval_operations/constants.jl	66.66% <66.66%> (ø)
src/intervals/construction.jl	73.52% <70.90%> (-11.96%)	⬇️
src/decorations/decorations.jl	75.00% <71.42%> (ø)
src/intervals/arithmetic/absmax.jl	85.71% <84.61%> (-14.29%)	⬇️
src/decorations/intervals.jl	85.18% <87.50%> (+0.97%)	⬆️
... and 18 more

... and 1 file with indirect coverage changes

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

[OlivierHnt]

Thank you @Kolaru for the review.
Since you gave me the green light, I decided to go ahead and remove the @interval, @tinterval and @decorated constructors as discussed.

[OlivierHnt]

Nightly is failing. It seems that this is due to the hashing mechanism; perhaps related to this recent commit JuliaLang/julia#49996?

I do not know how this works, but perhaps testing against a specific hash value is not optimal?

[oscardssmith]

Your tests were fine. We just broke hashing on nightly. Should be fixed by the end of today (fix is merged, but CI sometimes takes a bit to see new Julia versions).

[OlivierHnt]

@Kolaru is it possible for you to trigger the CI before merging? Just to double-check.

[Kolaru]

Every light is now green, so I'merging.

Thanks a lot ! This is really helping getting closer to the 1.0 dream :)