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.

Sign up for GitHub

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

Jump to bottom

return empty interval when an invalid input is given #461

Merged
merged 7 commits into from
Jun 2, 2021
Merged

return empty interval when an invalid input is given #461

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
69cd825
return empty interval for invalid input
lucaferranti May 15, 2021
80875fe
fix decorated interval construction with invalid input
lucaferranti May 15, 2021
65778de
fix invalid input for .., capture warnings in tests
lucaferranti May 29, 2021
e495251
fix type instability
lucaferranti May 30, 2021
53949ce
added a few more tests for invalid input with ..
lucaferranti May 30, 2021
73cc938
tests for emptyinterval(Int) and entireinterval(Int)
lucaferranti May 30, 2021
e24bf6e
update docstrings
lucaferranti May 31, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/decorations/intervals.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ DecoratedInterval(I::Interval{T}, d::DECORATION) where T<:AbstractFloat =
DecoratedInterval{T}(I, d)

function DecoratedInterval(a::T, b::T, d::DECORATION) where T<:Real
a > b && return nai(T)
lucaferranti marked this conversation as resolved.
Show resolved Hide resolved
is_valid_interval(a, b) || return nai(T)
DecoratedInterval(Interval(a,b), d)
end

Expand All @@ -63,7 +63,7 @@ DecoratedInterval(a::T, b::S, d::DECORATION) where {T<:Real, S<:Real} =
DecoratedInterval(I::Interval) = DecoratedInterval(I, decoration(I))

function DecoratedInterval(a::T, b::T) where T<:Real
a > b && return nai(T)
is_valid_interval(a, b) || return nai(T)
DecoratedInterval(Interval(a,b))
end

Expand Down
41 changes: 22 additions & 19 deletions src/intervals/intervals.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

return new(T(Inf), T(-Inf))
lbenet marked this conversation as resolved.
Show resolved Hide resolved
end

end
Expand Down Expand Up @@ -79,20 +80,10 @@ function is_valid_interval(a::Real, b::Real)
# println("isvalid()")

if isnan(a) || isnan(b)
if isnan(a) && isnan(b)
return true
else
return false
end
return false
end

if a > b
if isinf(a) && isinf(b)
return true # empty interval = [∞,-∞]
else
return false
end
end
a > b && return false

if a == Inf || b == -Inf
return false
lbenet marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -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}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

@lucaferranti lucaferranti May 31, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

@lucaferranti lucaferranti Jun 1, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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))
end

return Interval(a, b)
Expand Down Expand Up @@ -141,21 +133,32 @@ include("complex.jl")
# Syntax for intervals

function ..(a::T, b::S) where {T, S}
interval(atomic(Interval{T}, a).lo, atomic(Interval{S}, b).hi)
if !is_valid_interval(a, b)
@warn "Invalid input, empty interval is returned"
return emptyinterval(promote_type(T, S))
end
Interval(atomic(Interval{T}, a).lo, atomic(Interval{S}, b).hi)
end

function ..(a::T, b::Irrational{S}) where {T, S}
if !is_valid_interval(a, b)
@warn "Invalid input, empty interval is returned"
return emptyinterval(promote_type(T, Irrational{S}))
end
R = promote_type(T, Irrational{S})
interval(atomic(Interval{R}, a).lo, R(b, RoundUp))
Interval(atomic(Interval{R}, a).lo, R(b, RoundUp))
end

function ..(a::Irrational{T}, b::S) where {T, S}
if !is_valid_interval(a, b)
@warn "Invalid input, empty interval is returned"
return emptyinterval(promote_type(Irrational{T}, S))
end
R = promote_type(Irrational{T}, S)
return interval(R(a, RoundDown), atomic(Interval{R}, b).hi)
return Interval(R(a, RoundDown), atomic(Interval{R}, b).hi)
end

function ..(a::Irrational{T}, b::Irrational{S}) where {T, S}
R = promote_type(Irrational{T}, Irrational{S})
return interval(a, b)
end

Expand Down
2 changes: 2 additions & 0 deletions src/intervals/special.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ larger than the upper one."""
emptyinterval(::Type{T}) where T<:Real = Interval{T}(Inf, -Inf)
emptyinterval(x::Interval{T}) where T<:Real = emptyinterval(T)
emptyinterval() = emptyinterval(precision(Interval)[1])
emptyinterval(::Type{<:Integer}) = emptyinterval(Float64)
const ∅ = emptyinterval(Float64)

isempty(x::Interval) = x.lo == Inf && x.hi == -Inf
Expand All @@ -20,6 +21,7 @@ const ∞ = Inf
entireinterval(::Type{T}) where T<:Real = Interval{T}(-Inf, Inf)
entireinterval(x::Interval{T}) where T<:Real = entireinterval(T)
entireinterval() = entireinterval(precision(Interval)[1])
entireinterval(::Type{<:Integer}) = entireinterval(Float64)

isentire(x::Interval) = x.lo == -Inf && x.hi == Inf
isinf(x::Interval) = isentire(x)
Expand Down
9 changes: 9 additions & 0 deletions test/decoration_tests/decoration_tests.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,15 @@ let b
b = @decorated 3 4

@test dist(a, b) == 2.0

# invalid input
@test isnai(@decorated(3, 1, com))
@test isnai(@decorated(3, 1))
@test isnai(@decorated(Inf, Inf))
@test isnai(@decorated(-Inf, -Inf))
@test isnai(@decorated(NaN, 3))
@test isnai(@decorated(3, NaN))
@test isnai(@decorated(NaN, NaN))
end

@testset "Convert string to DecoratedInterval" begin
Expand Down
4 changes: 2 additions & 2 deletions test/interval_tests/consistency.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,10 +380,10 @@ setprecision(Interval, Float64)
@test interval(1, 2) == Interval(1, 2)

@test inf(Interval(3, 2)) == 3
@test_throws ArgumentError interval(3, 2)
@test_logs (:warn,) @test isempty(interval(3, 2))

@test sup(Interval(Inf, Inf)) == Inf
@test_throws ArgumentError interval(Inf, Inf)
@test_logs (:warn,) @test isempty(interval(Inf, Inf))

end

Expand Down
47 changes: 24 additions & 23 deletions test/interval_tests/construction.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,20 +69,20 @@ const eeuler = Base.MathConstants.e

# Disallowed conversions with a > b

@test_throws ArgumentError interval(2, 1)
@test_throws ArgumentError interval(big(2), big(1))
@test_throws ArgumentError interval(BigInt(1), 1//10)
@test_throws ArgumentError interval(1, 0.1)
@test_throws ArgumentError interval(big(1), big(0.1))

@test_throws ArgumentError @interval(2, 1)
@test_throws ArgumentError @interval(big(2), big(1))
@test_throws ArgumentError @interval(big(1), 1//10)
@test_throws ArgumentError @interval(1, 0.1)
@test_throws ArgumentError @interval(big(1), big(0.1))
@test_throws ArgumentError interval(Inf)
@test_throws ArgumentError interval(-Inf, -Inf)
@test_throws ArgumentError interval(Inf, Inf)
@test_logs (:warn,) @test isempty(interval(2, 1))
@test_logs (:warn,) @test isempty(interval(big(2), big(1)))
@test_logs (:warn,) @test isempty(interval(BigInt(1), 1//10))
@test_logs (:warn,) @test isempty(interval(1, 0.1))
@test_logs (:warn,) @test isempty(interval(big(1), big(0.1)))

@test_logs (:warn,) @test isempty(@interval(2, 1))
@test_logs (:warn,) @test isempty(@interval(big(2), big(1)))
@test_logs (:warn,) @test isempty(@interval(big(1), 1//10))
@test_logs (:warn,) @test isempty(@interval(1, 0.1))
@test_logs (:warn,) @test isempty(@interval(big(1), big(0.1)))
@test_logs (:warn,) @test isempty(interval(Inf))
@test_logs (:warn,) @test isempty(interval(-Inf, -Inf))
@test_logs (:warn,) @test isempty(interval(Inf, Inf))

# Conversion to Interval without type
@test convert(Interval, 1) == Interval(1.0)
Expand Down Expand Up @@ -237,10 +237,10 @@ end
a = big(0.1)..2
@test typeof(a) == Interval{BigFloat}

@test_throws ArgumentError 2..1
@test_throws ArgumentError π..1
@test_throws ArgumentError π..eeuler
@test_throws ArgumentError 4..π
@test_logs (:warn, ) @test isempty(2..1)
@test_logs (:warn, ) @test isempty(π..1)
@test_logs (:warn, ) @test isempty(π..eeuler)
@test_logs (:warn, ) @test isempty(4..π)
@test 1..π == Interval(1, π)
end

Expand Down Expand Up @@ -292,9 +292,10 @@ end
end

# issue 192:
@testset "Disallow a single NaN in an interval" begin
@test_throws ArgumentError interval(NaN, 2)
@test_throws ArgumentError interval(Inf, NaN)
@testset "Disallow NaN in an interval" begin
@test_logs (:warn, ) @test isempty(interval(NaN, 2))
@test_logs (:warn, ) @test isempty(interval(Inf, NaN))
@test_logs (:warn, ) @test isempty(interval(NaN, NaN))
end

# issue 206:
Expand Down Expand Up @@ -342,7 +343,7 @@ end
end

@testset "a..b with a > b" begin
@test_throws ArgumentError 3..2
@test_logs (:warn,) @test isempty(3..2)
end

@testset "Hashing of Intervals" begin
Expand Down Expand Up @@ -374,7 +375,7 @@ import IntervalArithmetic: force_interval
@test force_interval(4, Inf) == Interval(4, Inf)
@test force_interval(Inf, 4) == Interval(4, Inf)
@test force_interval(Inf, -Inf) == Interval(-Inf, Inf)
@test_throws ArgumentError force_interval(NaN, 3)
@test_logs (:warn,) @test isempty(force_interval(NaN, 3))
end

@testset "Zero interval" begin
Expand Down
8 changes: 4 additions & 4 deletions test/interval_tests/numeric.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ setprecision(Interval, Float64)
y = 4..5
a = 3
b = 12

@test sqrt(sum(x.^2 .+ y.^2)) == 5..13

for i in 1:20
Expand All @@ -24,7 +24,7 @@ setprecision(Interval, Float64)
end

a = 4
b = 5
b = 5
for i in 1:20
@test y.+i == (a+i)..(b+i)
end
Expand Down Expand Up @@ -115,7 +115,7 @@ end
@test Interval(1,2) ^ -3 == Interval(1/8, 1.0)
@test Interval(0,3) ^ -3 == @interval(1/27, Inf)
@test Interval(-1,2) ^ -3 == entireinterval()
@test_throws ArgumentError interval(-1, -2) ^ -3 # wrong way round
@test_logs (:warn, ) @test isempty(interval(-1, -2) ^ -3)
@test Interval(-3,2) ^ (3//1) == Interval(-27, 8)
@test Interval(0.0) ^ 1.1 == Interval(0, 0)
@test Interval(0.0) ^ 0.0 == emptyinterval()
Expand Down Expand Up @@ -432,4 +432,4 @@ end
@test nthroot(Interval{BigFloat}(-27, 27), -3) == Interval{BigFloat}(-Inf, Inf)
@test nthroot(Interval{BigFloat}(-81, -16), -4) == ∅
@test nthroot(Interval{BigFloat}(-81, -16), 1) == Interval{BigFloat}(-81, -16)
end
end