Repository navigation
Fix NaN in rising/setting search near a crossing (#1128) - #1145
Open
karimosama7 wants to merge 1 commit into
Open
karimosama7 wants to merge 1 commit into
karimosama7 wants to merge 1 commit into
Conversation
If `start_time` fell just microseconds before a rising or setting, the first interpolated guess in `_find()` was built as a single float Julian date, which rounded back to exactly the sample time and made the hour angle rate 0/0 = NaN. The NaN time then reached the ephemeris, which raised a misleading EphemerisRangeError. Now the interpolated offset is kept in the fraction, and a step that doesn't move `t` assumes about one turn of the sky per day instead of dividing by zero. Addresses the almanac half of skyfielders#1128. Co-Authored-By: Claude Opus 5.5 <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
1 participant
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.
This fixes the almanac half of #1128:
find_settings()/find_risings()raising a misleadingEphemerisRangeErrorwhenstart_timefalls just before a crossing.Cause. In @ejeschke's reproduction,
t0is itself almost exactly a sunset, so the Sun sets again only ~16 µs later anddifference[0]in_find()is just 3.58e-10 radians. The interpolated first guess was built as a single float Julian date, whose resolution near JD 2.46 million is ~40 µs, so it rounded back to exactlytt[i]. That madet_diffzero,ha_per_day = 0/0 = NaN, and the NaN time then reached jplephem. It reproduces with NumPy 2.4.4 as well as 2.5.3 (on Windows), so it isn't NumPy-specific.Fix.
ts.tt_jd(tt[i], offset), so a tiny offset survives in the fraction.t_diffis still exactly zero (a sample landing exactly on a crossing), assume about one turn of the sky per day for that step instead of dividing 0/0.Behavior change. For the reproduction,
find_settings()now returns two settings,2026-08-08T05:01:51.54Z(16 µs aftert0) and2026-08-09T05:01:15Z, because the first one is inside[t0, t1].Testing.
test_setting_a_few_microseconds_after_start_timeuses the inputs from the issue. It fails onmasterwithRuntimeWarning: invalid value encountered in divideand passes with this change.t_diffguard applied, and without the interpolation change, the reproduction also passes and returns the same two settings to within nanoseconds.yflags are identical. Times differ by under 0.03 µs, except up to a few µs at 66°–70°, mostly ony=Falseevents.assayneedsfcntl. All almanac tests pass on NumPy 2.4.4 and 2.5.3. The rest of the suite has the same results before and after this change.The other half of #1128 (
.apparent()on a star with NaN coordinates) is not touched here; I described it in the issue.🤖 Generated with Claude Code