return empty interval when an invalid input is given #461

lucaferranti · 2021-05-15T17:27:05Z

It seems that the standard requires invalid inputs to return an empty interval (see discussion in #457 ), this is also the octave behavior.

Now the package returns an empty interval but also prints a warning (like octave does), a few examples

julia> 3..1
┌ Warning: Invalid input, empty interval is returned
└ @ IntervalArithmetic C:\Users\lucaa\.julia\dev\IntervalArithmetic\src\intervals\intervals.jl:107
∅

julia> interval(Inf, Inf)
┌ Warning: Invalid input, empty interval is returned
└ @ IntervalArithmetic C:\Users\lucaa\.julia\dev\IntervalArithmetic\src\intervals\intervals.jl:107
∅

julia> interval(NaN, π)
┌ Warning: Invalid input, empty interval is returned
└ @ IntervalArithmetic C:\Users\lucaa\.julia\dev\IntervalArithmetic\src\intervals\intervals.jl:107
∅

julia> @decorated 3 1
[NaN, NaN]_ill

julia> @decorated NaN 3
[NaN, NaN]_ill

julia> @decorated Inf Inf
[NaN, NaN]_ill

julia> @decorated Inf -Inf
∅_trv
```

codecov-commenter · 2021-05-15T17:29:49Z

Codecov Report

Merging #461 (73cc938) into master (65ca77f) will increase coverage by 0.03%.
The diff coverage is 91.30%.

@@            Coverage Diff             @@
##           master     #461      +/-   ##
==========================================
+ Coverage   91.53%   91.56%   +0.03%     
==========================================
  Files          25       25              
  Lines        1748     1755       +7     
==========================================
+ Hits         1600     1607       +7     
  Misses        148      148

Impacted Files	Coverage Δ
src/intervals/intervals.jl	`92.18% <89.47%> (+0.66%)`	⬆️
src/decorations/intervals.jl	`84.78% <100.00%> (ø)`
src/intervals/special.jl	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 65ca77f...73cc938. Read the comment docs.

src/intervals/intervals.jl

src/decorations/intervals.jl

lucaferranti · 2021-05-15T17:51:06Z

is it possible to suppress warnings during tests? I guess printing all the warnings is not very nice/useful?

src/intervals/intervals.jl

dpsanders · 2021-05-15T18:26:56Z

is it possible to suppress warnings during tests? I guess printing all the warnings is not very nice/useful?

Looks like you can test the warning using @test_logs

dpsanders · 2021-05-15T19:00:08Z

Apparently the standard requires an empty interval to be returned when constructing incorrect intervals. There could maybe be a flag passed to `interval` yo specify the behaviour, but we would need to benchmark that.

…

On Sat, 15 May 2021, 14:57 Luis Benet, ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In src/intervals/intervals.jl <#461 (comment)> : > +function interval(a::T, b::S) where {T<:Real, S<:Real} if !is_valid_interval(a, b) - throw(ArgumentError("`[$a, $b]` is not a valid interval. Need `a ≤ b` to construct `interval(a, b)`.")) + @warn "Invalid input, empty interval is returned" + return emptyinterval(promote_type(T, S, Float64)) That's precisely my point: I think using interval should throw an error, instead of a warning, as it is proposed. This behavior could be changed for a specific "flavor". — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#461 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AABO2TUDZFT3DB2BLPABKRTTN2727ANCNFSM446CTKQA> .

lucaferranti · 2021-05-29T19:51:42Z

The current implementation of .. was calling atomic before checking the validity of the input, the commit above this message fixes it.

In the file ieee1788-exceptions.itl of the ITL testsuite there is a test

b-numsToInterval +infinity -infinity = [empty] signal UndefinedOperation;

so basically interval(Inf, -Inf) should return the empty interval and flag a warning.I modified the definition of is_valid_interval to be conformant to this, but personally I think printing the warning in this case is a little redundant, since [Inf, -Inf] is the representation of the empty interval, opinions on this?

lbenet

I've added few comments, some may be related to a possible type instability, which should be easy to check.

src/intervals/intervals.jl

lbenet

LGTM!

lucaferranti · 2021-05-30T19:25:32Z

Great! added a couple of more tests and as a far as I'm concerned, this is ready for merge now. This fixes step 6 of #465 .
I am next going to fix the constructors for decorated intervals (steps 4 and 10 of #465 ) and next investigate the parsing of strings.

Should we already bump the version and release or wait a few days for the upcoming PRs?

Kolaru · 2021-05-31T19:40:30Z

src/intervals/intervals.jl

@@ -25,7 +25,8 @@ struct Interval{T<:Real} <: AbstractInterval{T}
                new(a, b)

            else
-                throw(ArgumentError("Interval of form [$a, $b] not allowed. Must have a ≤ b to construct interval(a, b)."))
+                @warn "Invalid input, empty interval is returned"


Would it be reasonnable to never check the validity here, get rid of the validity_check setting and document that to get IEEE compliant and thouroughly validated interval once should use either interval or DecoratedInterval?

Related but not directly in the scope of this PR, I wonder if it wouldn't be clearer for users to have Interval as the default IEEE compliant one and use some trick to get an unsafe_interval function for internal operations.

If you thing it is too tangential to the issue tackled here, I can add a separate issue.

validity_check is indeed set by default to false and since you have functions like .. and interval I think it could be removed without too much harm.

The Interval vs interval might be an interesting discussion. One thing that puzzles me is the asymmetry of bare and decorated intervals, in the sense that for bare the constructors Interval doesn't do any checks and you should use interval or .., but for decorated intervals the constructors DecoratedIntervals does checks (well a few issues there, next PR on the way for that :D ). I think it might be important to have a mechanism to construct "unsafe intervals" which merely puts the given inputs as lower and upper bound, at least for testing purposes. Maybe this can be addressed in a separate issue?

Kolaru · 2021-05-31T19:42:21Z

src/intervals/intervals.jl

@@ -106,9 +97,10 @@ end

 `interval(a, b)` checks whether [a, b] is a valid `Interval`, which is the case if `-∞ <= a <= b <= ∞`, using the (non-exported) `is_valid_interval` function. If so, then an `Interval(a, b)` object is returned; if not, then an error is thrown.
 """
-function interval(a::Real, b::Real)
+function interval(a::T, b::S) where {T<:Real, S<:Real}


This docstring is no longer correct, it never throws an error.

Also precising that this is the way of creating IEEE compliant interval would be great.

right! I had a separated PR (#460 ) for the docstrings which I had opened before this. The idea was to rebase that once this is merged and fix the docstrings there but indeed... it makes no sense! I'll close that PR and take care of the docstrings here :D

updated the docstring. II think the note about IEEE compliance may be more appropriate after the package is actually IEEE compliant? what do you think?

I think it is fine like that. This will be discussed in #468 where will have to sort out what we want each constructor to do anyway.

Even if the full package is not yet compliant, I think it will be good to declare our intent on which constructor corresponds to the IEEE constructors.

Kolaru · 2021-06-02T00:58:20Z

Thanks a lot!

lucaferranti added 2 commits May 15, 2021 20:02

return empty interval for invalid input

69cd825

fix decorated interval construction with invalid input

80875fe

lucaferranti commented May 15, 2021

View reviewed changes

src/intervals/intervals.jl Outdated Show resolved Hide resolved

lucaferranti commented May 15, 2021

View reviewed changes

src/decorations/intervals.jl Show resolved Hide resolved

lucaferranti changed the title ~~Lf/invalid as empty~~ return empty interval when an invalid input is given May 15, 2021

lucaferranti linked an issue May 15, 2021 that may be closed by this pull request

Should Interval(Inf, Inf) give the empty set? #457

Closed

lbenet reviewed May 15, 2021

View reviewed changes

src/intervals/intervals.jl Outdated Show resolved Hide resolved

lucaferranti mentioned this pull request May 27, 2021

ITF1788 compliance #465

Closed

16 tasks

fix invalid input for .., capture warnings in tests

65778de

lucaferranti linked an issue May 29, 2021 that may be closed by this pull request

Inconsistent errors when constructing intervals with NaN bound #325

Closed

lbenet reviewed May 29, 2021

View reviewed changes

fix type instability

e495251

lbenet approved these changes May 30, 2021

View reviewed changes

lucaferranti added 2 commits May 30, 2021 21:01

added a few more tests for invalid input with ..

53949ce

tests for emptyinterval(Int) and entireinterval(Int)

73cc938

Kolaru reviewed May 31, 2021

View reviewed changes

lucaferranti mentioned this pull request May 31, 2021

update interval-macro docstring #460

Closed

update docstrings

e24bf6e

Kolaru mentioned this pull request May 31, 2021

Consistency in validity check during interval creation #468

Closed

Kolaru merged commit c7602f4 into JuliaIntervals:master Jun 2, 2021

lucaferranti deleted the lf/invalid_as_empty branch June 2, 2021 06:18

lucaferranti mentioned this pull request Jun 15, 2021

Add floatmin and floatmax #477

Closed

lbenet mentioned this pull request Jan 6, 2022

Type consistency for .. #418

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

return empty interval when an invalid input is given #461

return empty interval when an invalid input is given #461

lucaferranti commented May 15, 2021

codecov-commenter commented May 15, 2021 •

edited

Loading

lucaferranti commented May 15, 2021

dpsanders commented May 15, 2021

dpsanders commented May 15, 2021 via email

lucaferranti commented May 29, 2021

lbenet left a comment

lbenet left a comment

lucaferranti commented May 30, 2021

Kolaru May 31, 2021

lucaferranti May 31, 2021

Kolaru May 31, 2021

lucaferranti May 31, 2021 •

edited

Loading

lucaferranti Jun 1, 2021 •

edited

Loading

Kolaru Jun 2, 2021

Kolaru commented Jun 2, 2021

return empty interval when an invalid input is given #461

return empty interval when an invalid input is given #461

Conversation

lucaferranti commented May 15, 2021

codecov-commenter commented May 15, 2021 • edited Loading

Codecov Report

lucaferranti commented May 15, 2021

dpsanders commented May 15, 2021

dpsanders commented May 15, 2021 via email

lucaferranti commented May 29, 2021

lbenet left a comment

Choose a reason for hiding this comment

lbenet left a comment

Choose a reason for hiding this comment

lucaferranti commented May 30, 2021

Kolaru May 31, 2021

Choose a reason for hiding this comment

lucaferranti May 31, 2021

Choose a reason for hiding this comment

Kolaru May 31, 2021

Choose a reason for hiding this comment

lucaferranti May 31, 2021 • edited Loading

Choose a reason for hiding this comment

lucaferranti Jun 1, 2021 • edited Loading

Choose a reason for hiding this comment

Kolaru Jun 2, 2021

Choose a reason for hiding this comment

Kolaru commented Jun 2, 2021

codecov-commenter commented May 15, 2021 •

edited

Loading

lucaferranti May 31, 2021 •

edited

Loading

lucaferranti Jun 1, 2021 •

edited

Loading