JuliaReach / LazySets.jl

Scalable symbolic-numeric set computations in Julia
https://juliareach.github.io/LazySets.jl/
Other
227 stars 32 forks source link

`VPolygon(M::SMatrix)` errors #3631

Closed ederag closed 1 month ago

ederag commented 1 month ago

VPolygon(M) fails if M is an SMatrix:

julia> using StaticArrays

julia> M = @SMatrix [0 1 0; 0 0 1.]
2×3 SMatrix{2, 3, Float64, 6} with indices SOneTo(2)×SOneTo(3):
 0.0  1.0  0.0
 0.0  0.0  1.0

julia> P = VPolygon(M)
ERROR: MethodError: no method matching VPolygon(::SizedVector{3, SVector{2, Float64}, Vector{SVector{2, Float64}}}; apply_convex_hull::Bool, algorithm::String)

Closest candidates are:
  VPolygon() got unsupported keyword arguments "apply_convex_hull", "algorithm"
   @ LazySets ~/.julia/packages/LazySets/ZmGxF/src/Sets/VPolygon.jl:84
  VPolygon(::Vector{VN}; apply_convex_hull, algorithm) where {N, VN<:AbstractVector{N}}
   @ LazySets ~/.julia/packages/LazySets/ZmGxF/src/Sets/VPolygon.jl:68
  VPolygon(::MT; apply_convex_hull, algorithm) where MT<:(AbstractMatrix)
   @ LazySets ~/.julia/packages/LazySets/ZmGxF/src/Sets/VPolygon.jl:87

Stacktrace:
 [1] VPolygon(vertices_matrix::SMatrix{2, 3, Float64, 6}; apply_convex_hull::Bool, algorithm::String)
   @ LazySets ~/.julia/packages/LazySets/ZmGxF/src/Sets/VPolygon.jl:93
 [2] VPolygon(vertices_matrix::SMatrix{2, 3, Float64, 6})
   @ LazySets ~/.julia/packages/LazySets/ZmGxF/src/Sets/VPolygon.jl:87
 [3] top-level scope
   @ REPL[16]:1

This is breaking OpticSim.jl tests: https://github.com/brianguenter/OpticSim.jl/actions/runs/10272550301/job/28442096437?pr=34 Is this a bug in LazySets or a breaking change OpticSim has to adapt to ?

schillic commented 1 month ago

The problem is that the VPolygon type assumes that the outer container is a Vector. The matrix constructor is just a convenience constructor. An SMatrix does not work because the code below does not create a Vector but a SizedVector.

https://github.com/JuliaReach/LazySets.jl/blob/623a947d25c69970a141e1c028b77304a9ef4d23/src/Sets/VPolygon/VPolygon.jl#L78

I added an implementation for SMatrix in #3632, but it may take time until this gets merged and released.

My suggestion is to not use the matrix constructor, at least not for SMatrix, and rather manually convert the matrix to a Vector and then pass that to VPolygon:

julia> M = [@SVector[0, 0.], @SVector[1, 0.], @SVector[0, 1.]]
3-element Vector{SVector{2, Float64}}:
 [0.0, 0.0]
 [1.0, 0.0]
 [0.0, 1.0]

julia> P = VPolygon(M)
VPolygon{Float64, SVector{2, Float64}}(SVector{2, Float64}[[0.0, 0.0], [1.0, 0.0], [0.0, 1.0]])
ederag commented 1 month ago

Thanks for the PR and the advice, that indeed allowed to work around this issue.

schillic commented 1 month ago

@ederag The extension has now been released.