Port SIM_csr_compare gravity-acceleration octant sweep (#207)#613
Merged
Conversation
JEOD's SIM_csr_compare (the name is historical — JEOD 5.4 ships no CSR file) loads earth_GGM05C at degree/order 70, places a non-integrating vehicle, and teleports it through sign-permutations of an ISS-like position at t=1..5s, logging grav_pot/grav_accel/position. Because the body never integrates, this is a per-step gravity-evaluation cross-check, not a trajectory propagation. Adds: - A csr_compare_run01 entry + CSR_COMPARE_SNIPPET (DRAscii logger) to trick/generate_references.sh, and the regenerated reference CSV. - tier3_sim_csr_compare.rs evaluating our GGM05C 70x70 gravity via the production GravityControl::evaluate_accel_only path at each input-deck position, compared against JEOD's logged grav_accel. The sweep positions are JEOD source constants (not output), so computational independence holds. Earth orientation is the planet-generic default (no RNP object in the S_define), so the planet-fixed rotation is identity. - A PERMANENT_GAPS entry (csr_compare) in parity_coverage.rs: the non-integrating gravity-eval has no Bevy-parity counterpart. Our kernel reproduces JEOD's grav_accel to machine precision (observed per-component max <= 4.4e-16 m/s^2 on a ~7 m/s^2 signal); tolerances use a 1e-14 m/s^2 machine-epsilon floor. Closes #207. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Ports JEOD’s SIM_csr_compare gravity-acceleration “octant sweep” into the Rust verification suite by adding reference generation for the sim’s logged gravity outputs and a Tier 3 test that evaluates Astrodyn’s GGM05C 70×70 gravity via the production GravityControl::evaluate_accel_only path and cross-validates against the JEOD CSV.
Changes:
- Added a new
CSR_COMPARE_SNIPPETandcsr_compare_run01reference-generation entry to produce a committed DRAscii CSV forSIM_csr_compare. - Added
tier3_sim_csr_compare.rsto compare Astrodyn’s computedgrav_accelagainst JEOD’s loggedgrav_accelat each teleported position. - Added a
PERMANENT_GAPSentry (csr_compare) to document that this non-integrating per-step gravity evaluation has no Bevy parity counterpart.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
trick/generate_references.sh |
Adds DRAscii snippet + background run entry to generate csr_compare_run01 reference CSV. |
crates/astrodyn_verif_parity/tests/parity_coverage.rs |
Documents csr_compare as a permanent parity gap (non-Simulation::step() comparison). |
crates/astrodyn_verif_jeod/tests/tier3_sim_csr_compare.rs |
New Tier 3 test that evaluates GGM05C 70×70 accel-only gravity at JEOD deck positions and compares to JEOD CSV. |
crates/astrodyn_verif_jeod/test_data/csr_compare_run01_csr_compare.csv |
New committed JEOD reference CSV produced by the added DRAscii logging. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address Copilot review on #613, verified against JEOD source (models/environment/gravity/verif/SIM_csr_compare): - generate_references.sh: PID_CSR_COMPARE was launched but never waited on, so the script could report success before SIM_csr_compare finished (risking incomplete/missing CSV). Add the wait alongside PID_DYNCOMP. - tier3_sim_csr_compare.rs: module doc claimed `t_inertial_pfix = None`, but the code passes Some(&DMat3::IDENTITY) (evaluate_accel_only panics on None for non-spherical gravity). JEOD evaluates gravity with pfix->state.rot.T_parent_this left at its uninitialized identity since the S_define has no RNP object (spherical_harmonics_calc_nonspherical.cc :109/440), so identity is the correct rotation. Fix the doc to match. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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.
Closes #207. Part of the #99 Tier 3 coverage burn-down.
What
JEOD's
SIM_csr_compare(name is historical — JEOD 5.4 ships no CSR coefficient file) loadsearth_GGM05Cat degree/order 70, places a non-integrating vehicle, and teleports it through sign-permutations of an ISS-like position at t=1..5 s, logginggrav_pot/grav_accel/position. Since the body never integrates, this is a per-step gravity-evaluation cross-check, not a trajectory propagation.Adds:
csr_compare_run01entry +CSR_COMPARE_SNIPPET(DRAscii logger) intrick/generate_references.sh, and the regenerated reference CSV.crates/astrodyn_verif_jeod/tests/tier3_sim_csr_compare.rs, which evaluates our GGM05C 70×70 gravity through the productionGravityControl::evaluate_accel_onlypath at each input-deck position and compares against JEOD's loggedgrav_accel.PERMANENT_GAPSentry (csr_compare) inparity_coverage.rs— the non-integrating gravity-eval has no Bevy-parity counterpart.Notes
iss_typical_state.py), not JEOD output; only thegrav_accelcompared against is output.pfix->state.rot.T_parent_thisleft at its uninitialized identity (spherical_harmonics_calc_nonspherical.cc:109/440). The kernel is therefore passed the identity rotationt_inertial_pfix = Some(&DMat3::IDENTITY)(evaluate_accel_onlypanics onNonefor non-spherical gravity) — confirmed correct by the machine-precision result.grav_accelto machine precision (observed per-component max ≤ 4.4e-16 m/s² on a ~7 m/s² signal). Tolerances use a 1e-14 m/s² machine-epsilon floor (1.05× observed would be a sub-ULP/zero tolerance).Verification
tier3_csr_compare_gravity_octants— PASSparity_topics_are_a_superset_of_tier3_topics— PASS (new PERMANENT_GAPS entry)cargo fmt --check,cargo clippy -p astrodyn_verif_jeod -p astrodyn_verif_parity --tests -- -D warnings— cleanReview fixes (07cd668)
generate_references.sh: added the missingwait $PID_CSR_COMPAREso the regen script can't report success before the sim finishes.tier3_sim_csr_compare.rs: corrected the stale module doc that saidt_inertial_pfix = Noneto match the actualSome(&DMat3::IDENTITY), with the JEOD-source rationale above.🤖 Generated with Claude Code