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.

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

Interval set type. #277

Merged
merged 14 commits into from
Mar 6, 2018
Merged

Interval set type. #277

merged 14 commits into from
Mar 6, 2018

Conversation

mforets
Copy link
Member

@mforets mforets commented Mar 5, 2018

Closes #270.

src/Interval.jl Outdated
@@ -0,0 +1,148 @@
@require IntervalArithmetic begin

import IntervalArithmetic.AbstractInterval
Copy link
Member

Choose a reason for hiding this comment

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

I know that I proposed this, but since you need more from this package, better remove this import again.

src/Interval.jl Outdated
Support vector in the given direction.
"""
function σ(d::AbstractVector{N}, x::Interval{N, IN})::AbstractVector{N} where {N, IN <: IA.AbstractInterval{N}}
return d[1] > 0 ? [x.dat.hi] : [x.dat.lo]
Copy link
Member

Choose a reason for hiding this comment

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

It might be problematic that you can ask for σ([1., 2., 3.], x).

Copy link
Member Author

@mforets mforets Mar 5, 2018

Choose a reason for hiding this comment

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

do you mean that we don't check dimensions? it's not yet global, but yes, some types have an assertion.

julia> B
LazySets.Ball1{Float64}([0.0, 0.0], 1.0)

julia> σ(rand(4), B) # support vector of dim 4 over a dim 2 set
2-element Array{Float64,1}:
 0.0
 1.0

but

julia> v = VPolygon([rand(2) for i in 1:5])
LazySets.VPolygon{Float64}(Array{Float64,1}[[0.142884, 0.0902304], [0.973041, 0.0771555], [0.973452, 0.282573], [0.685435, 0.738067]])


julia> σ(rand(4), v)
ERROR: DimensionMismatch("dot product arguments have lengths 4 and 2")
Stacktrace:
 [1] dot(::Array{Float64,1}, ::Array{Float64,1}) at ./linalg/blas.jl:311
 [2] σ(::Array{Float64,1}, ::LazySets.VPolygon{Float64}) at /Users/forets/.julia/v0.6/LazySets/src/VPolygon.jl:139

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that is what I meant.

src/Interval.jl Outdated
"""
center(x::Interval)

Return the center of ther interval.
Copy link
Member

Choose a reason for hiding this comment

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

the

src/Interval.jl Outdated

julia> center(x)
0.5
```
Copy link
Member

Choose a reason for hiding this comment

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

Maybe add some computation example x + y * z - w.

src/Interval.jl Outdated

The list of vertices of the interval represented as two one-dimensional vectors.
"""
vertices_list(x::Interval) = [[low(x::Interval)], [high(x::Interval)]]
Copy link
Member

Choose a reason for hiding this comment

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

You can omit the types on the r.h.s.

p = x * y
@test dim(p) == 1
@test σ([1.0], p) == [0.5]
@test σ([-1.0], p) == [-2.0]
Copy link
Member

Choose a reason for hiding this comment

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

Add a test for -, high, low, and vertices_list.

Copy link
Member Author

Choose a reason for hiding this comment

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

done. any other method that we would like to have?

Copy link
Member

Choose a reason for hiding this comment

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

Those are all functions that are defined here 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

i really meant: do we want some other new method for the Interval type? i can think of overapproximate.

Copy link
Member Author

Choose a reason for hiding this comment

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

there is also and .

Copy link
Member

Choose a reason for hiding this comment

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

Ah yes, we definitely want to have overapproximate to be a no-op.

Copy link
Member

Choose a reason for hiding this comment

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

function (x::AbstractVector{N}, I::LazySets.Interval{N, IN}) where ...
    @assert length(x) == dim(I)
    return low(I) <= x[1] <= high(I)
end

Copy link
Member

Choose a reason for hiding this comment

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

function (X::LazySet{N}, I::LazySets.Interval{N, IN}) where ...
    @assert dim(X) == dim(I)
    return low(I) <= σ(X, [-one(N)]) && σ(X, [one(N)]) <= high(I)
end
function (I::LazySets.Interval{N, IN}, X::LazySet{N}) where ...
    @assert dim(X) == dim(I)
    return σ(X, [-one(N)]) <= low(I) && high(I) <= σ(X, [one(N)])
end
function (I1::LazySets.Interval{N, IN}, I2::LazySets.Interval{N, IN}) where ...
    return low(I2) <= low(I1) && high(I2) <= high(l1)
end

(no witness production)

Copy link
Member Author

Choose a reason for hiding this comment

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

that is all good. while we don't extend for witness production i've just called the interval arithmetic backend for these.

@mforets
Copy link
Member Author

mforets commented Mar 5, 2018

actually, for consistency with all other symbolic set types, + should trigger a fresh MinkowskiSum instance. minkowski_sum should behave like my current +. thoughts?

@schillic
Copy link
Member

schillic commented Mar 5, 2018

actually, for consistency with all other symbolic set types, + should trigger a fresh MinkowskiSum instance.

Okay, or make a difference between and +, like for × and *.

@mforets
Copy link
Member Author

mforets commented Mar 5, 2018

.. yes * should be a new CartesianProduct.

then i take back my suggestion and propose that for intervals, + and * are NOT symbolic.

@schillic
Copy link
Member

schillic commented Mar 5, 2018

I am fine with either way.

@test low(r) == -5.5 && high(r) == 4.0

# Minkowski sum

Copy link
Member

Choose a reason for hiding this comment

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

Something missing?

Copy link
Member Author

Choose a reason for hiding this comment

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

#278 and the unit test :)

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

@mforets
Copy link
Member Author

mforets commented Mar 5, 2018

just saw that center(x::Interval) should return a vector (not a scalar), so that Interval works smoothly with other interface methods, eg. an_element(x).

@mforets
Copy link
Member Author

mforets commented Mar 5, 2018

having added the new overapproximate(S::LazySets.Interval{N, IN}, ::Type{Hyperrectangle}) where {N, IN <: IntervalArithmetic.AbstractInterval{N}} i stumble upon some compilation error and i'm clueless.

in a fresh session:

julia> using LazySets # so far so good
INFO: Precompiling module LazySets.
julia> using IntervalArithmetic  # boom
WARNING: Error requiring IntervalArithmetic from LazySets.Approximations:
UndefVarError: Interval not defined
Stacktrace:
 [1] err(::LazySets.Approximations.##6#13, ::Module, ::String) at /Users/forets/.julia/v0.6/Requires/src/require.jl:42
 [2] withpath(::LazySets.Approximations.##5#12, ::String) at /Users/forets/.julia/v0.6/Requires/src/require.jl:32
 [3] (::LazySets.Approximations.##4#11)() at /Users/forets/.julia/v0.6/Requires/src/require.jl:53
 [4] _collect(::Array{Function,1}, ::Base.Generator{Array{Function,1},Requires.##3#4}, ::Base.EltypeUnknown, ::Base.HasShape) at ./array.jl:488
 [5] loadmod(::Symbol) at /Users/forets/.julia/v0.6/Requires/src/require.jl:20
 [6] require(::Symbol) at ./loading.jl:408
 [7] eval(::Module, ::Any) at ./boot.jl:235
 [8] eval_user_input(::Any, ::Base.REPL.REPLBackend) at ./REPL.jl:66
 [9] macro expansion at ./REPL.jl:97 [inlined]
 [10] (::Base.REPL.##1#2{Base.REPL.REPLBackend})() at ./event.jl:73

@schillic
Copy link
Member

schillic commented Mar 5, 2018

Confirmed here. Replacing @require IntervalArithmetic begin [...] end by using IntervalArithmetic solves the error, but we want to use @require.
My guess was that Interval is not exported correctly (the export is only inside the @reexport). But adding export Interval outside did not help.

@schillic
Copy link
Member

schillic commented Mar 5, 2018

The problem is that Requires stores the things to execute by the module name and in Julia LazySets.Approximations comes before LazySets.

You can wrap the Interval.jl inside a new module that comes before Approximations, e.g., module a. But then you always need to write LazySets.a.Interval and you have to spell out IA.

@mforets mforets merged commit 0d74723 into master Mar 6, 2018
@schillic schillic deleted the mforets/270 branch March 6, 2018 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants