Conversation
…g log
`summary_from_id_at_epoch` does an O(N) linear walk through every summary
in the file looking for one whose (id, epoch) range matches, and emits a
`debug!` line per non-matching candidate. Adjacent queries in the same
segment redo the entire scan; there is no last-found-segment cache analogous
to SPICE's internal segment selection table.
This test loads the bundled `earth_2025_250826_2125_predict.bpc` (37 ITRF93
segments) and queries Earth orientation at two epochs that fall in the same
segment ~21 segments deep. Both queries emit 20 identical 'not valid at'
debug records, demonstrating the linear scan is repeated each call.
Run with:
cargo test -p anise --no-default-features --features metaload \
--test perf_segment_cache -- --nocapture
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the ICRS orientation constant and associated frame bias parameters to the constants module. It also adds a performance regression test demonstrating that segment lookups currently perform a linear scan without caching, leading to redundant work and log spam. Feedback suggests that the new ICRS constant should be integrated into the name-to-ID mapping functions to ensure proper resolution and consistency.
| /// (~0.7 m at Earth's surface). ID 22 is the next sequential ID after | ||
| /// the SPICE built-in inertial frames (1-21); SPICE itself does not | ||
| /// define a separate ICRS orientation. | ||
| pub const ICRS: NaifId = 22; |
There was a problem hiding this comment.
The new ICRS constant should be integrated into the name-to-ID mapping functions to ensure it can be resolved by name and correctly identified in output. Specifically, add ICRS to the match statement in orientation_name_from_id (around line 283) and the string "ICRS" to id_from_orientation_name (around line 317). To maintain type safety and consistency with repository standards, ensure these match statements are exhaustive and avoid catch-all arms. Additionally, evaluate whether "ICRF" (which currently maps to J2000 at line 319) should be updated to map to ICRS to reflect the distinction being introduced between the dynamical J2000 and the kinematic ICRS/ICRF.
References
- When converting enums or handling variants in match statements, use exhaustive matches instead of catch-all arms to ensure all variants are explicitly handled and to improve type safety.
1 participant
This provides the failing test for #691 so you can test to see what I see. :)