Skip to content

Validate OMM reference context before propagation - #27

Closed
sylvesterkaczmarek wants to merge 3 commits into
esa:masterfrom
sylvesterkaczmarek:fix/validate-omm-reference-context
Closed

Validate OMM reference context before propagation#27
sylvesterkaczmarek wants to merge 3 commits into
esa:masterfrom
sylvesterkaczmarek:fix/validate-omm-reference-context

Conversation

@sylvesterkaczmarek

@sylvesterkaczmarek sylvesterkaczmarek commented Aug 14, 2026

Copy link
Copy Markdown

Summary

Reject OMM metadata that is incompatible with the reference context assumed by dSGP4 propagation.

Changes

  • require CENTER_NAME to resolve to EARTH
  • require REF_FRAME to resolve to TEME
  • require TIME_SYSTEM to resolve to UTC
  • preserve the existing supported defaults when those optional fields are absent
  • accept case-insensitive equivalents
  • add regression tests for invalid, case-insensitive, and missing reference-context fields

Rationale

load_from_omm() previously accepted incompatible reference metadata and then propagated the elements using the Earth/TEME/UTC assumptions used internally by dSGP4. This change rejects those mismatches before propagation.

Validation

Focused regression validation passes. The upstream GitHub Actions workflow is currently waiting for maintainer approval to run for this fork PR.

@sylvesterkaczmarek
sylvesterkaczmarek marked this pull request as ready for review August 15, 2026 18:41
@Sceki

Sceki commented Aug 16, 2026

Copy link
Copy Markdown
Member

Hi @sylvesterkaczmarek ,

can you provide an actual input/use case where dSGP4 currently produces an incorrect or misleading result because of this?

@sylvesterkaczmarek

Copy link
Copy Markdown
Author

Yes. A concrete case is an OMM with the normal SGP4 elements but REF_FRAME=EME2000.

Today load_from_omm() accepts it and produces the same propagatable object as if REF_FRAME=TEME, because the reference-frame metadata is not used when converting the OMM fields into the SGP4/TLE representation.

The caller therefore gets a propagated result without any indication that EME2000 elements were interpreted under dSGP4's TEME assumptions. The same issue applies to a non-UTC TIME_SYSTEM or non-Earth CENTER_NAME.

The purpose of this PR is to fail explicitly at ingestion rather than silently reinterpret incompatible metadata.

@sylvesterkaczmarek sylvesterkaczmarek left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing CENTER_NAME, REF_FRAME, or TIME_SYSTEM is silently defaulted to EARTH/TEME/UTC, so an OMM with unknown reference context is accepted and propagated under assumed coordinates. Require these fields on OMM input (the writer can still emit defaults) and make the missing-context regression fail closed.

@Sceki

Sceki commented Sep 3, 2026

Copy link
Copy Markdown
Member

@sylvesterkaczmarek

Thanks for the follow-up.

I don't think this change is a good fit for dSGP4, for a few reason (at least as of now).

A concrete case is an OMM with the normal SGP4 elements but REF_FRAME=EME2000.

This EME2000 example is constructed rather than observed. Under CCSDS 502.0-B-3, SGP4 mean elements are by definition expressed in TEME, so an OMM with MEAN_ELEMENT_THEORY = SGP4 and REF_FRAME = EME2000 is malformed at the source. Guarding against it means adding a permanent validation surface for a file that shouldn't exist in the first place and that hasn't been reported by any user (at least so far).

If we did want an ingestion guard, MEAN_ELEMENT_THEORY would be the field to check, since that is the one that actually varies across real OMMs (SGP4, SGP4-XP, DSST, USM). REF_FRAME, TIME_SYSTEM and CENTER_NAME follow from it.

The proposal in your last comment: requiring those three fields and failing closed when they are absent, I believe it would be a breaking change for a large share of users. CelesTrak's GP JSON/CSV output omits CENTER_NAME, REF_FRAME and TIME_SYSTEM entirely, and that is one of the most common ways OMMs reach load_from_omm(). Rejecting those inputs would break working pipelines to protect against a hypothetical one.

I will at some point add a guard on MEAN_ELEMENT_THEORY, but I'm going to close this for now. If you do hit a real OMM in the wild where dSGP4 silently produces a wrong result, please open an issue with the file attached and I'll happily look at it again.

@Sceki Sceki closed this Sep 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants