Skip to content

Divide and Truncate#228

Open
zfergus wants to merge 11 commits intomainfrom
divide-and-truncate
Open

Divide and Truncate#228
zfergus wants to merge 11 commits intomainfrom
divide-and-truncate

Conversation

@zfergus
Copy link
Copy Markdown
Member

@zfergus zfergus commented Apr 24, 2026

Description

This pull request introduces the Planar-DAT (Divide and Truncate) method for trust-region filtering in the OGC solver, providing a direction-aware alternative to isotropic filtering that reduces artificial damping and deadlock in dense-contact scenarios. It adds both C++ and Python bindings for the new planar_filter_step method, comprehensive documentation and usage examples, and includes minor code cleanups and improvements to type signatures.

This is based on the follow-up work by Chen et al. [2026]: https://arxiv.org/abs/2604.15513

Planar-DAT Trust Region Filtering:

  • Added the planar_filter_step method to the TrustRegion class, which implements the Planar-DAT algorithm for direction-aware truncation of vertex displacements during optimization. This method computes a division plane per collision pair and only truncates motion toward the opposing primitive, allowing for larger and less damped steps in dense contact scenarios. [1] [2] [3]
  • Exposed planar_filter_step to Python via pybind11, including detailed docstrings describing its usage and parameters.

Documentation and Tutorials:

  • Added a comprehensive section to the OGC tutorial explaining Planar-DAT, its motivation, algorithmic steps, and how to use it in both C++ and Python. Includes guidance on when to use isotropic vs. planar filtering and code examples for both languages.
  • Added a new BibTeX reference for the DAT paper to the bibliography.

Code Quality and Maintenance:

  • Improved function signatures in pybind11 bindings by passing string arguments as const std::string& instead of by value, reducing unnecessary copies. [1] [2]
  • Removed unused includes from several C++ source files for cleaner code. [1] [2] [3] [4] [5]

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • New unit tests

Checklist

  • I have followed the project style guide
  • My code follows the clang-format style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Introduces Planar-DAT (Divide and Truncate) trust-region filtering to the OGC solver’s trust-region step filtering, adds Python bindings, and documents the method with examples and references.

Changes:

  • Added ipc::ogc::TrustRegion::planar_filter_step() (Planar-DAT) plus supporting helpers.
  • Exposed planar_filter_step to Python and added unit tests covering FV/EE cases.
  • Updated OGC tutorial + bibliography; minor C++ include cleanup and pybind signature copy-avoidance.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
src/ipc/ogc/trust_region.hpp Declares Planar-DAT API and documents behavior (currently has parameter/doc mismatches).
src/ipc/ogc/trust_region.cpp Implements Planar-DAT filtering and refactors isotropic clamping into a helper.
python/src/ogc/trust_region.cpp Adds pybind11 binding + docstring for planar_filter_step (currently documents non-existent args).
tests/src/tests/ogc/test_trust_region.cpp Adds Planar-DAT-focused tests for separating/tangential/approaching FV and EE scenarios.
docs/source/tutorials/ogc.rst Adds a Planar-DAT tutorial section and usage examples (one statement currently misdescribes filter_step radius).
docs/source/references.bib Adds BibTeX entry for Chen et al. (2026).
python/src/collisions/normal/normal_collisions.cpp Avoids string copies in pybind helper signatures.
tests/src/tests/utils.hpp Adds // NOLINT to Matrix Market loader decls.
src/ipc/collisions/normal/normal_collision.hpp Removes unused include.
src/ipc/candidates/vertex_vertex.cpp Removes unused include.
src/ipc/candidates/face_vertex.cpp Removes unused include.
src/ipc/candidates/edge_vertex.cpp Removes unused include.
src/ipc/broad_phase/lbvh.hpp Removes unused include.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/ipc/ogc/trust_region.hpp
Comment thread python/src/ogc/trust_region.cpp Outdated
Comment thread docs/source/tutorials/ogc.rst Outdated
Comment thread src/ipc/ogc/trust_region.hpp Outdated
@zfergus zfergus force-pushed the divide-and-truncate branch from 837311a to 52fcb64 Compare April 24, 2026 15:41
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

❌ Patch coverage is 98.93333% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 95.85%. Comparing base (1db208d) to head (1ba3274).

Files with missing lines Patch % Lines
src/ipc/ogc/trust_region.cpp 96.33% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #228      +/-   ##
==========================================
- Coverage   95.85%   95.85%   -0.01%     
==========================================
  Files         160      160              
  Lines       16664    16638      -26     
  Branches      948      932      -16     
==========================================
- Hits        15974    15948      -26     
  Misses        690      690              
Flag Coverage Δ
unittests 95.85% <98.93%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

zfergus and others added 4 commits April 24, 2026 12:05
Implements `planar_filter_step()` on `TrustRegion` as a direction-aware
replacement for the isotropic `filter_step()`. For each active collision
pair, a division plane is computed from the closest points and approach
speeds; only displacement crossing that plane is truncated, leaving
tangential and separating motion unconstrained. An isotropic fallback
cap of 0.5·γ_r·r_q covers primitives beyond the query radius.

See "Divide and Truncate" [ACM SIGGRAPH 2026].

- src/ipc/ogc/trust_region.hpp: add `planar_filter_step` declaration
- src/ipc/ogc/trust_region.cpp: implement FV/EE loops + isotropic
  fallback
- python/src/ogc/trust_region.cpp: Python binding
- tests/src/tests/ogc/test_trust_region.cpp: 5 new Planar-DAT test cases
- docs/source/tutorials/ogc.rst: Planar-DAT tutorial section
- docs/source/references.bib: add Chen2026DivideAndTruncate entry

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Remove query_radius and relaxation_ratio parameters from
  planar_filter_step
- Use TrustRegion::relaxed_radius_scaling member to control relaxation
  ratio
- Update Python bindings and documentation to match new API
- Move planar_truncation_ratio to private method of TrustRegion
- Add compute_trust_region_beta helper for trust region fallback
- Update tests to reflect fallback trust region behavior for uncovered
  vertices
- Remove unused includes and minor formatting cleanup
- Update documentation to distinguish the isotropic fallback in
  planar_filter_step from the clamp in filter_step
- Remove outdated parameter descriptions for query_radius and
  relaxation_ratio in both C++ and Python docstrings
- Improve explanation of safe-zone clamping for uncovered vertices in
  the OGC tutorial
@zfergus zfergus force-pushed the divide-and-truncate branch from c0a0b25 to cfa632b Compare April 24, 2026 16:05
zfergus and others added 7 commits April 24, 2026 17:57
collisions

- Extend planar_filter_step to handle edge-vertex and vertex-vertex
  cases
- Use VectorMax3d for dimension-agnostic closest-point calculations
- Update planar_truncation_ratio to accept Eigen::ConstRef<VectorMax3d>
- Add comprehensive tests for edge-vertex and vertex-vertex truncation
- Ensure correct handling for 2D and 3D meshes
- Improve comments and clarify approach speed calculations
…tencil interface

Replace the four type-specific loops (FV/EE/EV/VV over NormalCollisions) with a
single unified loop over Candidates using the polymorphic CollisionStencil interface.
This eliminates the stale-distance-type bug where NormalCollisions decomposes FV
pairs into sub-types at update() time; as vertices move during optimization the
stored type goes stale, causing wrong division plane geometry. Using Candidates
directly means FaceVertexCandidate::known_dtype() always returns AUTO and
compute_distance_vector() recomputes the closest-point type from current positions.

The division plane formula is unified to p = c_second + λ·dv for all collision types,
consistent with the old per-type formulas and correct when c_first is a single vertex
(plane at λ=0 placed at c_second rather than at the vertex itself).

The candidates member is stored on TrustRegion and planar_filter_step uses it
directly (no longer takes a separate candidates/collisions argument).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace serial loops in warm_start_time_step, filter_step, and
  planar_filter_step with tbb::parallel_for for per-vertex and
  per-candidate
  operations
- Use std::atomic for thread-safe counters and min-reduction
- No changes to algorithm logic; improves scalability on large meshes

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Replace tbb::blocked_range-based parallel_for calls with simpler
  range-based
  overloads throughout the codebase
- Remove unnecessary #include <tbb/blocked_range.h> from affected files
- Improves code clarity and reduces boilerplate in parallel loops
- Replace tbb::blocked_range-based loops with simpler range-based
  tbb::parallel_for
- Remove redundant tbb_parallel_block_range_for helper in
  spatial_hash.cpp
- Improve readability and consistency in broad phase parallel loops
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants