Add get_last_dn_time, find_aos and find_aol - #246
Conversation
Closes pytroll#95. The ascending node search already knew how to walk backwards to an equator crossing and bisect onto it. A descending node is the same search with the satellite heading the other way, so the two now share _get_last_node_time and differ only in a sign.
The stub has never done anything. Rather than scan for the horizon crossing afresh, ask get_next_passes, which already samples elevation and bisects onto the crossing, and take the rise time of the first pass it reports. Closed PR pytroll#206 implemented this with its own scan in fixed five minute steps, which stepped clean over any pass shorter than the step. Sampling every minute makes that rarer but not impossible, and the docstring says so.
Loss of signal is asked about while the satellite is up, so unlike find_aos this counts a pass already under way and answers with the end of it. get_next_passes drops such a pass, so find_aol scans for the crossing itself; the scan both now share lives in _scan_elevation.
Asking for a horizon the satellite never clears got a bare IndexError out of the empty pass list, which says nothing about what was wrong. Name the satellite, the horizon and the window instead.
This changes find_aol's contract: it returned None when the satellite did not set within the window, which left the caller to discover the problem later, in arithmetic on a None. It now says what it looked for and did not find, as find_aos does. The wording is not shared verbatim between the two. find_aol can come up empty even when the satellite rose, if the pass has not ended before the window closes, so telling the caller it never rose would be untrue.
The bisection kept the midpoint's z coordinate in a variable named for the earlier end of the bracket, which was accurate at every point it was read but says the wrong thing. Give the midpoint its own name. The tests built the same NOAA-20 out of the same two lines seven times over. They share a fixture now, along with the observer and the pair of elevations either side of a horizon crossing.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #246 +/- ##
==========================================
+ Coverage 92.17% 92.34% +0.17%
==========================================
Files 19 19
Lines 4127 4180 +53
==========================================
+ Hits 3804 3860 +56
+ Misses 323 320 -3
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:
|
Manny7717
left a comment
Manny7717
left a comment
There was a problem hiding this comment.
Verified locally on head 2dd343e vs base 0a154c3 — implementation is correct and regression-proven.
Regression-proven: all 5 new tests FAIL on base (get_last_dn_time/find_aos/find_aol don't exist or are pass stubs) and PASS on head. Full pyorbital/tests suite: head 249 passed/1 skipped vs base 244 passed + 5 failed/1 skipped — delta is exactly the new tests, zero regressions (existing get_last_an_time tests still pass, so the node-search refactor preserved old numerics). ruff clean on both changed files.
Node search (_get_last_node_time): the heading-multiplication trick correctly unifies ascending/descending: for the NOAA-20 fixture, get_last_an_time returns the same 10:44:18.234375 as before (sub-second identical), and get_last_dn_time returns 09:53:29.015625 with pos_z ≈ 0, vel_z < 0 at the crossing. Alternation probe: AN(11:00) > DN > AN(DN) > DN — nodes strictly alternate. Bisection tolerance (1 km) and 10-min walk-back step match the old code exactly.
find_aos: delegates to the refactored get_next_passes and returns passes[0][0]. Verified it equals the first complete-pass rise computed independently (sub-second agreement with a direct get_next_passes(..., 24, ...) call), and — important semantic — because get_next_passes only emits a pass once a set follows a rise, an in-progress pass at utc_time (rise already past) is naturally skipped, matching the docstring. Elevation bracketing probe: 10 s before AOS elevation = −0.565°, 10 s after = +0.574°.
find_aol: the scan-and-root-find path is correct — crossing indices come from np.diff(np.sign(elev)), elev[crossing] > 0 selects the +→− (setting) crossings, and _get_root's brentq bracket is guaranteed to straddle zero by construction. Probe: 10 s before AOL +0.592°, 10 s after −0.581°, and AOL lands inside the pass in progress at 11:00 (11:08:30) while AOS is the next pass (12:33:49) — exactly the documented asymmetry, and it's asserted in test_find_aol_ends_the_pass_already_under_way.
Error paths: horizon=85 (never reached for this observer/window) raises ValueError for both, message includes satellite name, horizon, and window — no silent None/IndexError. Both accept datetime, tz-aware-UTC, and np.datetime64 inputs (the _get_tz_unaware_utctime normalization is inherited).
Refactor safety: get_next_passes' inline scan was extracted verbatim into _scan_elevation (same minute sampling, same zcs detection, same tol/60 root precision); a direct base-vs-head comparison of get_next_passes output on the fixture shows identical pass lists. Default tol=0.001 → _CROSSING_TOLERANCE_SECONDS is value-identical (0.001).
Non-blocking nits (no action required):
- The 24 h window with 1-minute sampling means a pass that both rises and sets entirely between two samples is invisible — this is documented in both docstrings, just noting it also applies to very low-altitude/fast objects.
find_aos/find_aoldeliberately describe different passes when a pass is in progress (AOS = next rise, AOL = current set). Documented and tested, but callers wanting a matched pair should useget_next_passesdirectly.
Nice salvage of #206 — clean factoring, honest docstrings (the "heading" unification comment is particularly good), and tests that assert physical properties (velocity sign at node, elevation sign flip at crossing) rather than implementation details.
2 participants
Picks up the two features from #206, which the author closed and invited us to
salvage. Both are written fresh; none of that branch's code is carried over.
get_last_dn_timegives the last descending node. It shares the existingascending node search, which already knew how to walk back to an equator
crossing and bisect onto it — a descending node is the same search with the
satellite heading the other way, so the two now differ only by a sign.
find_aosandfind_aolwere stubs that did nothing.find_aosasksget_next_passesand takes the rise time of the first pass.find_aolscansfor the setting itself, because it has to answer about a pass already under
way and
get_next_passesdiscards those.Two things worth knowing about the result:
find_aosanswersabout the next pass,
find_aolabout the end of the current one. That iswhat you want when you ask "when do I lose this satellite?", but it means
they don't pair up. Both docstrings say so.
two samples is not seen. Refactor orbital tracking with vectorized methods, bug fixes and improved utilities #206 sampled every five minutes and missed short
passes outright; this makes that rarer, not impossible. Fixing it properly
means a step derived from the pass duration, which is separate work.
Asking for a horizon the satellite never reaches used to give a bare
IndexErroror a silentNone. Both now say which satellite, which horizonand which window came up empty.
get_last_dn_timeto get the last descending node #95