predict(): accept one obscode per time (mixed-observatory prediction) - #429
Merged
Conversation
predict() built its observer positions from a single obscode for all times. The whole downstream chain is already per-time (obscodes_to_barycentric reads a per-row stn, _predict sets each Observation's observer position, and predict_sequence light-corrects per detection), so the single-observatory assumption lived in just one line. Let obscode be either a single string -- used for every time, exactly as before -- or a sequence of length len(times), giving one observatory per time. That predicts a mixed-observatory sequence in one call: each time is light-corrected against its own station while each orbit is integrated once across the whole sorted set (with the marching predict_sequence). A length mismatch raises. Fully backward compatible: a string obscode is broadcast to [obscode]*len(times), which reproduces the previous obs_data byte-for-byte (test asserts str output == same-code-per-time list output exactly). Existing callers (the predict CLI passes a single station; interactive/broad-phase callers pass a string) are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Resolve test_predict.py conflict with #428 (predict_sequence march): keep both new tests (mixed/single obscode + marching equivalence) as separate self-contained functions. Co-Authored-By: Claude Opus 4.8 (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.
Summary
predict()predicted every object to a single observatory for all requested times. The rest of the chain is already per-time —obscodes_to_barycentricreads a per-rowstn,_predictsets eachObservation's observer position, andpredict_sequencelight-corrects per detection — so the single-observatory assumption lived in exactly one line (obs_data = [(obscode, t) for t in times_et]).This lets
obscodebe either a single string (used for every time, as before) or a sequence of lengthlen(times)giving one observatory per time — a mixed-observatory prediction in one call. Each time is light-corrected against its own station, while each orbit is integrated once across the whole sorted set (with the marchingpredict_sequence).Backward compatibility (the important part)
A string
obscodeis broadcast to[obscode] * len(times), which produces the byte-for-byte identicalobs_dataand therefore identical output. The test asserts this directly:Existing callers are unaffected — the
predictCLI passes a singlestation, and interactive / broad-phase callers pass a string. A sequence whose length ≠len(times)raises a clearValueError.Tests
test_predict_mixed_and_single_obscode:ValueError.Why it matters
Combined with the marching
predict_sequence(#428), predicting a catalog to a whole set of(observatory, time)targets — e.g. an ITF attribution sweep — can now integrate each orbit once across all observatories and interpolate at every target, instead of grouping by observatory and re-integrating per group. This PR is independent of #428 (it is correct on its own; #428 just makes the multi-time case fast).🤖 Generated with Claude Code