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

Mesh_3 - edge_min_size may cause hanging #7863

Conversation

janetournois
Copy link
Member

Summary of Changes

When the criterion edge_min_size is set, the graph of features is likely to be invalid or at least inconsistent with the input graph.
Checking its topology does not make any sense and could lead to hanging when trying to compute polyline feature lengths (because it's trying to walk on different curves, jumping from one to the other, etc).

This PR disables the check-features-after-protect step when edge_min_size is used, since we anyway do not expect the feature graph to be exactly matching the one of the input.

Release Management

  • Affected package(s): Mesh_3
  • License and copyright ownership: unchanged

… is set

when minimal_size is set, the graph of features is likely to be invalid

or at least inconsistent with the input graph
so checking its topology does not make any sense and could lead to
- assertions failing in debug mode,
- hanging in release mode
@afabri
Copy link
Member

afabri commented Nov 24, 2023

------------------------------------------------------------------
- Mesh_3/ProgramOutput.execution   of  test_meshing_polyhedral_complex_with_manifold_and_min_size
------------------------------------------------------------------

CGAL::Random()::get_seed() = 1700777375
[WARNING] file C:/CGAL_ROOT/CGAL-6.0-Ic-114/data/meshes/polyhedral_complex_of_spheres/Intersection13.off does not exist or cannot be read (CGAL_DATA_DIR='C:/CGAL_ROOT/CGAL-6.0-Ic-114/data').

here

@lrineau lrineau added the Not yet approved The feature or pull-request has not yet been approved. label Nov 24, 2023
@janetournois
Copy link
Member Author

------------------------------------------------------------------
- Mesh_3/ProgramOutput.execution   of  test_meshing_polyhedral_complex_with_manifold_and_min_size
------------------------------------------------------------------

CGAL::Random()::get_seed() = 1700777375
[WARNING] file C:/CGAL_ROOT/CGAL-6.0-Ic-114/data/meshes/polyhedral_complex_of_spheres/Intersection13.off does not exist or cannot be read (CGAL_DATA_DIR='C:/CGAL_ROOT/CGAL-6.0-Ic-114/data').

here

this warning was caused by another PR (that has been merged since then)

@sloriot
Copy link
Member

sloriot commented Dec 14, 2023

@lrineau you added "not yet approved" before Jane's answer. Is that still the case?

@sloriot sloriot added Under Testing Tests failing and removed Not yet approved The feature or pull-request has not yet been approved. Under Testing labels Dec 14, 2023
@sloriot
Copy link
Member

sloriot commented Dec 14, 2023

The following tests FAILED:
	 37 - execution   of  test_meshing_polylines_only (Subprocess aborted)
	 43 - execution   of  test_meshing_unit_tetrahedron (Subprocess aborted)
	 45 - execution   of  test_meshing_with_default_edge_size (Subprocess aborted)

disabling it may give too bad results
change the arc_length computation when minimal size is used, instead,
by approximating it by the segment length
@lrineau
Copy link
Member

lrineau commented Dec 19, 2023

@lrineau you added "not yet approved" before Jane's answer. Is that still the case?

Yes, I would like to discuss that with Jane.

@lrineau lrineau self-assigned this Dec 19, 2023
@lrineau lrineau added the Not yet approved The feature or pull-request has not yet been approved. label Dec 19, 2023
@lrineau lrineau removed the Not yet approved The feature or pull-request has not yet been approved. label Dec 21, 2023
@lrineau
Copy link
Member

lrineau commented Dec 21, 2023

@lrineau you added "not yet approved" before Jane's answer. Is that still the case?

Yes, I would like to discuss that with Jane.

It is done. The PR is ready to be tested.

@sloriot
Copy link
Member

sloriot commented Dec 22, 2023

Successfully tested in CGAL-6.0-Ic-134

@sloriot sloriot merged commit eb65913 into CGAL:5.6.x-branch Dec 22, 2023
8 checks passed
@sloriot sloriot deleted the Mesh_3-min_size_protecting_balls-jtournois branch December 22, 2023 16:15
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