Update Tight Inclusion to 1.1.0 (bucket DFS root finding) - #248
Open
zfergus wants to merge 3 commits into
Open
Conversation
Tight Inclusion 1.1.0 adds a BUCKET_DEPTH_FIRST_SEARCH root-finding method and makes it the default for edgeEdgeCCD/vertexFaceCCD. * Expose BUCKET_DEPTH_FIRST_SEARCH in the Python CCDRootFindingMethod enum and default ipctk.tight_inclusion.edge_edge_ccd and point_triangle_ccd to it, so the bindings match the C++ default. * Fix the CCD benchmark test case: the dataset SECTIONs inside run_benchmark() were siblings of the CCD-selection SECTIONs, so Catch2 never entered both in a single run and the benchmark loop was dead code. Call run_benchmark() from inside each section instead. * Add the [Belgrod et al. 2023] scenes to the earliest-toi benchmark and report which meshes failed to load when skipping. * Fix stale IPC_TOOLKIT_CCD_BENCHMARK_DIR and IPC_TOOLKIT_CCD_NEW_BENCHMARK_DIR references in the CMake status messages; the cache variables are IPC_TOOLKIT_TESTS_CCD_BENCHMARK_DIR and IPC_TOOLKIT_TESTS_NEW_CCD_BENCHMARK_DIR. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR bumps the Tight-Inclusion dependency to v1.1.0 (introducing BUCKET_DEPTH_FIRST_SEARCH and making it the default root finder) and updates the toolkit’s Python API and benchmarks to stay consistent with the new default behavior.
Changes:
- Update Tight-Inclusion to v1.1.0 and expose/select
BUCKET_DEPTH_FIRST_SEARCHas the default in Python bindings. - Fix CCD benchmark execution structure so dataset
SECTIONs properly nest under the selected CCD implementation. - Expand/clarify benchmark coverage and messages (earliest-toi datasets, CMake status output).
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
cmake/recipes/tight_inclusion.cmake |
Bumps Tight-Inclusion CPM dependency to 1.1.0. |
python/src/ccd/tight_inclusion_ccd.cpp |
Exposes the new root-finding enum value to Python and updates defaults/docs to match the new library default. |
tests/src/tests/ccd/test_ccd_benchmark.cpp |
Calls run_benchmark() inside CCD-selection sections so nested dataset sections execute. |
tests/src/tests/ccd/benchmark_ccd.cpp |
Adds additional benchmark scenes and improves skip messaging to report which meshes failed to load. |
tests/CMakeLists.txt |
Fixes status messages to reference the correct CCD benchmark cache variable names. |
Suppressed comments (1)
tests/CMakeLists.txt:85
- Same issue as above: if the variable is empty,
if(NOT (VAR STREQUAL ""))can evaluate incorrectly due to argument expansion. Quote the variable to ensure the comparison is well-formed.
if(NOT (IPC_TOOLKIT_TESTS_NEW_CCD_BENCHMARK_DIR STREQUAL ""))
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
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
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
Bumps Tight-Inclusion from 1.0.6 to 1.1.0, which adds a third root-finding method,
BUCKET_DEPTH_FIRST_SEARCH— DFS with a dedicated traversal stack per time lower bound — and makes it the default forticcd::edgeEdgeCCD/ticcd::vertexFaceCCD.Because
ipc::TightInclusionCCDnever passesccd_method, this changes the narrow-phase root finder for every C++ query in the library. Comparing the two tags, the default enum value is the only behavioral change:interval_root_finder_BFSitself is byte-identical between 1.0.6 and 1.1.0.Changes
Python bindings —
CCDRootFindingMethodonly boundDEPTH_FIRST_SEARCHandBREADTH_FIRST_SEARCH, and the two free functions hard-coded BFS as their default. Without this,BUCKET_DEPTH_FIRST_SEARCHwould be unnameable from Python, andipctk.tight_inclusion.edge_edge_ccd(...)would silently use a different algorithm thanipctk.TightInclusionCCD(...). Now the enumerator is exposed and bothedge_edge_ccdandpoint_triangle_ccddefault to bucket DFS.CCD benchmark test case — the dataset
SECTIONs insiderun_benchmark()were siblings of the CCD-selectionSECTIONs in the test case body, so Catch2 never entered both in a single run: on runs whereccdwas non-nullcsv_dirswas empty, and on runs where a dataset section was enteredccdwas null and the function returned early at its nullptr guard. The benchmark loop was dead code.run_benchmark()is now called from inside each section so the dataset sections nest properly.Earliest-toi benchmark — added the [Belgrod et al. 2023] scenes (cloth-funnel, armadillo-rollers, n-body-simulation, rod-twist) and made the skip message name the meshes that failed to load, rather than always claiming they are private.
CMake — the
IPC_TOOLKIT_TESTS_CCD_BENCHMARKstatus messages referencedIPC_TOOLKIT_CCD_BENCHMARK_DIR/IPC_TOOLKIT_CCD_NEW_BENCHMARK_DIR, which are defined nowhere; the actual cache variables areIPC_TOOLKIT_TESTS_CCD_BENCHMARK_DIR/IPC_TOOLKIT_TESTS_NEW_CCD_BENCHMARK_DIR. The messages never printed.🤖 Generated with Claude Code