Skip to content

Fix silently wrong BJD_TDB when -ov and -nea are combined - #1391

Open
opusbuilds wants to merge 1 commit into
rzellem:michael_fitzgeralds_wonderful_branch_of_magicfrom
opusbuilds:fix-radec-string-bjd
Open

Fix silently wrong BJD_TDB when -ov and -nea are combined#1391
opusbuilds wants to merge 1 commit into
rzellem:michael_fitzgeralds_wonderful_branch_of_magicfrom
opusbuilds:fix-radec-string-bjd

Conversation

@opusbuilds

Copy link
Copy Markdown

Fixes #1390.

Root fix: the coordinate-conversion dispatch now runs radec_hours_to_degree when both flags are set (-nea alone and -ov alone unchanged). Defensive fix: convert_jd_to_bjd coerces string coordinates before any conversion, so any future path delivering strings converts correctly or fails loudly instead of silently mis-parsing hours as degrees in the astropy fallback.

Verified: string and numeric inputs now agree to 0.000000 s, barycorrpy handles the conversion (no fallback warning), and the regression test pins both the agreement and the absolute +522.5 s offset for the CoRoT-2 geometry so a sign error can't return. Full measurement table and cause chain in #1390. Test suites: only the two pre-existing *windows* failures, unrelated (platform-specific, run on Linux).

…l RA/Dec reached the time conversion unconverted

With --override and --nasaexoarch together, the override wins the data
choice (pDict = raw init-file values, sexagesimal strings) but the
nasaexoarch branch wins the coordinate-conversion dispatch, so
radec_hours_to_degree never ran. barycorrpy then raised TypeError on the
string RA, and the astropy fallback parsed "19:27:06.50" with unit=deg,
i.e. 19.45 degrees instead of 19h27m = 291.78 degrees, computing the
barycentric correction for a point up to ~150 degrees away on the sky.
Measured BJD_TDB errors: -538 s (CoRoT-2), -206 s (HAT-P-27), -93 s
(TOI-1516) - at or beyond a typical Tmid error bar, roughly constant
within a night, so reported mid-transit times shift by the same amount.
The same unconverted string reaches the airmass code, dormant only when
headers carry TELALT.

Two changes: the branch dispatch now runs the conversion when both flags
are set (-nea alone and -ov alone unchanged), and convert_jd_to_bjd
defensively coerces string coordinates through radec_hours_to_degree so
any future path delivering strings converts correctly or fails loudly
instead of silently shifting the time base. The log tell for affected
runs: 'barycorrpy JDUTC_to_BJDTDB conversion failed; falling back to
astropy light-travel-time conversion.'
@opusbuilds

Copy link
Copy Markdown
Author

Re-verified against the current tip (d295d8c, 4.4.0), per the always-test-latest rule: the bug is still present there. Repro on the tip: string coordinates crash barycorrpy (TypeError), the astropy fallback engages and mis-parses hours as degrees — "19:27:06.50" vs its numeric equivalent differ by −198.7 s of BJD_TDB for the CoRoT-2 geometry. The branch cherry-picks onto d295d8c cleanly (no conflicts; verified locally, acceptance test passes: string-vs-numeric agree to 0.000 s). Happy to rebase the PR itself onto the tip if that's preferred.

@opusbuilds

Copy link
Copy Markdown
Author

Re-verified on the current tip 9100cd3 just now (my previous comment cited d295d8c, which was two days stale — my error, corrected). The bug is unchanged there: string coordinates crash barycorrpy with TypeError: can't multiply sequence by non-int of type 'float', the astropy fallback engages, and "19:27:06.50" vs its numeric equivalent still differ by −198.7 s of BJD_TDB for the CoRoT-2 geometry.

The branch cherry-picks onto 9100cd3 cleanly (no conflicts), and the acceptance test passes on the resulting build (4.3.2.dev120): string and numeric agree to 0.000 s. Happy to rebase the PR onto the tip if you'd rather review it that way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant