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

Refactoring polytopes #82

Merged
merged 62 commits into from
Sep 29, 2020
Merged

Refactoring polytopes #82

merged 62 commits into from
Sep 29, 2020

Conversation

elias-tsigaridas
Copy link
Collaborator

I moved CMakeLists.txt in the root directory.
Eliminated the use of init in HPolytope and VPolytope.
Add constructors and destructor in VPolytope

Copy link
Member

@vissarion vissarion left a comment

Choose a reason for hiding this comment

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

Thanks Elias for this PR! I am generally ok with merging after addressing some minor remarks and update the branch such that it passes all the tests.

include/convex_bodies/hpolytope.h Outdated Show resolved Hide resolved
include/convex_bodies/hpolytope.h Outdated Show resolved Hide resolved
include/convex_bodies/hpolytope.h Outdated Show resolved Hide resolved
include/convex_bodies/hpolytope.h Outdated Show resolved Hide resolved
include/convex_bodies/hpolytope.h Outdated Show resolved Hide resolved
include/volume/volume_cooling_balls.hpp Outdated Show resolved Hide resolved
include/volume/volume_cooling_hpoly.hpp Outdated Show resolved Hide resolved
test/generator.cpp Show resolved Hide resolved
test/new_volume_example.cpp Show resolved Hide resolved
test/volume_sob_vpolytope.cpp Outdated Show resolved Hide resolved
Copy link
Member

@vissarion vissarion left a comment

Choose a reason for hiding this comment

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

Thanks for the revision. Any clue on why circleci tests are failing? Also if you merge with current develop branch you will activate other tests (github actions) too.

CMakeLists.txt Outdated Show resolved Hide resolved
Copy link
Member

@vissarion vissarion left a comment

Choose a reason for hiding this comment

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

Thanks for this refactoring @elias-tsigaridas ! I am ok with merging and I have a few comments regarding code cleaning.

include/convex_bodies/vpolyintersectvpoly.h Outdated Show resolved Hide resolved
include/convex_bodies/vpolytope.h Outdated Show resolved Hide resolved
include/convex_bodies/zpolytope.h Show resolved Hide resolved
include/convex_bodies/zpolytope.h Outdated Show resolved Hide resolved
include/convex_bodies/zpolytope.h Outdated Show resolved Hide resolved
include/volume/volume_cooling_balls.hpp Outdated Show resolved Hide resolved
include/volume/volume_cooling_balls.hpp Outdated Show resolved Hide resolved
include/volume/volume_cooling_hpoly.hpp Outdated Show resolved Hide resolved
test/benchmarks_cb.cpp Outdated Show resolved Hide resolved
test/volume_cg_vpolytope.cpp Outdated Show resolved Hide resolved
@vissarion vissarion merged commit 20649bd into GeomScale:develop Sep 29, 2020
vissarion pushed a commit that referenced this pull request Oct 26, 2020
* Eliminating "init" and refactoring of the constructors
and introducing std::make_pair
* Add implementation for the Big3
* Eliminate calls to Hpolytope.init
* Eliminate free_them_all function call
* fix R tests and improve c++ polytope generators
* fix errors in R interface and cross polytope generator
* Polishing according to the review in the PR.
* fix bugs on zonotope destructor
* remove Chebychev ball computation from the constructor
Co-authored-by: TSIGARIDAS Elias <[email protected]>
Co-authored-by: Tolis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants