Add SIM_verif_attach_mass RUN_09 (non-identity root orientation) (18→19/21) (#99)#631
Open
simnaut wants to merge 3 commits into
Open
Add SIM_verif_attach_mass RUN_09 (non-identity root orientation) (18→19/21) (#99)#631simnaut wants to merge 3 commits into
simnaut wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Adds JEOD cross-validation coverage for SIM_verif_attach_mass RUN_09, the first attach-mass scenario in this suite where the root parent has a non-identity struct→body orientation, and updates the reference-generation script and committed JEOD mass.out baseline accordingly.
Changes:
- Add RUN_09 to
generate_references.shso its JEODmass.outbaseline can be regenerated alongside existing attach-mass runs. - Extend
tier3_sim_attach_masswithbuild_run_09,parent_option_a(), and acheck_body_rotatedhelper that rotates only the composite inertia into the body frame for comparison. - Commit new reference output
attach_mass_09_mass.out.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| trick/generate_references.sh | Adds RUN_09 to the SIM_verif_attach_mass reference generation group. |
| crates/astrodyn_verif_jeod/tests/tier3_sim_attach_mass.rs | Adds RUN_09 test construction and a rotated-inertia comparison path to match JEOD’s mixed-frame print convention. |
| crates/astrodyn_verif_jeod/test_data/attach_mass_09_mass.out | Adds JEOD baseline output for RUN_09 mass-tree print validation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…19/21) (#99) RUN_09 = parent (StructCG inertia + non-identity parent_mass_orientation_optionA struct→body) + child1 (Body), offset attach [-1,0,0]. It's the first attach_mass RUN with a non-identity root struct→body orientation. Key finding — JEOD's mass print uses a MIXED frame convention for the composite of an oriented body: the composite CoM (`C.M.P. CM vector`) stays in the STRUCT frame, but the composite inertia (`C.M.P. Ib tensor`) is in the BODY frame. Our `recompute_composites` keeps both in the struct frame, and the two inertias are related by the single rotation `I_body = T_parent_this·I·Tᵀ` (frame covariance of the composite combination). So no kernel change is needed: `check_body_rotated` rotates only our composite inertia by `T_parent_this` before comparing, leaving mass + CoM unrotated. Verified element-wise against JEOD's `.out` (trace-preserving rotation; all 20 RUNs pass). Adds `build_run_09`, the `parent_option_a` matrix (verbatim from JEOD Modified_data), `check_body_rotated`, the RUN_09 regen entry, and the committed `.out`. Tolerances at the existing JEOD `%20lf` print-precision floor. Remaining attach_mass gaps (tracked in #99): RUN_08/108 (child attached to two parents — topology resolution) and RUN_109 (named-point attach + the non-identity root orientation). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…accessor RUN_09 previously sidestepped the non-identity root struct→body orientation: it built the parent's mass tree with identity orientation and reconstructed JEOD's body-frame composite inertia inside a bespoke `check_body_rotated` test helper. That worked numerically (the rotation is exact) but kept the JEOD `print_tree` frame convention in test code and never recorded the real orientation on the body. Move the conversion into production and make the scenario faithful: - Add `MassTree::composite_inertia_in_body`, mirroring the struct→body rotation JEOD's `MassBody::print_tree` (`mass_print_body.cc`) applies to the `C.M.P. Ib tensor` line. A bit-exact no-op for identity-oriented bodies. - Set the parent's real `t_parent_this` (optionA) in `build_run_09`. The struct-frame composite is unaffected (the kernel does not fold the root's own orientation into it), so this also exercises that invariant. - Route `composite_errors` through the accessor, delete `check_body_rotated`, and collapse the RUN_09 block to the standard `validate_run` like every other run. - Fix the module docstring count (17 → 18) flagged in review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
JEOD stores `core_properties.inertia` in the body frame (`mass_properties_init.cc:103/119` rotate the StructCG/Struct specs struct→body at init) and computes the composite in the body frame — "the core and composite masses share a common body frame" (`mass_calc_composite_inertia.cc`), with the cm-to-cm offsets rotated by `composite_properties.T_parent_this` (`mass_update.cc:101/107`). Our `calc_composite_inertia` accumulated in the struct frame instead, which is correct only when a body's own struct→body orientation is identity. The divergence was masked everywhere: the sole non-identity orientation in the test suite is Apollo's `yaw_180`, which is inertia-invariant on its diagonal tensors. RUN_09 (the first general, non-180° orientation) exposed it — and previously papered over it with a test-only rotation of the struct-frame composite, which only matched because its core inertia was stored struct-frame (contrary to JEOD). Rewrite `calc_composite_inertia` to compute in the body frame: body-frame core unrotated, parallel-axis offsets rotated struct→body by `t_parent_this`, each child rotated child-body→parent-body (`r = T·Sᵀ·T_childᵀ`). Reduces exactly to the prior struct-frame code when every `t_parent_this` is identity, and is bit-identical for Apollo's yaw_180+diagonal case. No consumer changes: `composite_properties.inertia` now genuinely holds the body-frame tensor that `sync_body_mass_from_tree` and the rotational integrator already expect. - RUN_09 rebuilt with a body-frame core (the StructCG init rotation) and compares `composite_properties.inertia` directly against JEOD's `C.M.P. Ib tensor`; the test-only rotation accessor is removed. - New `runner_attach_composite_inertia_is_body_frame` drives a general orientation through the full `attach → sync_body_mass_from_tree → integrate` pipeline, cross-checking the body-frame composite via an independent frame-invariance derivation (+ sensitivity guard + a torque-free propagation smoke). - Catalog MA.25 (composite inertia in the body frame) + `// JEOD_INV` source tag. Verified: tier3_sim_attach_mass (RUN_09 direct), full dynamics + attach/detach + Apollo trajectory suite (365 tests), the new analytical test, invariant_coverage, and the mass/attach/detach/Apollo/dyncomp bevy↔runner parity wrappers (109 tests) — all green; fmt/clippy/rustdoc clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5612584 to
48fd667
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
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
Adds RUN_09 — the first
SIM_verif_attach_massRUN with a non-identity, non-180° parent struct→body orientation (parent_mass_orientation_optionA): parent (StructCGinertia) + child1 (Body), offset attach[-1,0,0]. Takes attach_mass to 19/21.In the course of validating RUN_09 it surfaced — and this PR fixes — a latent frame bug in the composite-inertia kernel.
Key fix — composite inertia is computed in the body frame
JEOD stores
core_properties.inertiain the body frame (mass_properties_init.cc:103/119rotate theStructCG/Structspecs struct→body at init) and computes the composite in the body frame — "the core and composite masses share a common body frame" (mass_calc_composite_inertia.cc), with the cm-to-cm offsets rotated bycomposite_properties.T_parent_this(mass_update.cc:101/107).Our
calc_composite_inertiaaccumulated in the struct frame, which is correct only when a body's own struct→body orientation is identity.calc_composite_inertiais rewritten to compute in the body frame (body-frame core unrotated, offsets rotated struct→body byt_parent_this, children rotated child-body→parent-bodyr = T·Sᵀ·T_childᵀ). The change is contained toastrodyn_dynamics—composite_properties.inertianow genuinely holds the body-frame tensor thatsync_body_mass_from_treeand the rotational integrator already expect, so no runner/Bevy consumer changes are needed.Why this change is needed, and why it is correct
The bug
MassTree::recompute_compositesproducedcomposite_properties.inertiain the structural frame: it addedcore.inertiadirectly to struct-frame parallel-axis offsets (composite_wrt_pstr.position − cm). Butcore.inertiais defined body-frame (the field doc, the typedInertiaTensor<BodyFrame<V>>, and JEOD's init), and every consumer treats the composite as body-frame:sync_body_mass_from_treecopiescomposite_propertiesinto the integrated body'smassunchanged;torque_body = t_struct_body · torque,ω = ang_vel_body,α = I⁻¹(τ − ω×Iω)— somass.inertiamust be body-frame.For a body whose struct and body frames differ by a non-trivial rotation
T, the struct-frame composite and the body-frame composite differ byT·(·)·Tᵀ, so the integrator was fed a wrong-frame inertia.Why it stayed invisible
The two frames coincide whenever
Tleaves the inertia invariant. Every non-identity orientation anywhere in the suite is Apollo'syaw_180(180° about Z,R = diag(−1,−1,1)), and every Apollo body inertia is diagonal (Modified_data/mass/*.py), soR·diag·Rᵀ = diagexactly — and the stack's X-axis offsets keep the composite diagonal. Identity orientations are trivially invariant. So no existing scenario could observe the difference; RUN_09's generaloptionArotation is the first that does.Why the new computation is correct (JEOD-faithful)
The rewrite is a direct port of JEOD's body-frame composite:
mass_calc_composite_inertia.cc("common body frame");composite_properties.T_parent_this == core_properties.T_parent_this(mass.cc:203).composite_properties.t_parent_this— JEODmass_update.cc:101/107(Vector3::transform(T_parent_this, r_cm_cm_str, …)).composite_wrt_pbdy.T_parent_this— JEODmass_calc_composite_inertia.cc:78(transpose_transform_matrix). For a direct child this rotation isr = T · Sᵀ · T_childᵀ(S = structure_point.t_parent_this, parent-struct→child-struct;T_child = child composite t_parent_this), derived frommass_attach.cc:519.Three independent checks substantiate correctness:
t_parent_thisis identity,r = Sᵀand the offsets stay struct-frame, so the new code is the same expression as the old struct-frame sum (Sᵀ·I·S). Bit-for-bit no-op → the full existing suite (incl. all attach-mass identity RUNs and Apollo's yaw_180+diagonal) passes unchanged.composite_properties.inertiadirectly, element-wise, against JEOD'sC.M.P. Ib tensorfrommass.outfor the generaloptionArotation.runner_attach_composite_inertia_is_body_framebuilds a general-orientation composite, runs the fullattach → sync_body_mass_from_tree → integratepath, and checks the pipeline's body-frame composite against an independent derivationS·(struct composite)·Sᵀ, plus a sensitivity guard asserting it is not the unrotated struct composite (so a regression that dropped the rotation fails loudly).Changes
calc_composite_inertiabody-frame rewrite (crates/astrodyn_dynamics/src/mass_body.rs).composite_properties.inertiadirectly against JEOD'sC.M.P. Ib tensor.runner_attach_composite_inertia_is_body_frameanalytical test.// JEOD_INVsource tag.generate_references.sh+ committedattach_mass_09_mass.out.Verification
tier3_sim_attach_mass(RUN_09 direct), the full dynamics + attach/detach + Apollo trajectory suite, the new analytical test,invariant_coverage, and the bevy↔runner parity wrappers — all green (full local run: 1111 passed).cargo fmt --check+clippy --tests -D warnings+rustdoc -D warningsclean.Remaining (tracked in #99)
RUN_08/108 (child attached to two parents — topology resolution) and RUN_109 (named-point attach + non-identity root orientation).
🤖 Generated with Claude Code