Repository navigation
Conversation
The verification harness looped over SGP4-VER.TLE inside a single test and swallowed NotImplementedError, so only three of the 29 satellites were actually asserted; the rest were skipped without a trace. Turn it into one parametrised test per satellite, with the satellites the propagator cannot handle yet named in NOT_YET_SUPPORTED, so that enabling one is a one-line change and a meta-test forbids a satellite from escaping unnoticed. Take the times to propagate to from the reference file rather than from the start/stop/step on line 2 of the TLE. The reference is the authority on which states are defined: it stops early for satellites that decay during the requested span, and it does not cover the second span that 20413 asks for. Reach the propagator through the public Orbital interface, which makes the LineOrbital subclass and its use of private classes unnecessary. Add propagation benchmarks to record a baseline before the propagator is changed.
Only three of the verification satellites were propagated at all: the propagator refused every mode but the near-earth normal one, so the five satellites whose perigee is low enough for the simplified drag equations were turned away, and the correction of mean anomaly and argument of perigee for drag was applied to them regardless, which the simplified equations must not do. Accept the simplified mode and skip the correction there. Compute the mean motion constant and the atmospheric drag fit instead of copying the nine-digit decimals printed in Spacetrack Report pytroll#3. Rounding of the mean motion constant carries into the semi-major axis, and from there into a difference of two nearly equal altitudes that the drag fit raises to the fourth power. For a satellite with a low perigee that moved the position half a metre along track over a day, ten times the accuracy the model is verified to, and it kept three satellites from being propagated at all. Every near-earth verification satellite now agrees with the reference to a few nanometres, where the closest agreement before was two millimetres. Drop two constants nothing uses, one of them a rounded copy of a value that is already computed exactly.
A satellite whose orbit takes 225 minutes or more was refused outright. At that distance the Sun and the Moon move it as much as the Earth's own oblateness does, and SGP4 leaves them out. Add them, in a module of their own: the geometry of the two bodies relative to the orbit and the coefficients that follow from it, computed once at the epoch; the slow drift they impose, added to the mean elements at each propagation; and the periodic wobble, applied to the elements themselves. Near the equator the node is read back from the perturbed orbital plane rather than shifted directly, which keeps it well behaved as the orbit flattens out. The right ascension is used as a number rather than as the argument of a sine in that last step, so whether it is measured from zero or from minus half a turn changes the result by a kilometre for an orbit whose node sits just below zero. Measure it from zero, as the software that produces the element sets does. Deep-space satellites share the simplified drag equations with the near-earth satellites whose perigee is low, so name that condition rather than testing the mode in four places. Recompute what depends on the inclination once the Moon has moved it. This propagates the nine deep-space satellites of the verification set that do not resonate with the Earth's rotation, to within five nanometres of the reference. The resonant ones are still refused, as is propagation to several times at once.
An orbit that goes round twice while the Earth turns once keeps meeting the same bulges of the equator, so their small pull accumulates instead of averaging away, and the orbit drifts in a way no closed expression captures. Follow it step by step instead: fit the strength of the beat to the orbit's eccentricity and inclination at the epoch, then march the mean longitude and the mean motion forward in steps of twelve hours. The march restarts from the epoch every time, rather than carrying the integrator's position between calls as the article's own code does. Asking for one time then has the same answer whichever times were asked for before, and whether any were asked for at all. The bulges turn with the Earth, so the sidereal angle the beat is measured against advances with the propagation rather than staying at the epoch. Holding it fixed moves a Molniya orbit thousands of kilometres. This propagates the five Molniya-like satellites of the verification set. The ones that circle once a day are still refused.
A satellite that keeps station over one spot of the equator feels the same three bulges of that equator turn after turn, in the same direction each time. Fit the strength of that beat to the orbit and march it forward with the same integrator the twelve hour orbits use, which differs only in which harmonics it sums and in how the mean anomaly is read back from the mean longitude. Every satellite of the verification set is now propagated, so the list of the ones that were not, and the skip it drove, are gone. What remains is a test that the reference file and the set of satellites propagated here still describe the same satellites, so that none can be quietly dropped.
Only one time at a time could be asked for, because the choice between the two ways of applying the periodic wobble, the righting of an orbit tipped past the equator, and the march of the resonance were all written as if the time were a single number. Work out both ways of applying the wobble and choose between them elementwise, and march every time from the epoch together, each with its own direction and its own number of steps, advancing only those that still have steps left to take. The march still restarts from the epoch, so asking for a set of times in any order, or asking again, gives the same answers. A satellite that resonates is now propagated to two thousand times in the time it took to propagate to fifteen. Asking for many times at once does not give bit for bit what asking for each one separately gives: Kepler's equation is solved by iterating until every time in the array has converged, so the ones that converge first take a few more turns of a loop that has all but stopped moving them. The difference is a thousand times smaller than the accuracy the model is verified to, and removing the shared stopping test makes the two agree exactly. The new test that every verification satellite agrees between the two ways of asking is what turned this up.
Kepler's equation was iterated until every time in the array had converged, and those that converged first kept being carried along by a loop that had all but stopped moving them. Their last bits ended up depending on which other times they were asked for alongside, which the model should never do. Leave a time alone once it has converged. The two ways of asking now agree exactly, so the test that they agree says so rather than allowing a millimetre. Propagating to one time costs a comparison more than before; propagating to ten thousand is no slower.
Forty properties forwarded attribute by attribute to the object that holds the orbit, and nothing read them: propagation reached past the wrapper to that object directly, and nothing outside this module ever held one. Let the class that does the work carry the name and the one method the wrapper really added. Benchmark the deep-space paths as well, from each satellite's own epoch. A resonant orbit is followed in steps of twelve hours from its epoch, so what it costs depends on how far from that epoch it is asked about: twenty years out is fourteen thousand steps and four hundred milliseconds, which would have timed the march rather than the propagation. Measured against the propagator as it stood before this branch, by alternating between the two fifteen times and taking medians: propagating to ten thousand times is around eight per cent quicker, which is the one figure here that several ways of estimating it agree on. Propagating to a single time is a few per cent slower, somewhere between two and six depending on how the medians are taken; this machine is too noisy to say better than that, individual pairs of runs differing by more than fourfold in both directions.
Changing three places in the propagator leaves every AIAA test passing: the eccentricity at which one fit of the twelve-hour resonance gives way to the next, the lower edge of the band of mean motions that resonance is held to apply to, and the eccentricity terms of the geosynchronous fit. The article's satellites straddle the first two without pinning where they lie, and are all so nearly circular that the third contributes a millionth of what it could. Propagate three orbits that sit in those gaps, one of them for a week as well as a day, since the beat against the Earth's rotation builds up with time and a difference too small to show in a day is plain in a week. Their expected positions come from a port of the reference implementation which reproduces every state of the verification file to within five nanometres; it is not needed to run these tests, only to have written them.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #243 +/- ##
==========================================
+ Coverage 92.34% 92.68% +0.34%
==========================================
Files 19 22 +3
Lines 4207 4664 +457
==========================================
+ Hits 3885 4323 +438
- Misses 322 341 +19
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Propagating to many times at once gives what propagating to each one gives, and the propagator was asked to prove it exactly. That asks more of numpy than of the propagator: evaluating a sine over an array need not use the same instructions as over a single number, and on some machines the two differ in the last bit. Allow a nanometre, which is a thousand times finer than the accuracy the model is verified to and still shows an answer that depended on what else was asked for. Comparing a set of times against the same set permuted, or asked for twice, is the same arithmetic either way, so those stay exact. Also drop the tolerance of the deep-space branch tests to the one the verification uses, which its comment already claimed.
A pixel whose ray misses the ellipsoid has no intersection, and the square root of the negative discriminant gives a nan and a warning with it. The attitude estimator reaches such rays on its way: it explores to the edge of its bounds, where a tilt of half a radian with a scan half-angle of fifty-five degrees puts the outermost pixels exactly on the limb, and whether the discriminant lands just above or just below zero there depends on the machine. It sits at 1.2e-10 on this one. Clamp it to zero, taking the point of closest approach on the tangent line, so that the result stays finite and continuous wherever the optimizer looks.
The article reaches days-since-1949 by way of a Julian day. One of those for a date in the nineteen-nineties is a number near two and a half million, and the doubles either side of it lie forty microseconds apart, so what comes back is wrong by up to that much: nineteen microseconds for the WIND spacecraft, one of the verification satellites. Counting straight from the epoch, as here, is right to a fraction of a microsecond. That is worth a note because the shorter route looks like a simplification and is not, and because the difference is measurable. It moves where the Sun and the Moon are taken to be, and WIND's orbit is eccentric enough to turn that into four micrometres of position, which is most of what separates this propagator from the verification file for that satellite. The file was made with the Julian day and carries its rounding.
Manny7717
approved these changes
Aug 31, 2026
Manny7717
left a comment
Manny7717
left a comment
There was a problem hiding this comment.
Verified locally on head b582f15 (AIAA 2006-6753 acceptance suite + full test run):
- AIAA verification: 65/65 pass (test_aiaa.py + test_deep_space_branches.py) — every satellite in SGP4-VER.TLE propagated to every reference state within the declared tolerances (5e-6 km position / 5e-9 km/s velocity), including the once-a-day, twice-a-day, and non-resonant deep-space branches, the simplified-drag near-earth branch, and the checksum-error satellites. The acceptance suite structure is sound: guards against silently dropping cases.
- No regressions: full pyorbital/tests run → 12 failed / 201 passed on head vs 12 failed / X passed on base c6a2120 — byte-identical failure sets (all pre-existing xarray/dask env noise in TestGetObserverLook*, present on base too). Head additionally passes the 65 new verification tests + new test_orbital.py cases.
- Code review: new cleanly separated from the propagator core; DeepSpace/apply_periodics wiring in _SGDP4 gated on the period threshold; epoch handling and the 2005-12-31 leap-second discontinuity explicitly accounted for in the test harness (one-second offset between reference calendar times and epoch-relative minutes). geoloc.py discriminant clamp (commit 35264db) is a sound, documented behavior fix: negative discriminant → closest-approach point instead of NaN, keeping results finite and continuous for off-Earth attitude sweeps.
- Design notes (non-blocking): this is a large rewrite (417 lines in orbital.py) of a numerically sensitive module; the AIAA reference suite is exactly the right safety net, and it passes. The mantissa-level AT_ONCE_TOLERANCE for vectorized-vs-scalar agreement is pragmatic.
Clean, well-tested, and reference-verified. Approving.
One of the four modes the propagator was ported with, in 2011, is entered when the eccentricity is less than zero. The same commit that added it also added the check that refuses any element set whose eccentricity is not greater than zero, so the branch has never been taken. It could not have run in any case: it sets the mode from an attribute of the instance, and the name it reads is a module constant, so it would have raised an attribute error rather than setting anything. What it guarded in the propagation was a placeholder that raised. Nothing is lost with it. A circular orbit, which the name suggests it was meant for, is a separate matter: that one is turned away by the eccentricity check above, not by this.
Upstream already clamps the ellipsoid discriminant, so the duplicate from this branch is dropped. The reworked AIAA suite supersedes the logging and tolerance tweaks upstream made to the old test. The ISS altitude pinned upstream came from the previous propagator; this branch moves that position by a few centimetres, so the value is regenerated from the merged code.
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
2 participants
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.
What this does
pyorbital refused any satellite whose orbit takes 225 minutes or more, and also refused near-earth satellites with a low enough perigee to need the simplified drag equations. Between them that was 26 of the 29 satellites in the AIAA verification set. This adds the deep-space model from AIAA 2006-6753 in a new module, pyorbital/_sgdp4_deep.py, hooked into the existing propagator: the geometry of the Sun and Moon at the epoch, the slow drift they impose, the periodic wobble they impose, and — for orbits that circle once or twice while the Earth turns once — the beat against the Earth's own equatorial bulges, integrated forward in twelve-hour steps.
All 29 verification satellites now propagate, with no skips.
Two bugs found on the way
The mean motion constant was rounded. XKE was hardcoded as 0.743669161e-1, the nine-digit decimal printed in Spacetrack Report #3. Its relative error of 4.5e-10 lands in the semi-major axis, shifts the computed perigee by 1.9e-6 km, and because the atmospheric drag fit takes (120 - sfour) — a difference of nearly equal altitudes — to the fourth power, it comes out amplified 370× for low-perigee satellites. It cost half a metre of along-track position over a day and kept three satellites from being propagated at all. XKE and QOMS2T are now computed from the defining WGS-72 parameters. This improved every near-earth satellite by roughly 100×, including ones that were already passing.
The drag correction was applied where it shouldn't be. The mean-anomaly and argument-of-perigee correction was applied unconditionally; Vallado applies it only when the full drag expansion is in use. Deep-space and low-perigee satellites share the simplified equations, so that condition is now named rather than tested in four places.
Also
Accuracy
All 29 satellites match aiaa_results within main's tolerances (5e-6 km, 5e-9 km/s). Worst position error is 4.475e-06 km on satellite 23333 (WIND, eccentricity 0.973) — 89% of the budget. Worst velocity error is 45% of budget. Beyond the verification set, ~24,000 propagated states across ~2,270 synthetic orbits spanning inclination, eccentricity and mean motion agree with the reference implementation to under 1e-5 km for every orbit whose perigee is above ground.
Known follow-ups