perf: build the real-Moon centerline once per real eclipse in fixed_system_search#7
Merged
Merged
Conversation
generate_central_track(context, real_tt, "real_moon", ...) depends only on real_tt, but was called once per second-moon candidate inside the inner loop of fixed_system_search. Hoist it above the loop. Results are unchanged: the hoisted call takes identical arguments and the track is treated as read-only downstream.
Fails on main, where generate_central_track is called once per second-moon candidate instead of once per real eclipse.
13 tasks
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
In
fixed_system_search,generate_central_track(context, real_tt, "real_moon", step_seconds=120.0, include_partial_maximum=True)sat inside thefor second_event in second_events:loop. Its only time-dependent argument isreal_tt, which is fixed for the whole loop, so the identical track was recomputed for every second-moon candidate of the same real eclipse.generate_central_trackcall above the inner loopif not second_events: continuefirst, so real eclipses with no candidates do less work than before rather than more (without this guard, hoisting would build a track for every real eclipse in the window, including the ones that currently build none)Work plan item P1 / 4.
Note on aliasing
Candidates from the same real eclipse now share one
real_trackdict rather than each holding an equal-valued copy, including the entries stored intracks[event.event_id]. Every downstream consumer (minimum_track_distance_km,_best_common_location, plotting) reads without mutating, so this is safe — but it is the one behavioural difference worth a reviewer's eye.Scientific impact
None. The hoisted call is argument-for-argument identical to the one it replaces, and
generate_central_trackis a pure function of(context, tt_jd, body, step_seconds, include_partial_maximum). Track distances, classifications, and site selection are bit-for-bit unchanged.Suggested verification per the work plan: run the fixed-system search over a 1-year window and diff
all_candidates.csvbefore/after — it should be byte-identical.Checklist
ruff check .passespytestpasses locallytests/test_fixed_system_search_track_reuse.pystubs the search's collaborators so it needs no ephemeris kernel, and asserts onereal_moonbuild against threesecond_moonbuilds. It fails onmain(3 real-Moon builds) and passes here. A second case asserts no track is built when a real eclipse yields no candidates.Lint/test boxes left unchecked because this branch was pushed without a local run — CI on this PR is the first execution.