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

#1851 - Add type parameter of linear constraints #2044

Merged
merged 19 commits into from
Mar 12, 2020
Merged

#1851 - Add type parameter of linear constraints #2044

merged 19 commits into from
Mar 12, 2020

Conversation

mforets
Copy link
Member

@mforets mforets commented Mar 2, 2020

Closes #1851.
Closes #2035.
Closes #2036.
Closes #1771.
Closes #1802.

@mforets mforets changed the title #1851 - Add type parameter of linear constraint in HPolygon WIP #1851 - Add type parameter of linear constraint in HPolygon Mar 2, 2020
@mforets
Copy link
Member Author

mforets commented Mar 2, 2020

the next test that fails is a conversion: convert(HPolygon, po). to fix this properly, would require doing #2036, because the implementation just passes over that array (as it should). similarly, we may require #2035 for HPolytope and #1771 for HPolyhedron.

@schillic do you have any particular suggestion about this?

@schillic
Copy link
Member

schillic commented Mar 2, 2020

I do not understand the problem. If you have to do all of them in one PR, that is fine with me.

@mforets
Copy link
Member Author

mforets commented Mar 2, 2020

If you have to do all of them in one PR, that is fine with me.

okay. i'll try to make the minimal changes...

I do not understand the problem.

because HPolygon type is now concretely typed, one cannot convert back to an HPolygon with the constraints list of an HPolygonOpt.

julia> p = rand(HPolygon)
HPolygon{Float64,Array{Float64,1}}(HalfSpace{Float64,Array{Float64,1}}[HalfSpace{Float64,Array{Float64,1}}([1.0123623399683432, 0.1830447941142368], 2.8290283339025173), HalfSpace{Float
64,Array{Float64,1}}([0.6808271818788811, 0.9234031649421612], 2.9682105564317505), HalfSpace{Float64,Array{Float64,1}}([0.3046243758979319, 0.6597164293182575], 1.8242398560365105), Ha
lfSpace{Float64,Array{Float64,1}}([-1.6682351717436381, 0.6468752884603611], -0.12037797885224744), HalfSpace{Float64,Array{Float64,1}}([-0.904336674244401, -0.4010639622029236], -0.552
9601287375698), HalfSpace{Float64,Array{Float64,1}}([-0.36130622049394745, -1.1136226665645736], 0.022726211675922647), HalfSpace{Float64,Array{Float64,1}}([0.5523450116922028, -0.87508
97492045417], 1.5552533054118776), HalfSpace{Float64,Array{Float64,1}}([0.3837191570446274, -0.02326329886297751], 1.0427247616868596)])


julia> convert(HPolygonOpt, p)
HPolygonOpt{Float64}(HalfSpace{Float64,VN} where VN<:AbstractArray{Float64,1}[HalfSpace{Float64,Array{Float64,1}}([1.0123623399683432, 0.1830447941142368], 2.8290283339025173), HalfSpac
e{Float64,Array{Float64,1}}([0.6808271818788811, 0.9234031649421612], 2.9682105564317505), HalfSpace{Float64,Array{Float64,1}}([0.3046243758979319, 0.6597164293182575], 1.82423985603651
05), HalfSpace{Float64,Array{Float64,1}}([-1.6682351717436381, 0.6468752884603611], -0.12037797885224744), HalfSpace{Float64,Array{Float64,1}}([-0.904336674244401, -0.4010639622029236],
 -0.5529601287375698), HalfSpace{Float64,Array{Float64,1}}([-0.36130622049394745, -1.1136226665645736], 0.022726211675922647), HalfSpace{Float64,Array{Float64,1}}([0.5523450116922028, -
0.8750897492045417], 1.5552533054118776), HalfSpace{Float64,Array{Float64,1}}([0.3837191570446274, -0.02326329886297751], 1.0427247616868596)], 1)

julia> constraints_list(ans)
8-element Array{HalfSpace{Float64,VN} where VN<:AbstractArray{Float64,1},1}:
 HalfSpace{Float64,Array{Float64,1}}([1.0123623399683432, 0.1830447941142368], 2.8290283339025173)
 HalfSpace{Float64,Array{Float64,1}}([0.6808271818788811, 0.9234031649421612], 2.9682105564317505)
 HalfSpace{Float64,Array{Float64,1}}([0.3046243758979319, 0.6597164293182575], 1.8242398560365105)
 HalfSpace{Float64,Array{Float64,1}}([-1.6682351717436381, 0.6468752884603611], -0.12037797885224744)
 HalfSpace{Float64,Array{Float64,1}}([-0.904336674244401, -0.4010639622029236], -0.5529601287375698)
 HalfSpace{Float64,Array{Float64,1}}([-0.36130622049394745, -1.1136226665645736], 0.022726211675922647)
 HalfSpace{Float64,Array{Float64,1}}([0.5523450116922028, -0.8750897492045417], 1.5552533054118776)
 HalfSpace{Float64,Array{Float64,1}}([0.3837191570446274, -0.02326329886297751], 1.0427247616868596)

julia> HPolygon(ans)
ERROR: MethodError: no method matching HPolygon(::Array{HalfSpace{Float64,VN} where VN<:AbstractArray{Float64,1},1})
Closest candidates are:
  HPolygon() where N<:Real at /home/mforets/.julia/dev/LazySets/src/Sets/HPolygon.jl:78
  HPolygon(::Array{HalfSpace{N<:Real,VN<:AbstractArray{N<:Real,1}},1}; sort_constraints, check_boundedness, prune) where {N<:Real, VN<:AbstractArray{N,1}} at /home/mforets/.julia/dev/La
zySets/src/Sets/HPolygon.jl:52
  HPolygon(::AbstractArray{N<:Real,2}, ::AbstractArray{N<:Real,1}; sort_constraints, check_boundedness, prune) where N<:Real at /home/mforets/.julia/dev/LazySets/src/Sets/HPolygon.jl:86
Stacktrace:
 [1] top-level scope at REPL[5]:1

@mforets
Copy link
Member Author

mforets commented Mar 2, 2020

though if it's only this issue, i think i prefer to comment out those tests for a while, and put them back once the types are ready..

@schillic
Copy link
Member

schillic commented Mar 2, 2020

though if it's only this issue, i think i prefer to comment out those tests for a while, and put them back once the types are ready..

I changed my mind. It is not good to break code. It also ultimately requires more work to comment out the tests.

@mforets
Copy link
Member Author

mforets commented Mar 2, 2020

the following error is similar with conversion, but with HPolytope

julia> H = rand(Hyperrectangle)
Hyperrectangle{Float64,Array{Float64,1},Array{Float64,1}}([-0.5488471214626174, 1.6132145460478071], [0.9929214238936357, 0.8834733482003816])

julia> convert(HPolytope, H)

HPolytope{Float64}(HalfSpace{Float64,VN} where VN<:AbstractArray{Float64,1}[HalfSpace{Float64,LazySets.Arrays.SingleEntryVector{Float64}}([1.0, 0.0], 0.4440743024310183), HalfSpace{Fl
oat64,LazySets.Arrays.SingleEntryVector{Float64}}([0.0, 1.0], 2.496687894248189), HalfSpace{Float64,LazySets.Arrays.SingleEntryVector{Float64}}([-1.0, 0.0], 1.541768545356253), HalfSp
ace{Float64,LazySets.Arrays.SingleEntryVector{Float64}}([0.0, -1.0], -0.7297411978474255)])

julia> HPolygon(constraints_list(ans))
ERROR: MethodError: no method matching HPolygon(::Array{HalfSpace{Float64,VN} where VN<:AbstractArray{Float64,1},1})
Closest candidates are:
  HPolygon() where N<:Real at /home/mforets/.julia/dev/LazySets/src/Sets/HPolygon.jl:78
  HPolygon(::Array{HalfSpace{N<:Real,VN<:AbstractArray{N<:Real,1}},1}; sort_constraints, check_boundedness, prune) where {N<:Real, VN<:AbstractArray{N,1}} at /home/mforets/.julia/dev/
LazySets/src/Sets/HPolygon.jl:52
  HPolygon(::AbstractArray{N<:Real,2}, ::AbstractArray{N<:Real,1}; sort_constraints, check_boundedness, prune) where N<:Real at /home/mforets/.julia/dev/LazySets/src/Sets/HPolygon.jl:
83
Stacktrace:
 [1] top-level scope at REPL[97]:1

@mforets
Copy link
Member Author

mforets commented Mar 2, 2020

I changed my mind. It is not good to break code. It also ultimately requires more work to comment out the tests.

ok, i will then make the changes related to sets with constraints here.. unfortunately it is not only the tests that i pasted above that fail; there are others, because in many places the output of a function can return either HPolygon/HPolytope/HPolyhedron depending on the dimension etc.

@mforets mforets changed the title WIP #1851 - Add type parameter of linear constraint in HPolygon WIP #1851 - Add type parameter of linear constraints Mar 2, 2020
@mforets
Copy link
Member Author

mforets commented Mar 3, 2020

The PR now passes tests locally, it is ready to go from my side if CI doesn't complain!

Most (if not all) of the issues in #1802 are solved, so that issue can also be considered closed, in the sense that concretely typed array containers are now used.

I left some TODOs for my continuation of this work in a follow up PR that will begin to narrow the specialization for the normal direction types, such that they are more predictable and better match those from their input types.

@mforets mforets requested a review from schillic March 3, 2020 12:25
@mforets mforets changed the title WIP #1851 - Add type parameter of linear constraints #1851 - Add type parameter of linear constraints Mar 3, 2020
src/Interfaces/AbstractPolyhedron_functions.jl Outdated Show resolved Hide resolved
src/Sets/HPolygon.jl Outdated Show resolved Hide resolved
src/Sets/HPolygonOpt.jl Outdated Show resolved Hide resolved
src/Sets/HPolyhedron.jl Outdated Show resolved Hide resolved
src/Sets/HPolyhedron.jl Show resolved Hide resolved
src/Sets/HPolyhedron.jl Show resolved Hide resolved
src/Sets/HPolytope.jl Outdated Show resolved Hide resolved
@mforets mforets requested a review from schillic March 4, 2020 11:59
Copy link
Member

@schillic schillic left a comment

Choose a reason for hiding this comment

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

There are still some problems in the documentation build.

@mforets mforets merged commit a0df966 into master Mar 12, 2020
@mforets mforets deleted the mforets/1851 branch March 12, 2020 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants