Bucket DFS - #9
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new “bucket DFS” interval root-finding strategy for CCD, integrates it into the CCD method selection, and makes it the new default root-finding method. It also updates the CLI11 dependency version.
Changes:
- Add
interval_root_finder_bucket_DFSplus edge-edge / vertex-face bucket-DFS entry points. - Extend CCD method selection with
BUCKET_DEPTH_FIRST_SEARCHand switch the public CCD API defaults to it. - Bump CLI11 from
2.3.2to2.6.2.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/tight_inclusion/interval_root_finder.hpp | Declares the new bucket-DFS root-finding APIs. |
| src/tight_inclusion/interval_root_finder.cpp | Implements the bucket-DFS traversal and adds explicit template instantiations and wrappers. |
| src/tight_inclusion/ccd.hpp | Adds the new enum value and changes default root-finding method to bucket-DFS. |
| src/tight_inclusion/ccd.cpp | Wires the new method into the CCD dispatch switch. |
| cmake/recipes/cli11.cmake | Updates the CLI11 CPM dependency version. |
| app/main.cpp | Switches the sample query runner to use bucket-DFS. |
💡 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>
|
I do not have permission to merge, @zfergus can you do that for me? |
|
I benchmarked this branch (
False positive counts (16, 471) and false negative counts (0, 0) match, and IPC Toolkit's CCD tests pass unchanged. Full pipeline, five scenesStandalone queries aren't the whole story for a simulator, so I also ran IPC Toolkit's earliest-ToI narrow phase on frame pairs from five scenes. That's
Every scene returns exactly the same step size on both versions, from the same candidate count, so none of this is a difference in what got computed. Cloth-funnel is the one that behaves like the query datasets, and it's also the only one where the queries are expensive: 660 ns per candidate on The regression looks like a fixed cost per queryConverting each of those four to overhead per candidate:
A few nanoseconds each, roughly constant across scenes that differ by 15× in candidate count and 4× in per-query cost. That's the shape of a fixed setup cost, not anything algorithmic. It's invisible when a query costs 660 ns and it's most of the query when a candidate pair is far apart over the whole motion and gets thrown out on the first For what it's worth, the container setup differs by one allocation, which is about the right size:
Overall I think this is a good change. 2.8-3.8× on real query sets and 2.4× on cloth-funnel is a much bigger effect than the 1.06-1.15× the other scenes give up, and the crossover seems to sit somewhere above 200 ns per candidate, since rod-twist at 182 ns is still slightly net negative. |
|
Follow-up on the per-query overhead above. I tried the smallest change I could think of and it removes the regression on every scene, so it seemed worth sharing rather than just describing. The idea is to process the root box before touching the heap. A default-constructed std::map<NumCCD, std::vector<Interval3>> stacks;
- stacks[initial[0].lower].push_back(initial);
+
+ Interval3 current = initial;
+ bool has_current = true;
long iteration_count = 0;
- while (!stacks.empty()) {
- auto bucket = stacks.begin();
- std::vector<Interval3> &stack = bucket->second;
- Interval3 current = std::move(stack.back());
- stack.pop_back();
- if (stack.empty()) {
- stacks.erase(bucket);
+ while (has_current || !stacks.empty()) {
+ if (!has_current) {
+ auto bucket = stacks.begin();
+ std::vector<Interval3> &stack = bucket->second;
+ current = std::move(stack.back());
+ stack.pop_back();
+ if (stack.empty()) {
+ stacks.erase(bucket);
+ }
}
+ has_current = false;
++iteration_count;The rest of the loop body is untouched. Three-way interleaved sweep, same setup as before, 3 pairs per scene and 20 samples each. All three versions return the same step size on every scene:
These are a fresh sweep, so the On the query datasets nothing changes: 1.67 µs/query on the simulation set against your 1.66, and the handcrafted set comes out a bit better at 1254 µs/query against 1352. False positives and false negatives are identical in every case, and IPC Toolkit's CCD tests pass. One thing worth noting: the patch saves 15 to 20 ns per candidate against this PR, which is more than the 4 to 11 ns I measured as the regression. That's why cloth-ball comes back to even instead of staying behind. Happy to open this as a PR against your branch if that's easier than applying it by hand. |
|
Oh, wow. Thanks for the in-depth analysis! In my benchmark I did not prune queries based on tmax as in ipc-toolkit, and turns out that make bucket DFS much less relevant. I'd like to do a bit more profiling based on the ipc-toolkit workload. The no allocation for root strategy is indeed useful, will add it later. |
|
Update PR to reduce allocation by:
I benchmark ipc-toolkit end-to-end pipeline on i5-13600k on linux to compared with bucket DFS before reduced allocation and BFS without root allocation. Each task is pinned to P core and are sampled 20 times.
Overall aside from cloth-funnel other scene are either tie or shows minor improvements. My final verdict is that this PR speedup difficult scenes that require deep tree traversal while keeping equivalent performance otherwise. |
Verification: Test queries in Scalable CCD dataset with no max iter cap, get identical TOI compared to original. only test armadillo and cloth ball scene due to time constraint.
Benchmark: Generate queries from Scalable CCD dataset using Embree BVH and set max iter to 1e6 per PolyFEM default. All times are in ms.
New algorithm:
Observation is that majority of the CPU time is spent on queries with large bisection tree traversal depth. But compared to uv, the final refinement level of t is rather small. So instead of storing intervals in min heap, we make dedicate DFS traversal stack for each t lower bound and stored them in a binary tree (std::map). For each iteration, we always traverse DFS stack with smallest t.
In addition to performance benefits, bucket DFS guarantees better TOI lower bound compared to BFS because it tests intervals in strict chronological order thus achieves deeper refinement under fixed iteration budget.