Repository navigation
Fix degenerate ball-ball collision roots - #354
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
|
@martin0359 Hey Martin, can you tell me a little more about this PR? What motivated it? Cheers. |
Thank you for your reply. I am developing a Chinese 8-ball billiard physics engine using WASM. During this process, I used some YOLO-processed event video materials and your engine to align and calibrate, and then find this issue. |
Each root of the contact equation is a time the balls are exactly 2R apart, which happens both when they come together and when they come apart. get_smallest_approaching_root keeps only the earliest real, non-negative root with dot(r, v) < 0, and both ball-ball detectors use it. This replaces the exact-contact special cases, which only covered the degenerate quadratic branch.
63b7aa3 to
40afd0f
Compare
|
Thank you for spotting these issues and proposing a fix. I built on your changes and I'm going to merge. Thanks! |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #354 +/- ##
==========================================
- Coverage 50.34% 50.24% -0.10%
==========================================
Files 166 166
Lines 11461 11489 +28
==========================================
+ Hits 5770 5773 +3
- Misses 5691 5716 +25
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Summary
This fixes degenerate ball-ball collision detection cases where the collision polynomial is not actually quartic. I found this while running a chain2ball-style regression: two balls can have zero relative acceleration, producing a lower-degree distance equation. The 2D detector still passed that equation to
quartic.solve(...), which now raises when the leading coefficient is zero. Older behavior could also turn the degenerate case into a falset = 0collision.The fix keeps the quartic solver contract intact and handles the degenerate polynomial at the collision-detection call sites. When the quartic term is zero, the detector dispatches to the quadratic solver. Exact-contact cases are only reported as
t = 0collisions if the balls are closing; tangent non-closing cases returninf.Changes
quadratic.solve(...)instead ofquartic.solve(...).quadratic.solve(...).t = 0events for exact tangent balls that are stationary relative to each other or moving apart.Additional non-degenerate path check
To make sure the fix does not change ordinary ball-ball detection, I also ran a local randomized check over 500 non-degenerate rolling/sliding 2D cases where
a != 0. For each sample, I compared the patched detector result against the pre-existing quartic path (quartic.solve(a, b, c, d, e)followed byget_real_positive_smallest_root). The maximum observed absolute difference was0.0, so the ordinary quartic path was unchanged in that sample set.I also compared constructed degenerate and near-degenerate 2D cases against analytic roots to check the lower-degree branch directly.
Validation
uv run ruff check pooltool/evolution/event_based/detect/ball_ball.py tests/evolution/event_based/test_ball_ball.pyuv run pytest tests/evolution/event_based/test_ball_ball.py -quv run pytest tests/evolution/event_based -quv run pytest -q