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

#773 Add concrete intersection between HPolyhedron and any AbstractPolytope #774

Merged
merged 2 commits into from
Oct 12, 2018
Merged

#773 Add concrete intersection between HPolyhedron and any AbstractPolytope #774

merged 2 commits into from
Oct 12, 2018

Conversation

schillic
Copy link
Member

Closes #773.

@schillic schillic requested a review from mforets October 12, 2018 13:22
@schillic
Copy link
Member Author

I also could not resist: The first commit is unrelated.

mforets
mforets reviewed Oct 12, 2018
View reviewed changes
test/unit_Polyhedron.jl
@@ -81,15 +81,24 @@ if test_suite_polyhedra
# -----

# intersection
# TODO these polyhedra are empty. do the tests make any sense?
Copy link
Member

Choose a reason for hiding this comment

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

hmm true, vertices_list(p1) gives [0.0, 0.0], i think it we should rather define another nonempty polyhedron instance for intersection.

mforets
mforets approved these changes Oct 12, 2018
View reviewed changes
Copy link
Member

@mforets mforets left a comment

Choose a reason for hiding this comment

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

lgtm

@mforets
Copy link
Member

mforets commented Oct 12, 2018

I also could not resist: The first commit is unrelated.

well it depends on the given concrete set whether it is easier to calculate its vertices_list or its constraints_list, i don't think there is a unique answer.

@schillic
Copy link
Member Author

schillic commented Oct 12, 2018 •
edited
Loading

well it depends on the given concrete set whether it is easier to calculate its vertices_list or its constraints_list, i don't think there is a unique answer

Right, I created #775 for obtaining the most efficient polytope type.

@schillic schillic merged commit 0790448 into master Oct 12, 2018
@schillic schillic deleted the schillic/773 branch October 12, 2018 13:48
@codecov-io
Copy link

codecov-io commented Oct 12, 2018 •
edited
Loading

Codecov Report

Merging #774 into master will increase coverage by 0.22%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #774      +/-   ##
==========================================
+ Coverage   94.44%   94.66%   +0.22%     
==========================================
  Files          53       53              
  Lines        1835     1837       +2     
==========================================
+ Hits         1733     1739       +6     
+ Misses        102       98       -4
Impacted Files Coverage Δ
src/concrete_intersection.jl 86.13% <100%> (+0.28%) ⬆️
src/Zonotope.jl 100% <0%> (+2.12%) ⬆️
src/convert.jl 90.24% <0%> (+7.31%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4d3b617...db64b6f. Read the comment docs.

@schillic
Copy link
Member Author

Codecov, you are 9 minutes late to the party ⌚

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mforets mforets mforets approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@schillic @mforets @codecov-io