Fix edge-edge parallelism threshold and add regression test#225
Merged
Fix edge-edge parallelism threshold and add regression test#225
Conversation
- Use a relative PARALLEL_THRESHOLD in edge_edge_distance_type to correctly classify nearly-collinear coplanar edges, preventing zero-distance errors - Add defensive guards in NormalPotential gradient/hessian for mollified collisions at d=0 to avoid assertion failures - Add regression test for coplanar near-collinear non-overlapping edges - Minor test cleanups for clarity and consistency - Add CLAUDE.md to .gitignore
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a numerical classification edge case in edge–edge distance type selection (nearly collinear coplanar edges) and adds a regression test to prevent reintroducing the issue, along with a small defensive change in the normal potential implementation.
Changes:
- Adjust edge–edge “parallelism” tolerance to be relative (scaled by
a*c) to correctly route nearly parallel cases to the parallel handler. - Add a regression test covering coplanar near-collinear non-overlapping edges.
- Add defensive early-returns in
NormalPotentialfor mollified collisions atd <= 0, plus minor test cleanup and.gitignoreupdate.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/ipc/distance/distance_type.cpp |
Updates edge–edge parallelism threshold logic to use a relative tolerance. |
src/ipc/potentials/normal_potential.cpp |
Adds guards in gradient/hessian for mollified collisions when d <= 0. |
tests/src/tests/distance/test_distance_type.cpp |
Adds regression test for the nearly-collinear coplanar edge-edge distance type bug. |
tests/src/tests/distance/test_edge_edge.cpp |
Minor test formatting cleanup (if (s == 0) block braces). |
.gitignore |
Ignores CLAUDE.md. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #225 +/- ##
=======================================
Coverage 95.85% 95.85%
=======================================
Files 160 160
Lines 16655 16661 +6
Branches 944 947 +3
=======================================
+ Hits 15964 15970 +6
Misses 691 691
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Check m(x) directly instead of d for mollified collisions - Avoid evaluating barrier derivatives when m <= 0 - Update comments for clarity on product rule and defensive checks - No change to core logic; improves robustness and maintainability - Update test comment to match current edge-edge parallelism threshold logic
- Apply mollifier hessian only when collision is mollified and m <= 0 - Use correct barrier value and mollifier hessian in this case - Ensure distance is computed before mollifier for consistency
zfergus
added a commit
that referenced
this pull request
Apr 24, 2026
* Fix edge-edge parallelism threshold and add regression test - Use a relative PARALLEL_THRESHOLD in edge_edge_distance_type to correctly classify nearly-collinear coplanar edges, preventing zero-distance errors - Add defensive guards in NormalPotential gradient/hessian for mollified collisions at d=0 to avoid assertion failures - Add regression test for coplanar near-collinear non-overlapping edges - Minor test cleanups for clarity and consistency - Add CLAUDE.md to .gitignore * Refactor mollifier guard in NormalPotential gradient and hessian - Check m(x) directly instead of d for mollified collisions - Avoid evaluating barrier derivatives when m <= 0 - Update comments for clarity on product rule and defensive checks - No change to core logic; improves robustness and maintainability - Update test comment to match current edge-edge parallelism threshold logic * Fix mollifier handling in NormalPotential::hessian - Apply mollifier hessian only when collision is mollified and m <= 0 - Use correct barrier value and mollifier hessian in this case - Ensure distance is computed before mollifier for consistency
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.
Description
Type of change