WP-PG.50: restore full-quality contact streams in the dart detector - #3428
WP-PG.50: restore full-quality contact streams in the dart detector#3428jslee02 wants to merge 11 commits into
Conversation
…ctor Local evidence checkpoint for the 2026-07-31 PLAN-621 re-baseline session; ship/hold is gated on maintainer decisions D9/D10 (task README). - Raise the consolidated detector's solver-facing per-pair clamp from the previous three-contact target to the full ContactManifold capacity (4): a three-point face manifold cannot hold a resting box stack, which broke the issue-3056 S6 pile-sleep criterion after the #3381 consolidation. - Pin the behavior with SolverFacingQueriesCarryFullBoxManifold and RestingBoxStackKeepsFourCornerContacts; update Collision.Options and the stale three-contact comments; refresh the deformable-body design note. - Record the full re-baseline, root-cause chain, WS-G matrices (first complete 8-scene run including both HUM rows), fix trade-off evidence, and decisions D9/D10 in the performance-generalization task docs; refresh PLAN-621 and draft the changelog entry (#PENDING link). Gates: 154/154 C++ tests, lint/check-lint, gazebo test-gz end-to-end, fcl/bullet/ode guard rows bit-identical; raw artifacts archived at ~/dart-wsg-evidence-20260731.
Record the local checkpoint commit state in the tracker (replacing stale 'uncommitted' descriptors), align the session-log #3381 citation with the PR-body phrasing, widen the recorded host clock-swing bound to the measured ~3x, and rewrap the D10 note.
Completes the detector stream-quality bundle (D9/D10 decided 2026-08-01, maintainer-delegated; records in the task README): - Emit a stable two-point contact-line manifold for cylinders lying on a near-parallel box face (clipped to the face slabs and the positive- penetration interval, per-endpoint depths) instead of falling through to convex GJK/EPA, whose single arbitrary support point wandered ~90 mm along the line under 50 um pose steps at every tilt. - Keep shallow crossed-cylinder contacts via the exact capsule-equivalent interior closest-point contact; the convex fallback intermittently reported no contact at ~1 mm penetration, injecting free-fall/impact noise wherever crossed cylinders touched. - Pin the behavior with SideOnFaceEmitsStableEndpointContacts, TiltedSideOnFaceKeepsBothEndpointDepths, and CrossedCylindersKeepShallowContact. With this, the complete bundle measures speed-parity with the audited pre-consolidation stack on the S1 primary fixture (direct interleave 1.009x) while carrying ~20% more real contacts, so criterion 1 holds at ~3.5x; criterion 2 is re-anchored per D10 (the 5-seed matrix proves the mixed-pile all-resting outcome chaotic on every stack; bounded penetration is the universal property). Untouched detectors stay bit-identical; S2/S3 dart hashes are unchanged; S1 dart rows re-baseline. Gates: 154/154 C++ tests, cylinder suite 30/30, lint/check-lint clean. Evidence: task folder 01-baseline-evidence.md 2026-08-01 section; raw artifacts archived at ~/dart-wsg-evidence-20260731.
Closes the criterion-2 loop found by the evidence audit: 60k trend runs showed seed-dependent unbounded creep (seed 101: 0.100 -> 0.137 m at ~2.6 um/step), and final-scene reconstruction identified the creeping body as an upright cylinder standing on its end-cap with an arbitrary spin about its own axis. The aligned analytic cap-patch path demanded exact rotational identity, so spun-but-upright cylinders — physically identical to unspun ones — fell to the convex fallback, whose degenerate rim points cannot support a loaded cap. - Detect a box axis parallel to the cylinder axis, canonicalize the spin and axis permutation away, and reuse the aligned analytic math for the whole class. - Hoist the side-line path ahead of the aligned block (its lateral branch would answer side-lying cylinders with one rocking point) and gate it to genuinely shallow poses so deep overlaps keep the legacy minimal-translation behavior. - Apply the tilted-support effective-radius correction from the review lane (support along the face normal is r*sqrt(1-dot^2), not r), with a separation-boundary pin proving no fabricated contacts. - Pin spin-invariance (spun and unspun upright caps emit identical world contacts) and modernize four stale single-support pins to the line-manifold behavior. Outcome: S6 fully deactivates under defaults — 71/71 resting with max penetration 0 on 60k runs of the canonical seed and the worst creeper; 4/5 seeds sleep within the original 20k window (audit-era stack: 3/5). Direct quiet-host interleave vs the audited pre-consolidation stack: 0.946x (slightly faster) with fuller manifolds. 154/154 C++ tests, cylinder suite 32/32, lint clean. Criterion 1 ~3.7x; criterion 2 MET on original terms; D9/D10 records updated in the task README.
- Pin endpoint distinctness in the modernized line-manifold tests (the two contacts must be distinct supports, not duplicates). - Record the accepted narrow boundaries from the review lane in the evidence doc: the axis-parallel gate's 1e-9 matrix tolerance (~4.5e-5 rad) routing discontinuity, and exact box-edge one-face manifolds (a pre-existing ambiguity class). 154/154 C++ tests, check-lint clean.
- Record the true branch state: the first two commits were pushed to origin on 2026-08-01 outside the session; the later commits are local and push/PR of the remainder stays gated on explicit approval. - Replace 'monotonically declining' with the measured bounded ~1-3.6 mm band and the actual canonical-seed freeze window (36k-38k steps). - Attribute the 60k creep evidence to the intermediate effective-radius build, name its logs, and use the full-run creep span (~0.016 to 0.137 m). - Mark the intermediate cylinder-stability section's superseded criterion-2 claims explicitly; list the effective-radius correction in the final fix inventory; correct the modernized-pin count (four assertions across three test bodies) and the two-file change-set description. Docs-only; check-lint clean.
Fill the changelog entry's PR link and move the task RESUME, packet board, and PLAN-621 next step to the open-PR state (milestone DART 6.20.0).
![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: 58955b05ba
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| contact.position | ||
| = boxTransform | ||
| * (axisPoint - normalLocal * (effectiveRadius - 0.5 * depth)); |
There was a problem hiding this comment.
When a side-on-box cylinder has any permitted nonzero tilt (0 < |axisNormalDot| <= 0.05), effectiveRadius is only the support distance along the face normal; multiplying that normal by it is not a radial offset from the cylinder axis. For example, with radius 0.05 and dot 0.04, an endpoint is displaced by roughly 2 mm along the cylinder axis, placing the shallow-contact midpoint near the wrong side of the cap boundary and giving the solver incorrect lever arms. Project the face normal onto the plane perpendicular to the cylinder axis and normalize it when constructing the generator endpoints and contact midpoint.
AGENTS.md reference: AGENTS.md:L54-L56
Useful? React with 👍 / 👎.
| const Eigen::Vector3d lateralDir = dist > kContactEpsilon | ||
| ? (delta / dist).eval() | ||
| : chooseRadialDirection(axis1); |
There was a problem hiding this comment.
When crossed cylinder axes intersect exactly, or within kContactEpsilon, this fallback only guarantees that the chosen direction is perpendicular to axis1. For perpendicular Y/X axes, chooseRadialDirection(axis1) can return X, which is parallel to axis2; the emitted normal is therefore not a valid side normal for the second cylinder, and applying the reported r1 + r2 correction merely slides it along its own axis rather than separating the overlap. Use a deterministic direction based on the normalized axes' cross product for this degenerate-distance case.
AGENTS.md reference: AGENTS.md:L54-L56
Useful? React with 👍 / 👎.
The gcc/clang (newest) lanes fail MemoryDiagnostics.DenseMapStaysBelowTheOpenGL2DrawIndexLimit identically on the release-6.20 tip itself (introduced by the #3379 demos merge; ImGui synthetic draw-list test reporting zero draw lists; the tip's own CI Toolchain run shows both jobs failing while the run reports success, i.e. non-required lanes). Recorded in RESUME so the PR is not blocked on an unrelated pre-existing failure.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4be23b49b3
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (distance > closestFaceDistance) { | ||
| closestFaceDistance = distance; | ||
| faceAxis = axisIndex; | ||
| faceSign = sign; | ||
| } |
There was a problem hiding this comment.
When the cylinder overhangs a short box along its axis by more than its radius, this loop selects the box end face because its center clearance is greatest; the subsequent axis-normal check rejects that face, even though the cylinder still has a valid clipped side-contact line on another face. For example, a radius-0.05, half-height-0.1 cylinder with its axis along Y and center at Y=0.11 overlaps a box spanning Y=[-0.05, 0.05], but the 0.06 end-face clearance outranks a 0.049 top-face clearance, so the code falls through to the old single-point aligned path instead of emitting the intended two endpoint supports. Filter for near-parallel candidate faces before choosing the closest one.
AGENTS.md reference: AGENTS.md:L54-L56
Useful? React with 👍 / 👎.
| if (std::abs(std::abs(boxRotInCyl(2, k)) - 1.0) <= kAxisParallelTolerance) { | ||
| alignedBoxAxis = k; |
There was a problem hiding this comment.
For shallow cap contacts with a small real tilt, comparing 1 - |dot| to 1e-9 accepts angles up to roughly sqrt(2e-9) = 4.5e-5 radians and then erases that tilt. A radius-0.5 cylinder on a face tilted by 4e-5 radians has about 20 µm of depth variation across its cap, so at micrometer-scale penetration this path can emit a full uniform patch containing separated points instead of using the tilted convex geometry. Compare the axes' cross-product norm against the intended angular tolerance (or square the tolerance used with the dot difference).
AGENTS.md reference: AGENTS.md:L54-L56
Useful? React with 👍 / 👎.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## release-6.20 #3428 +/- ##
================================================
- Coverage 75.42% 75.39% -0.04%
================================================
Files 449 449
Lines 53705 53874 +169
================================================
+ Hits 40507 40618 +111
- Misses 13198 13256 +58
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
The pre-existing toolchain-newest failure was fixed upstream by #3429 and the maintainer base-merged the branch; the full hosted matrix concluded green on merge head 4be23b4 (22 pass + 1 skip of 23 checks, zero failures). Remaining: review and merge, then task-folder retirement in the completing PR.
1 participant
Summary
Restores full-quality contact streams in the DART-owned
dartcollisiondetector and, with them, the issue-#3056 pile-sleep outcome. Four measured
stream defects are fixed as one cohesive bundle:
kSolverFacingManifoldContactTarget3 → 4): every ordinary detector query truncated face manifolds to three
points. A three-point tripod cannot hold a resting face-face box stack —
the support polygon stops containing the weight vector under micro-tilts
toward the missing corner, so stacked boxes rock forever. Only the legacy
DARTCollideadapter (explicit unlimited request) received fullmanifolds, which is how the four-contact compatibility test passed while
the solver stream lost its fourth support point.
face fell through to the convex GJK/EPA fallback, which returns one
arbitrary support point on the under-constrained contact line — measured
contact-point teleports of ~90 mm under 50 µm pose steps, at every tilt.
A dedicated path now emits a stable two-point line manifold (segment
clipped to the face slabs and the positive-penetration interval,
per-endpoint depths, ~3° face-parallel gate, shallow-pose gate so deep
overlaps keep the legacy minimal-translation behavior).
reported no contact for crossed cylinders at ~1 mm penetration (3 of 41
sweep poses), so supports vanished for single steps and re-impacted.
While both axis closest points are interior, the swept-circle side
surfaces match capsules exactly, so the closest-point contact is now
computed analytically.
analytic cylinder-box path demanded exact rotational identity, so an
upright cylinder that settled with an arbitrary spin about its own axis
— physically identical to an unspun one — fell to the convex fallback
and crept through its cap support without bound (measured: seed-101 S6
penetration rising ~0.016 → 0.137 m across a 60k-step run; the creeping
body was identified from a final-scene dump as an upright cylinder with
a pure-z spin quaternion, sunk exactly the reported penetration). The
aligned path now detects a box axis parallel to the cylinder axis and
canonicalizes the spin and axis permutation away, reusing the aligned
math for the whole class. A tilted-support effective-radius correction
(
r*sqrt(1-dot^2), from review) rides along with a separation-boundarytest proving no fabricated contacts.
FCL remains the built-in default; FCL/Bullet/ODE implementations are
untouched.
Outcome
71-body mixed-shape container pile fully deactivates under default
settings — 71/71 resting with max penetration 0 on 60 000-step runs of
both the canonical seed and the previously worst-creeping seed; 4 of 5
tested seeds fully deactivate within the original 20 000-step window
(the pre-consolidation stack managed 3 of 5 on the same matrix);
the canonical seed freezes between 36k–38k steps with penetration in a
bounded ~1–3.6 mm band and no growth trend before the freeze.
interleaved A/B against the audited pre-consolidation stack on
S1 (120-object active container,
dart, 1 thread) measures0.946x — slightly faster — while reporting fuller manifolds
(290 vs 242 contacts). Chained to the audited 3.51x, criterion 1 sits
at ≈3.7x of the round-2 baseline.
(S4 900-object: 0.331 → 0.083 ms/step; S5 90-object: 0.024 → 0.0058;
both already fully resting, the delta is settling speed).
DART-vs-MuJoCo matrix (0.72x → 1.27x, MuJoCo's own number stable
across runs).
Determinism and compatibility boundary
PRIMITIVE, unchangeddartdetector contact profileINTEGRATION_entity_system)dart-detector guard rows re-baselined by this PR (recorded indocs/dev_tasks/dart6_performance_generalization/01-baseline-evidence.mdwith the full old→new lineage):
0xd6736cd716faf01d0xfc20c4880fdbca050x1e227311a3f7188e0x6dab35ce2618d4220x266da31836a314a6/0x6088ea0177efa6a0x55bf77ebc1c491b2/0x4f265a803b596035(900/900, 90/90 resting)0x70bf5dd9e4f15051/0xd8de4ae15996321f(900/900, 90/90 resting)Performance report
58955b05baf; base:origin/release-6.20@718651d0d6e; audited reference stack:db255a08e8e(the 2026-07-10completion-audit head, rebuilt in a scratch worktree; its S1/S6 runs
reproduce the audited hashes
0x123ee9779bccacfb/0xec80f734df6d5e74bit-exactly, validating the comparison frame).clock swings (up to ~3x between blocks of the same binary), so all
timing comparisons are same-host interleaved ABAB A/Bs whose per-arm
hashes are checked; hashes/contacts/resting counts are the guards, not
RTF cells. An initially-apparent 1.6–6x regression against July
step-time cells was refuted this way as a host clock-state artifact.
scripts, and every raw log/JSON are archived (path recorded in the task
docs) and summarized in
01-baseline-evidence.md.S1 120/dart/1 interleave (7 ABAB pairs, quiet host, per-arm hashes
stable):
Ratio 0.946x (PR faster). Intermediate states for the record: the
manifold-only state measured 1.95x vs the interim 3-contact era and the
cylinder-stability state 1.21x; the full bundle recovers it all — the
interim 3-contact speed was performance bought with lost contacts.
S6 seed matrix (20 000 steps,
--container-seed, default settings):The audited stack's 3/5 shows the single-seed all-resting outcome was
never a robust property; this PR makes 4/5 sleep inside the window and
all tested seeds sleep by 60k with zero penetration. Non-equivalent
dartrows (contact-profile re-baseline) are reported as re-baselines,not counted as speedups; equivalence-scoped timing claims come only from
the interleaved A/Bs above.
Stream-quality micro-benchmarks (constructed states swept in 50 µm
steps, old vs new dartpy builds; probe scripts archived): cylinder
side-on-face max contact-point motion per step 90 mm → 0.05 mm (exactly
the commanded motion) at every tilt 0–1e-2 rad; crossed-cylinder misses
3/41 poses → 0; box-on-cylinder single wobbly point → two stable
points; box-face tilt sweeps: the old engine showed 1 mm point jumps
with 180° normal flips that the consolidated engine does not have.
Testing
regressions:
SideOnFaceEmitsStableEndpointContacts,TiltedSideOnFaceKeepsBothEndpointDepths,TiltedSeparationIsNotFabricated,CrossedCylindersKeepShallowContact,SpunUprightCapOnFaceUsesStablePatch; plusSolverFacingQueriesCarryFullBoxManifoldandRestingBoxStackKeepsFourCornerContactsat the engine level).modernized to the two-point line-manifold behavior with endpoint
distinctness checks.
pixi run lint/pixi run check-lintclean;DART_PARALLEL_JOBS=8 pixi run -e gazebo test-gzpassed end-to-end onthe final tree.
semantically inspected +
image-verdictnon-blank pass) recorded inthe task evidence.
Breaking changes
dartdetector's contact stream changes: full four-contactface manifolds, two-point cylinder side lines, analytically retained
shallow crossed-cylinder contacts, and spin-invariant cap patches. Its
guard hashes re-baseline as tabled above. The built-in default (
fcl)and the FCL/Bullet/ODE detectors are unchanged.
Known accepted boundaries (recorded in the evidence doc)
routing boundary between two previously-existing behaviors.
one-face line manifold (pre-existing ambiguity class).
latch latency for large single-island piles near true stillness,
rolling friction for roller-heavy scenes, and parallel-line
single-point contacts (cyl-cyl/capsule pairs).
Related work
Continues issue #3056 (performance-generalization round 2) on top of
#3381 (detector consolidation), #3353 (D7 dense-island sleep policy), and
the WS-G cross-engine harness (#3367, #3369). Decision records D9/D10 and
the full evidence narrative live in
docs/dev_tasks/dart6_performance_generalization/.