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

Fix initialisation issues in Mesh_3 #7552

Closed
wants to merge 8 commits into from

Conversation

MaelRL
Copy link
Member

@MaelRL MaelRL commented Jun 26, 2023

Summary of Changes

  • Issue 1 (fbecb18): bad comparison in minimal weight clamping in 2D case, caused protection balls to become hidden.
  • Issue 2 (39c0a9f): robust intersection traits returned no intersection for a ray spawned within a triangle, so the init_c3t3() could not insert initial points.
  • Issue 3 (49bb14b): in the 2D case, the code was exiting too early while adjusting ball sizes before a new point insertion.
  • Issue 4 (1f258bc): after 39c0a9f, initialization did not work anymore if the source is on the domain.

Release Management

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

@MaelRL MaelRL added this to the 5.4.5 milestone Jun 26, 2023
@MaelRL MaelRL requested a review from janetournois June 26, 2023 15:42
@sloriot sloriot added Under Testing Batch_2 Second Batch of PRs under testing labels Jun 28, 2023
@sloriot
Copy link
Member

sloriot commented Jun 30, 2023

Runtime errors in CGAL-6.0-Ic-13 in Generator package (at least)

Built target random_points_on_tetrahedral_mesh_3
terminate called after throwing an instance of 'CGAL::Assertion_exception'
  what():  CGAL ERROR: assertion violation!
Expr: c3t3.triangulation().dimension() >= 2
File: /home/sloriot/CGAL/git/integration/Mesh_3/include/CGAL/make_mesh_3.h
Line: 464
Aborted

@MaelRL
Copy link
Member Author

MaelRL commented Jun 30, 2023

@janetournois @lrineau That's actually the configuration that we were discussing: the source of the ray being shot in the C3T3 initialization is a point exactly on the domain. Beforehand, it would find another intersection; now, source of the ray is returned whatever the ray direction, and the initialization only spawns a single point.

@lrineau

This comment was marked as outdated.

@MaelRL MaelRL force-pushed the Mesh_3-PMD_init_bug_fixes-GF branch from 7002332 to 0d34585 Compare July 12, 2023 08:47
@MaelRL

This comment was marked as outdated.

@MaelRL

This comment was marked as off-topic.

@MaelRL MaelRL changed the title Fix initialisation issues for 2D polyhedral mesh domains Fix initialisation issues in Mesh_3 Jul 18, 2023
@afabri

This comment was marked as off-topic.

@MaelRL

This comment was marked as off-topic.

@lrineau

This comment was marked as off-topic.

@MaelRL

This comment was marked as off-topic.

@MaelRL

This comment was marked as off-topic.

This is a band aid fix, a proper fix is to rework the full initilization
pipeline. This is the purpose of the following pull request:
CGAL#7606
@MaelRL
Copy link
Member Author

MaelRL commented Jul 19, 2023

This PR should be finished now.

Other issues are related to edge protection and will be handled in a different pull request.

@sloriot sloriot added Under Testing and removed Batch_2 Second Batch of PRs under testing labels Jul 19, 2023
@lrineau lrineau self-assigned this Jul 26, 2023
@lrineau lrineau added rm only: ready for master For the release team only: that indicates that a PR is about to be merged in 'master' Tested and removed Under Testing labels Jul 26, 2023
@lrineau
Copy link
Member

lrineau commented Jul 26, 2023

lrineau added a commit that referenced this pull request Jul 26, 2023
lrineau added a commit that referenced this pull request Jul 26, 2023
lrineau added a commit that referenced this pull request Jul 26, 2023
@lrineau lrineau modified the milestones: 5.4.5, 5.5.3, 5.5.4 Jul 28, 2023
@lrineau
Copy link
Member

lrineau commented Jul 28, 2023

This PR was merged into 5.5.x-branch (too late for 5.5.3 though).

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