Phase 1 / Forge / NGS position solver#61
Merged
Conversation
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.
Brief:
briefs/M1.1.7-solver-ngs-position.md— Status CLOSED.Squash title:
feat(forge): add ngs position solver (M1.1.7)Closing notes
Delivered. The NGS position pass (
rigid/position_solver.zig) resorbs contact penetration by correcting poses only, exactly asengine-physics-forge.md§1.7.2 prescribes: normal fixed in world for the whole pass, anchors and lever arms and world inverse inertia re-derived from the current poses at every point of every iteration, clamp on the error going in with asep >= 0gate, first-order linearized orientation step with no absolute guard, true-zero pose-write guards, termination on the first iteration that applies nothing.SolverConfigmoved to its own file and gained the four position fields;preparestores the two body-local surface anchors and the pose-invariant local inverse inertias;BodyManagergained internal pose mutators. The M1.1.6 interim artifact is gone: a box dropped from 2 m now rests one slop under the analytic height instead of keeping its ≈ 7 cm impact penetration, andposition_iterations = 0reproduces the old behaviour exactly, pinned by a dedicated test.Validation. 180/180 in
test-forge-3dacross debug × ReleaseSafe × f32 ×-Dphysics_f64=true; fullzig build testgreen;zig build,zig fmt --check,zig build lintclean. RED-first was observed and reported on both pinnable behaviours (the anchor round-trip, including a discrimination step where the rejected midpoint-anchor alternative fails the assertion; and the tightened M1.1.6 resting test). Both oracles in the position solver's unit tests carry discrimination guards, and the current-rotation inertia oracle reproduces the implementation bit-for-bit.What we do NOT know — the iteration budget. The five-box stack came out of the milestone stable, but the reason it is stable is a measurement, not an account. We know that at
velocity_iterations = 8the stack never settles and that at 16 it stops dead at both precisions and holds through 1800 ticks. We do not know why 16 rather than 12 or 20: there is no first-principles derivation, no model of how the required budget scales with chain depth, patch size or mass ratio. 16 is a measured floor for the worst case this milestone accepts, and it is a global constant standing in for something that should not be global at all. The real answer is the per-island budget Jolt derives (PhysicsSystem.cpp,steps_calculator.GetNumVelocitySteps()), which lands with the island manager at M1.1.8 and demotes this field to a floor. Anyone reading the green stack test should not conclude the subject is closed.Second open thread. The reference friction model — friction solved first with the cone clamped on the previous iteration's λₙ, per-manifold aggregation, twist friction — is NOT implemented, and the three parts were each measured to make things worse when adopted alone (RD-3, with the f32/f64 tables at 600/1200/1800 ticks). It needs a dedicated milestone and must be ported whole. The ≈ 7 cm settling-transient lateral offset that exists even with the position pass disabled is hypothesized to come from the missing twist friction; that remains a hypothesis, and nothing measured here confirms it.
Deviations, all recorded, none silent. RD-1:
resting penetration <= penetration_slopis unreachable as frozen, the slop being the pass's fixed point approached from above; the assertion carries an M1.1.4-form noise margin and the physical content (7 cm resorbed to 5 mm) is intact. RD-2:velocity_iterationsdefault 8 → 16, a config default with the M1.1.8 floor seam recorded. RD-3: the three friction fragments measured and rejected. RD-4: the "reference face carried by B" test did not reach the branch it claimed — a sphere core is a point and exits the manifold generator before the reference/incident selection — corrected to a lying capsule against a box withcount == 2asserted as the coverage witness (also the suite's first 2-point manifold). Two E4 conclusions of mine were wrong and are corrected in place rather than deleted — the "joint convergence deficit" reading, refuted by the monotonic degradation with the NGS budget, and "M1.1.8 sleep would freeze it", refuted by Jolt's 0.03 m/s sleep threshold against a 0.047–0.075 m/s residual.Three spec lines for the KB re-upload at close (Claude.ai produces the file, Guy re-uploads): §1.7.2 the attainable resting bound; §1.7.1 the friction/normal ordering attribution; §1.7.1 the
velocity_iterationsdefault with its per-island rationale.Files touched outside the brief's list: none. Two placements are justified in the Execution log rather than the file list: the pose-mutator coverage is inline in
body_manager.zig(its contract is that file's handle validation) and the acceptance suite imports the harness fromsolver_test.ziginstead of copying it (the normative per-tick cycle must have exactly one definition).Validation checklist
position_solver.zig, E3 cycle step 6 + M1.1.6 tightening +position_iterations = 0seam pin, E4 acceptance suite, E4b scope amendment, E5 closing)velocity_solver.zig, noContactManifoldextension, no barecollide, no hash container on the solver path, nopipeline/import fromrigid/debugANDReleaseSafe, inf32AND-Dphysics_f64=true— 180/180 in each of the four configurationszig build,zig build test,zig build test-forge-3d,zig fmt --checkgreenposition_iterations = 0CLAUDE.mdupdated (§3.4) and committed on the branch — state table, Tags row, four Open-decisions entriesvelocity_solver.SolverConfig, novelocity_iterations = 8insrc/, no "Jolt order" attribution left in codeStatus: CLOSED,Closed: 2026-07-25docs(brief): close M1.1.7Merge and tag are Guy's — this PR does neither.