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

Alpha wrapping: re-use and resume functionalities #7805

Merged
merged 76 commits into from
Dec 11, 2023

Conversation

MaelRL
Copy link
Member

@MaelRL MaelRL commented Oct 17, 2023

Summary of Changes

  • AW3: improve manifoldness enforcing heuristics #7629
  • Add benchmarking executables and scripts to generate reports on the state of AW3.
  • Add functionality + example to interrupt the wrapping process, and resume it.
  • Add functionality + example on re-using the 3D triangulation of a previous wrap with new alpha (and offset) values.
  • Introduce zombie mechanism to avoid having to clean the queues when facets are destroyed by a new Steiner point.
  • Use a consistent value in the sorted queue (circumradius of the surface Delaunay ball, and not of the facet).
  • Distinguish between inside/outside cells, and cells that are flagged "inside" to get a manifold result.
  • Add a function + named parameter to purge pockets of outside cells that are disconnected from the outside.
  • Introduce a new queue that is a simple LIFO instead of sorting by circumradius, which speeds things up:
    image
  • Misc general improvements.

TODO:

Release Management

  • Affected package(s): Alpha_wrap_3
  • Issue(s) solved (if any): -
  • Feature/Small Feature (if any): -
  • License and copyright ownership: no change

This combinatorial choice seemed like a good idea, but it can have
nasty cascading effects, adding very large tetrahedra. See this
issue: CGAL#7625

In the end, the only thing we care about is small volumes being added.

I keep the artificial vertex for now, but I am not fully convinced
these should be actually kept too.
In a normal run of the algorithm, we shall never ask the facet status
of a facet that is already outside, but it's better to be complete
and it costs nothing.
…ormal facets

Artificial facets are *not* infinite facets.
Meaning, use the value that we compare against alpha, and not
simply the radius of the smallest circumscribing ball.

This strongly changes the order of the queue and thus thus results
are very different, but still the same (same guarantees, same
element quality, only a little bit more elements, etc.)

Also a massive, ~35% speed-up, that needs to be investigated.
This doesn't bring any speed-up because it was a very fast exit
in push_facet(): the neighbor was necessarily outside (since we
come from it), and we are done.
If one day this becomes annoying because one wishes to call
oracle.add_XXX() multiple times AND it's a significant
runtime burden, we can just add a function add_XXXs()
with a single call of accelerate_distance_queries()
@lrineau lrineau removed the rm only: ready for master For the release team only: that indicates that a PR is about to be merged in 'master' label Nov 29, 2023
@lrineau lrineau added the Not yet approved The feature or pull-request has not yet been approved. label Nov 29, 2023
@github-actions github-actions bot removed the Tested label Nov 29, 2023

This comment was marked as outdated.

@lrineau
Copy link
Member

lrineau commented Nov 29, 2023

Mael asked me not to merge this PR. I tag it with Not yet approved The feature or pull-request has not yet been approved. .

@MaelRL MaelRL added Ready to be tested and removed Not yet approved The feature or pull-request has not yet been approved. labels Nov 29, 2023
@sloriot
Copy link
Member

sloriot commented Dec 7, 2023

Successfully tested in CGAL-6.0-Ic-122

@lrineau lrineau added the rm only: ready for master For the release team only: that indicates that a PR is about to be merged in 'master' label Dec 7, 2023
@github-actions github-actions bot removed the Tested label Dec 11, 2023

This comment was marked as resolved.

@MaelRL MaelRL added the Tested label Dec 11, 2023
@lrineau lrineau merged commit 222fafc into CGAL:master Dec 11, 2023
9 checks passed
@lrineau lrineau removed the rm only: ready for master For the release team only: that indicates that a PR is about to be merged in 'master' label Dec 11, 2023
@lrineau lrineau deleted the AW3-Resume_aw3-GF branch December 11, 2023 13:11
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.

4 participants