[FEATURE] Add analytical sphere-sphere narrow-phase contact#2963
Open
Kashu7100 wants to merge 7 commits into
Open
[FEATURE] Add analytical sphere-sphere narrow-phase contact#2963Kashu7100 wants to merge 7 commits into
Kashu7100 wants to merge 7 commits into
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94a77bab8c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Kashu7100
changed the title
Collaborator
|
Nice that you updated tests/test_rigid_physics_analytical_vs_gjk.py 🙌 Skimming the PR, looks plausible to me. (skimmed very quickly, like 30 seconds, to be clear). |
|
🔴 Benchmark Regression Detected ➡️ Report |
Kashu7100
added a commit
to Kashu7100/Genesis
that referenced
this pull request
Jun 17, 2026
Addresses Codex review on Genesis-Embodied-AI#2963: the analytic sphere-sphere path was unreachable under requires_grad=True because the collider guard rejected all smooth/smooth pairs before narrowphase, so the closed-form path could not unblock the differentiable case it was meant to. Make sphere-sphere fully differentiable: - collider guard: exempt sphere-sphere (still rejects sphere-ellipsoid and ellipsoid-ellipsoid, which have no analytic path and tunnel through diff_gjk's EPA). - backward: func_differentiable_sphere_contact reconstructs the contact in closed form from the geom centres and radii, bypassing the Minkowski- triangle reconstruction (degenerate for two smoothly-curved surfaces). - backward kernel: both contact loops dispatch sphere-sphere pairs to the analytic reconstruction. - forward: func_add_diff_contact_input_sphere registers a self-referential diff-contact-input entry so the backward pass processes the analytic contact. Gradients w.r.t. geom positions match finite differences to ~1e-11 (64-bit). Tests: test_diff_smooth_pair_raises now covers a sphere-ellipsoid pair (still unsupported); new test_diff_sphere_sphere_contact checks the exact forward contact and FD-validates the analytic gradients. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Addressed the Codex P2 in 38c9a78 — sphere-sphere is now fully supported under Codex correctly noted the guard made the analytic path unreachable. Just relaxing the guard wasn't enough, though: the differentiable backward (
Gradients w.r.t. geom positions match finite differences to ~1e-11 (64-bit). @codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38c9a78b11
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Collaborator
This table is false advertising. sphere-sphere collision detection is NOT drifting but exact (and there is neither penetration depth nor normal error). MPR is biased, but the post-processing step |
Collaborator
Author
|
that's fair. but the speed ups are real. |
Milotrince
added a commit
to Kashu7100/Genesis
that referenced
this pull request
Jul 7, 2026
Addresses Codex review on Genesis-Embodied-AI#2963: func_differentiable_sphere_contact divided delta by its norm unconditionally, so two differentiable spheres with coincident (or near-coincident) centers produced NaN contact normals and gradients in collider.backward() - even though the forward func_sphere_sphere_contact generates a finite contact with an arbitrary fallback normal there. The distance itself is a non-differentiable cusp at coincident centers: both the normal (delta / dist) and the norm's own gradient (also delta / dist) are 0/0, and reverse-mode turns that into 0 * NaN = NaN. Compute dist_sq without the sqrt and only take the sqrt / divide when the centers are separated (dist_sq > EPS^2); otherwise reuse the same arbitrary direction and constant penetration as the forward path. This threads rigid_global_info (for EPS) into the backward func and its kernel. test_diff_sphere_sphere_contact now also checks that a coincident-center pair yields finite forward contacts and finite backward gradients.
Milotrince
pushed a commit
to Kashu7100/Genesis
that referenced
this pull request
Jul 7, 2026
Addresses Codex review on Genesis-Embodied-AI#2963: the analytic sphere-sphere path was unreachable under requires_grad=True because the collider guard rejected all smooth/smooth pairs before narrowphase, so the closed-form path could not unblock the differentiable case it was meant to. Make sphere-sphere fully differentiable: - collider guard: exempt sphere-sphere (still rejects sphere-ellipsoid and ellipsoid-ellipsoid, which have no analytic path and tunnel through diff_gjk's EPA). - backward: func_differentiable_sphere_contact reconstructs the contact in closed form from the geom centres and radii, bypassing the Minkowski- triangle reconstruction (degenerate for two smoothly-curved surfaces). - backward kernel: both contact loops dispatch sphere-sphere pairs to the analytic reconstruction. - forward: func_add_diff_contact_input_sphere registers a self-referential diff-contact-input entry so the backward pass processes the analytic contact. Gradients w.r.t. geom positions match finite differences to ~1e-11 (64-bit). Tests: test_diff_smooth_pair_raises now covers a sphere-ellipsoid pair (still unsupported); new test_diff_sphere_sphere_contact checks the exact forward contact and FD-validates the analytic gradients. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Milotrince
added a commit
to Kashu7100/Genesis
that referenced
this pull request
Jul 7, 2026
Addresses Codex review on Genesis-Embodied-AI#2963: func_differentiable_sphere_contact divided delta by its norm unconditionally, so two differentiable spheres with coincident (or near-coincident) centers produced NaN contact normals and gradients in collider.backward() - even though the forward func_sphere_sphere_contact generates a finite contact with an arbitrary fallback normal there. The distance itself is a non-differentiable cusp at coincident centers: both the normal (delta / dist) and the norm's own gradient (also delta / dist) are 0/0, and reverse-mode turns that into 0 * NaN = NaN. Compute dist_sq without the sqrt and only take the sqrt / divide when the centers are separated (dist_sq > EPS^2); otherwise reuse the same arbitrary direction and constant penetration as the forward path. This threads rigid_global_info (for EPS) into the backward func and its kernel. test_diff_sphere_sphere_contact now also checks that a coincident-center pair yields finite forward contacts and finite backward gradients.
Milotrince
force-pushed
the
kashu/analytic-sphere-sphere-narrowphase
branch
2 times, most recently
from
July 7, 2026 22:33
8578834 to
d34ed5c
Compare
Milotrince
added
the
ready for review
|
🔴 Benchmark Regression Detected ➡️ Report |
Sphere-sphere pairs previously fell through to the iterative MPR
(and, on a cold cache, GJK+EPA) narrow-phase path. A sphere-sphere
contact is a closed form, so route it through a dedicated analytic
function alongside the existing sphere-capsule / sphere-box /
capsule-capsule specializations.
func_sphere_sphere_contact computes the contact directly from the two
centers and radii:
normal = (c_a - c_b) / ||c_a - c_b|| (B -> A)
penetration = r_a + r_b - ||c_a - c_b||
contact = c_a - (r_a - penetration/2) * normal
with an arbitrary-direction fallback for coincident centers.
Wired into all three narrow-phase dispatch sites (multi-contact loop,
func_narrow_phase_convex_vs_convex, and the split contact0 path).
Benchmark (8192 parallel pairs, float32 GPU, pure detection()):
- analytic: ~96 us/call, penetration error ~7e-9 (float32 eps),
exact normal and contact point
- MPR baseline: ~12408 us/call, penetration error ~6e-6, normal
~0.5 deg off-axis
=> ~128x faster and more accurate for sphere-sphere.
The closed form is also exactly differentiable, unlike the EPA path
which fails to converge on the smoothly-curved sphere-sphere Minkowski
boundary.
Adds test_sphere_sphere_analytical_accuracy.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses Codex review on Genesis-Embodied-AI#2963: the analytic sphere-sphere path was unreachable under requires_grad=True because the collider guard rejected all smooth/smooth pairs before narrowphase, so the closed-form path could not unblock the differentiable case it was meant to. Make sphere-sphere fully differentiable: - collider guard: exempt sphere-sphere (still rejects sphere-ellipsoid and ellipsoid-ellipsoid, which have no analytic path and tunnel through diff_gjk's EPA). - backward: func_differentiable_sphere_contact reconstructs the contact in closed form from the geom centres and radii, bypassing the Minkowski- triangle reconstruction (degenerate for two smoothly-curved surfaces). - backward kernel: both contact loops dispatch sphere-sphere pairs to the analytic reconstruction. - forward: func_add_diff_contact_input_sphere registers a self-referential diff-contact-input entry so the backward pass processes the analytic contact. Gradients w.r.t. geom positions match finite differences to ~1e-11 (64-bit). Tests: test_diff_smooth_pair_raises now covers a sphere-ellipsoid pair (still unsupported); new test_diff_sphere_sphere_contact checks the exact forward contact and FD-validates the analytic gradients. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Addresses Codex review on Genesis-Embodied-AI#2963: func_differentiable_sphere_contact divided delta by its norm unconditionally, so two differentiable spheres with coincident (or near-coincident) centers produced NaN contact normals and gradients in collider.backward() - even though the forward func_sphere_sphere_contact generates a finite contact with an arbitrary fallback normal there. The distance itself is a non-differentiable cusp at coincident centers: both the normal (delta / dist) and the norm's own gradient (also delta / dist) are 0/0, and reverse-mode turns that into 0 * NaN = NaN. Compute dist_sq without the sqrt and only take the sqrt / divide when the centers are separated (dist_sq > EPS^2); otherwise reuse the same arbitrary direction and constant penetration as the forward path. This threads rigid_global_info (for EPS) into the backward func and its kernel. test_diff_sphere_sphere_contact now also checks that a coincident-center pair yields finite forward contacts and finite backward gradients.
The comments and docstring for the analytic sphere-sphere path pointed at func_differentiable_contact (the general GJK reconstruction), but the sphere-sphere backward is reconstructed by func_differentiable_sphere_contact. Correct the references so they name the function that actually runs.
The analytic contact funcs carried numpydoc Parameters sections that only documented the transform args (skipping i_ga/i_gb/geoms_info/...) with self-explanatory descriptions - bloat for internal @qd.func helpers. Fold the one load-bearing detail into the opening sentence instead: the poses are passed in (rather than read from the geom state) so the multi-contact loop can perturb them, and a sphere's orientation is unused.
…pass func_narrow_phase_diff_convex_vs_convex duplicated the zero-init + sphere-vs- Minkowski-triangle branch identically across its reference and non-reference contact loops. Factor it into func_dispatch_differentiable_contact in diff_gjk.py, called once per loop with the loop's own ref_penetration.
Match the established pattern (create_two_free_bodies_mjcf in test_rigid_physics.py): a builder returns the ET.Element, converted to inline XML via ET.tostring() at the call site, rather than writing a file to tmp_path. Drops the now-unused tmp_path fixture argument.
Milotrince
force-pushed
the
kashu/analytic-sphere-sphere-narrowphase
branch
from
July 10, 2026 21:50
d34ed5c to
d86a9b8
Compare
|
|
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
4 participants
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
Sphere-sphere collision pairs currently fall through to the iterative MPR narrow-phase path (and, on a cold contact cache, GJK+EPA as well). A sphere-sphere contact is a closed form, so this PR routes it through a dedicated analytic function — mirroring the existing
sphere-capsule,sphere-box, andcapsule-capsulespecializations.func_sphere_sphere_contact(incollider/capsule_contact.py) computes the contact directly from the two centers and radii:with an arbitrary-direction fallback for coincident centers. It is wired into all three narrow-phase dispatch sites (the multi-contact perturbation loop,
func_narrow_phase_convex_vs_convex, and the splitcontact0path).Motivation
The closed form is also exactly differentiable, unlike the EPA path, which fails to converge on the smoothly-curved sphere-sphere Minkowski boundary (see the
requires_gradguard incollider.py).Benchmark
Two overlapping spheres (r=0.10 / r=0.15, centers 0.18 apart → exact penetration 0.07), 8192 parallel envs, float32 GPU, timing pure
collider.detection()with pinned positions (500 calls after warmup). MPR baseline measured by reverting only the dispatch branch in the same build.detection()call(-1,0,0)→ ~128× faster on the full detection workload and more accurate.
Test
Adds
test_sphere_sphere_analytical_accuracytotests/test_rigid_physics_analytical_vs_gjk.py(passes).🤖 Generated with Claude Code