Tetrahedral_remeshing: add parallel execution framework#9508
Draft
IasonManolas wants to merge 124 commits into
Draft
Tetrahedral_remeshing: add parallel execution framework#9508IasonManolas wants to merge 124 commits into
IasonManolas wants to merge 124 commits into
Conversation
This reverts commit f312425.
…g XML exporter, implicit functions, and Python scripts for benchmarking and chart generation.
…dding support for adaptive remeshing, integrating Python metadata generation, and improving dependency management. Introduced a new Python script for generating executable metadata and updated build commands accordingly.
… results output. Removed deprecated performance data macros.
This commit introduces a new header file, benchmark_tetrahedral_remeshing_common.h, which includes common typedefs, helper functions, and utilities for managing benchmark results in JSON format. The file facilitates the integration of performance metrics and run metadata for both uniform and adaptive remeshing benchmarks.
…shing This commit introduces several new Python scripts and a YAML configuration file to facilitate the benchmarking process for tetrahedral remeshing. Key additions include: - `config.yaml`: Configuration for benchmark execution, including result directories and chart generation settings. - `generate_charts.py`: Script to generate performance charts from benchmark results. - `generate_report.py`: Script to create an HTML report summarizing benchmark results. - `generate_run_matrix.py`: Script to generate a run matrix from the benchmark configuration. - `parse_results.py`: Script to parse and validate benchmark results from JSON files. - `run_benchmarks.py`: Main script to execute benchmarks based on the provided configuration and manage results. Additionally, a results schema file (`results_schema.yaml`) is included to define the expected structure of benchmark results.
…meshing This commit introduces new Python scripts and a YAML configuration file to facilitate the comparison of benchmark results for tetrahedral remeshing. Key additions include: - `benchmark_comparison_config.yaml`: Configuration for benchmark comparison, including result directories and chart generation settings. - `compare_benchmark_results.py`: Script to load benchmark results, compare metrics, and generate visualizations for performance analysis. These additions enhance the benchmarking framework by enabling detailed analysis and reporting of performance metrics across different runs.
…ng. This commit deletes `benchmark_config.h` and `concurrent_mesher_config.cfg`, which are no longer needed for the current benchmarking framework.
…irectory structure
This commit introduces two new README files: `README.md` provides an overview of the generic benchmarking pipeline, including features, quickstart instructions, and directory structure. `README_DEEPDIVE.md` offers an in-depth technical explanation of the pipeline's components, implementation details, and best practices for usage. These additions enhance documentation and usability for users and contributors.
… uses my generic atomic operation functions.
…rocessor macro. In the future this will change but will use this approach for now in order to gradually build up the refactored atomic operations
…d_tetrahedral_remeshing
Replaced tabs with spaces
…istic-jtournois' into gsoc2025-Tetra_remeshing_parallel-imanolas
-Using CMAKE_CURRENT_SOURCE_DIR instead of CMAKE_SOURCE_DIR
…esults as the original implementation but currently has some workarounds for code that was not fitting in our current ElementaryOperation interface. Its performance is also currently worse than the original.
…w returning and ElementSource instead of an iterator range. This is used in the edge flip operation to return a vector of vertex pairs instead of an iterator range
- Fixed InternalEdgeFlipOperation to use Finite_edges_iterator correctly - Fixed execute_internal_edge_flip to construct vertex pairs from edge iterator - Resolved static member definition type compatibility issues - Both original and refactored benchmarks now run without crashes - Verified with sphere.mesh test case producing meaningful results
This reverts commit 2c5293aca29ba5950aa1979e0f754958acc2e64b. 3c5293a#
…dded config templates
and indentation
@janetournois @IasonManolas you might want to double check this is expected
between the empty() test and the call to try_pop(), another thread may have popped the last element, leading to an infinite loop
…4bfccec Updated the vertex smoothing operations to utilize a shared context for parameters, simplifying the function signatures and improving code clarity. Removed unnecessary parameters from the smooth function calls and adjusted the internal logic to reference the context directly.
…github.com/IasonManolas/cgal into gsoc2025-Tetra_remeshing_parallel-imanolas
Merges gsoc2025-Tetra_remeshing_parallel-imanolas onto cgal/main. Core changes ------------ - Add ElementaryOperation<> abstract base and two execution policies: ElementaryOperationExecutionSequential and ElementaryOperationExecutionParallel. - Add concrete operation classes: EdgeSplitOperation, EdgeCollapseOperation, InternalEdgeFlipOperation, BoundaryEdgeFlipOperation, and three vertex-smooth specialisations (internal, surface, complex-edge). - Add Elementary_remesher orchestrator that selects the sequential or parallel policy at compile time via CGAL_CONCURRENT_TETRAHEDRAL_REMESHING && CGAL_LINKED_WITH_TBB. - Wire Elementary_remesher into Adaptive_remesher; each operation falls back to the original code path when the corresponding CGAL_TETRAHEDRAL_REMESHING_USE_REFACTORED_* macro is not defined. - Generalise try_lock_and_get_incident_cells in Triangulation_3 to accept any container type (not just std::vector<Cell_handle>). - Expose CGAL_ACTIVATE_CONCURRENT_REMESHING CMake option in examples/CMakeLists.txt and Remeshing_triangulation_3.h. Benchmarks ---------- Adds a Python-based benchmark pipeline (benchmark/Tetrahedral_remeshing/) with YAML configuration, per-run JSON results, and HTML report generation. Includes bear.mesh and sphere.mesh reference meshes. Tests ----- Extends existing tests with parallel-tag variants and a new determinism test (test_tetrahedral_remeshing_determinism.cpp).
…x tooling - smooth_vertices.h: remove debug_helpers namespace (point_to_string, vector_to_string) and debug_log lines - tetrahedral_adaptive_remeshing_impl.h: fix flip() indentation (was incorrectly inside if-block), remove spurious blank line - tetrahedral_remeshing_helpers.h: remove ThreadSafeLogger, dump_edge, dump_edge_pairs, dump_far_points, dump_cells_timestamps, dump_triangulation_cells_timestamps, get_cells_timestamps - benchmark/CMakeLists.txt: consolidate to three targets via foreach loop (original, refactored_sequential, refactored_parallel); remove adaptive and duplicate refactored targets - Remove benchmark_tetrahedral_remeshing.cpp (superseded by original), benchmark_adaptive_tetrahedral_remeshing.cpp (stale parallel path, incompatible interface), benchmark_refactored_tetrahedral_remeshing.cpp (duplicate of refactored_parallel) - Remove bear.mesh and sphere.mesh (moved to Data/data/meshes/) - Remove README_DEEPDIVE.md and config.template.yaml - Remove orphan doc/fig/sizing_same_as_mesh_3.png - Benchmark_creation: fix export_charts key in README example, add --config to comparison command, remove dangling README_DEEPDIVE ref; fix generate_charts.py and generate_report.py to accept results_dir; fix run_benchmarks.py to pass results_dir to chart script - Benchmark_comparison: fix YAML indentation in README, document exclude_from_grouping option; add relative-spread floor to outlier detection to suppress floating-point noise false positives; add exclude_from_grouping support to load_pipeline_results()
Merge EdgeSplitOperation into split_long_edges.h, EdgeCollapseOperation into collapse_short_edges.h, InternalEdgeFlipOperation and BoundaryEdgeFlipOperation into flip_edges.h, and Vertex_smoothing_context plus the three VertexSmooth operation classes into smooth_vertices.h. Each original file is left untouched; the operation class is appended after the existing functions, before the closing namespace braces. The four separate *_operation.h files are deleted. elementary_remesh_impl.h is updated to include the four original headers in place of the deleted ones.
collapse_short_edges.h: remove two is_valid() check blocks introduced in the stale cgal_origin/master ref but removed by Laurent Rineau in bc96ef7 (CGAL-6.2-beta1); add const to collapse() and collapse_edge() edge parameters as required by EdgeCollapseOperation. smooth_vertices.h: apply upstream struct Move refactor from bc96ef7 (moves/neighbors/masses consolidated into std::vector<Move>); use structured bindings and 7-arg sizing_at_midpoint matching the helper signature in bc96ef7.
janetournois
left a comment
janetournois
left a comment
Member
There was a problem hiding this comment.
A few comments on this version
As a general remark : please try to reduce the diff so that the original code of elementary operations keep used, without a git diff, as much as possible
| bool lock_zone(const ElementType& edge, const C3t3& c3t3) const override | ||
| { | ||
| auto& tr = c3t3.triangulation(); | ||
| if(should_skip_edge.contains(edge)) |
Member
There was a problem hiding this comment.
the Edge (={Cell_handle, int, int}) representation of an edge is not unique, it is equivalent to as many representations as the nb of incident cells to the edge.
I would suggest to use ordered pairs of vertices to identify edges and reduce the risk of mismatch
| const Vertex_handle v1 = edge.first->vertex(edge.third); | ||
| if(!(tr.try_lock_vertex(v0) && tr.try_lock_vertex(v1))) | ||
| return false; | ||
| if(!tr.is_vertex(v0) || !tr.is_vertex(v1)) |
Member
There was a problem hiding this comment.
is_vertex(v) checks if the vertex is part of the triangulation.
Would try_lock_vertex(v) have succeeded if is_vertex(v) returns false?
With the compact container recycling process, there is a risk that one of these Vertex_handles belongs to the triangulation, but does not correspond to the vertex you think.
| Cell_handle c_new; | ||
| int li, lj, lk; | ||
| if(tr.tds().is_facet(vh2, vh3, vh0, c_new, li, lj, lk)) | ||
| c3t3.add_to_complex(c_new, (6 - li - lj - lk), surfi); |
Member
There was a problem hiding this comment.
todo : where facets removed from complex, and not reinserted, in flip_on_surface()?
| };//end class Tetrahedral_remeshing_smoother | ||
|
|
||
| template <typename C3t3, typename SizingFunction, typename CellSelector> | ||
| class Vertex_smoothing_context |
Member
There was a problem hiding this comment.
it's a pity to duplicate everything here (and other operations too)
Can you please think of a smaller diff that keeps the original code as much as possible, interleaved with parallelization code, but that the core of the functions, member variables, etc, remain unchanged?
| #endif | ||
|
|
||
| if (input_is_c3t3()) | ||
| backup_far_points(); |
Member
There was a problem hiding this comment.
why is this removed again?
| typename ShortEdgesBimap, | ||
| typename Visitor> | ||
| typename C3t3::Vertex_handle collapse_edge(typename C3t3::Edge& edge, | ||
| typename C3t3::Vertex_handle collapse_edge(const typename C3t3::Edge& edge, |
Member
There was a problem hiding this comment.
does it make sense to add const though the edge will be collapsed?
| bool lock_zone(const ElementType& edge, const C3t3& c3t3) const override | ||
| { | ||
| auto& tr = c3t3.triangulation(); | ||
| if(should_skip_edge.contains(edge)) |
Member
There was a problem hiding this comment.
this is risky
For each edge of the triangulation, there are as many valid Edge=(Cell_handle, int, int) valid representations as the nb of incident cells to the edge.
I think that using an ordered pair of vertices would be more consistently safe.
What do you think?
| const Vertex_handle v1 = edge.first->vertex(edge.third); | ||
| if(!(tr.try_lock_vertex(v0) && tr.try_lock_vertex(v1))) | ||
| return false; | ||
| if(!tr.is_vertex(v0) || !tr.is_vertex(v1)) |
Member
There was a problem hiding this comment.
Could is_vertex(v) return false if try_lock_vertex(v) returns true?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR introduces a parallel execution framework for the Tetrahedral Remeshing package, developed during GSoC 2025.
The main idea is to separate execution policy (sequential vs parallel) from the implementation of each elementary operation (split, collapse, flip, smooth). This mirrors the STL execution policy
design and lays groundwork for extending parallelism to other CGAL packages in the future.
New classes
ElementaryOperation<>— abstract base defining the interface all four operations implement:get_element_source(),lock_zone(),execute_operation().ElementaryOperationExecutionSequential/ElementaryOperationExecutionParallel— the two execution policies.EdgeSplitOperation,EdgeCollapseOperation,InternalEdgeFlipOperation,BoundaryEdgeFlipOperation— concrete operation classes.Vertex_smoothing_context:InternalVertexSmoothOperation,SurfaceVertexSmoothOperation,ComplexEdgeVertexSmoothOperation.Elementary_remesher— orchestrator that selects the policy at compile time viaCGAL_CONCURRENT_TETRAHEDRAL_REMESHING && CGAL_LINKED_WITH_TBB.Integration
Adaptive_remeshernow delegates toElementary_remesherwhen theCGAL_TETRAHEDRAL_REMESHING_USE_REFACTORED_*macros are defined (set automatically whenCGAL_CONCURRENT_TETRAHEDRAL_REMESHINGisdefined). Each operation falls back to the original code path otherwise, so the sequential behaviour is fully preserved for existing users.
Known divergence
The refactored collapse operation intentionally omits on-the-fly re-queuing of newly short edges (present in the original). This simplifies the parallel path. This was discussed with @janetournois laste year (see project wiki page August 4-8). See the following benchmark comparison report in which original, parallel and refactored sequential are compared
benchmark_comparison_report.html .
Supporting changes
Triangulation_3::try_lock_and_get_incident_cellsgeneralised to accept any container type (not juststd::vector<Cell_handle>).CGAL_ACTIVATE_CONCURRENT_REMESHINGCMake option exposed inexamples/CMakeLists.txtandRemeshing_triangulation_3.h.benchmark/Tetrahedral_remeshing/) with YAML-configurable runs, per-run JSON results, and HTML report generation. Includesbear.meshandsphere.meshreference meshes.Test plan
-DCGAL_ACTIVATE_CONCURRENT_REMESHING=ONand runctest -C Releasein the test build directory-DCGAL_ACTIVATE_CONCURRENT_REMESHING=OFFand confirm sequential behaviour is unchanged/path/to/...placeholders inbenchmark/Tetrahedral_remeshing/Benchmark_creation/config.yamlwith your local paths, thencd benchmark/Tetrahedral_remeshing/Benchmark_creation && python run_benchmarks.py --config config.yaml