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

check convexity in line segment #3007

Merged
merged 5 commits into from
Jul 15, 2022
Merged

check convexity in line segment #3007

merged 5 commits into from
Jul 15, 2022

Conversation

mforets
Copy link
Member

@mforets mforets commented Jul 15, 2022

No description provided.

@mforets mforets requested a review from lucaferranti July 15, 2022 15:00
@lucaferranti
Copy link
Member

@schillic (continuing from the previous comment) what is the ambiguity?

@schillic
Copy link
Member

You can open the log in the build.

@lucaferranti
Copy link
Member

julia> Aqua.test_ambiguities(LazySets)
Skipping LazySets.Assertions.are_assertions_enabled
4 ambiguities found
Ambiguity #1
issubset(X::LazySets.ConvexSet, P::LazySets.ConvexSet, witness::Bool) in LazySets at /home/lferrant/.julia/dev/LazySets/src/ConcreteOperations/issubset.jl:71
issubset(L::LazySets.LineSegment{N, VN} where VN<:AbstractVector{N}, S::ST, witness::Bool) where {N, ST<:LazySets.LazySet{N}} in LazySets at /home/lferrant/.julia/dev/LazySets/src/ConcreteOperations/issubset.jl:492

Possible fix, define
  issubset(::LazySets.LineSegment{N, VN} where VN<:AbstractVector{N}, ::ST, ::Bool) where {N, ST<:LazySets.ConvexSet{N}}

Ambiguity #2
issubset(P::LazySets.AbstractPolytope, S::LazySets.ConvexSet) in LazySets at /home/lferrant/.julia/dev/LazySets/src/ConcreteOperations/issubset.jl:213
issubset(L::LazySets.LineSegment{N, VN} where VN<:AbstractVector{N}, S::ST) where {N, ST<:LazySets.LazySet{N}} in LazySets at /home/lferrant/.julia/dev/LazySets/src/ConcreteOperations/issubset.jl:492

Possible fix, define
  issubset(::LazySets.LineSegment{N, VN} where VN<:AbstractVector{N}, ::ST) where {N, ST<:LazySets.ConvexSet{N}}

Ambiguity #3
issubset(P::LazySets.AbstractPolytope, S::LazySets.ConvexSet, witness::Bool; algorithm) in LazySets at /home/lferrant/.julia/dev/LazySets/src/ConcreteOperations/issubset.jl:213
issubset(L::LazySets.LineSegment{N, VN} where VN<:AbstractVector{N}, S::ST, witness::Bool) where {N, ST<:LazySets.LazySet{N}} in LazySets at /home/lferrant/.julia/dev/LazySets/src/ConcreteOperations/issubset.jl:492

Possible fix, define
  issubset(::LazySets.LineSegment{N, VN} where VN<:AbstractVector{N}, ::ST, ::Bool) where {N, ST<:LazySets.ConvexSet{N}}

Ambiguity #4
issubset(X::LazySets.ConvexSet, P::LazySets.ConvexSet) in LazySets at /home/lferrant/.julia/dev/LazySets/src/ConcreteOperations/issubset.jl:71
issubset(L::LazySets.LineSegment{N, VN} where VN<:AbstractVector{N}, S::ST) where {N, ST<:LazySets.LazySet{N}} in LazySets at /home/lferrant/.julia/dev/LazySets/src/ConcreteOperations/issubset.jl:492

Possible fix, define
  issubset(::LazySets.LineSegment{N, VN} where VN<:AbstractVector{N}, ::ST) where {N, ST<:LazySets.ConvexSet{N}}

Test Failed at /home/lferrant/.julia/packages/Aqua/HWLbM/src/ambiguities.jl:117
  Expression: success(pipeline(cmd; stdout = stdout, stderr = stderr))
ERROR: There was an error during testing

@lucaferranti
Copy link
Member

a @test_throw for the fallback could be nice to have, otherwise LGTM

mforets and others added 2 commits July 15, 2022 15:21
Copy link
Member

@lucaferranti lucaferranti left a comment

Choose a reason for hiding this comment

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

(not sure if my review counts anymore given I also committed, but...) LGTM, a couple of things that could be added

  1. A test_throw for the fallback case (arguably quite pedantic)
  2. A test for the case witness=true (falls out of the original scope of the PR, but since the change was incorporated)

@mforets mforets merged commit 5ab93bc into lf-lazyToConvex Jul 15, 2022
@mforets
Copy link
Member Author

mforets commented Jul 15, 2022

a couple of things that could be added

i opened #3008

@mforets mforets deleted the mforets/1856 branch July 15, 2022 20:33
schillic added a commit that referenced this pull request Aug 3, 2022
* check convexity in line segment

* Update docs/src/lib/binary_functions.md

* fix ambiguity

* Update src/ConcreteOperations/issubset.jl

Co-authored-by: Christian Schilling <[email protected]>

* Update src/ConcreteOperations/issubset.jl

Co-authored-by: Luca Ferranti <[email protected]>

Co-authored-by: Christian Schilling <[email protected]>
Co-authored-by: Luca Ferranti <[email protected]>
schillic added a commit that referenced this pull request Aug 3, 2022
* change LazySet to ConvexSet

* rename file LazySet.jl -> ConvexSet.jl

* rename another LazySet.jl to ConvexSet.jl

* add abstract LazySet type

* update docs

* update docstring

* check convexity in line segment (#3007)

* check convexity in line segment

* Update docs/src/lib/binary_functions.md

* fix ambiguity

* Update src/ConcreteOperations/issubset.jl

Co-authored-by: Christian Schilling <[email protected]>

* Update src/ConcreteOperations/issubset.jl

Co-authored-by: Luca Ferranti <[email protected]>

Co-authored-by: Christian Schilling <[email protected]>
Co-authored-by: Luca Ferranti <[email protected]>

Co-authored-by: Marcelo Forets <[email protected]>
Co-authored-by: Christian Schilling <[email protected]>
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.

3 participants