From 0d218c7f6095716503fb7abf694e741ed9eff929 Mon Sep 17 00:00:00 2001
From: matthewholman <mholman@cfa.harvard.edu>
Date: Fri, 19 Jun 2026 21:51:05 -0400
Subject: [PATCH] Warn when an orbit fit fails on a sub-24h arc (#312)

Orbit determination needs an observational baseline longer than ~24 hours.
When a fit fails on a single night of data the user previously got an opaque
failure. Now, on a failed fit, _orbitfit checks the observation arc span and,
if it is under a day, logs a clear warning naming the object and the (too
short) arc length so the likely cause is obvious.

Implemented as a small _warn_if_short_arc helper (threshold _MIN_ARC_DAYS =
1.0 day) called where the fit result is known to have failed. Adds unit tests
for the sub-day (warns), multi-day (silent), and empty (no crash) cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 src/layup/orbitfit.py         | 27 +++++++++++++++++++++++++++
 tests/layup/test_orbit_fit.py | 30 +++++++++++++++++++++++++++++-
 2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/src/layup/orbitfit.py b/src/layup/orbitfit.py
index afab1dbe..354d6c94 100644
--- a/src/layup/orbitfit.py
+++ b/src/layup/orbitfit.py
@@ -449,6 +449,31 @@ def do_other_fit(iod: str):
     raise ValueError(f"The IOD, {iod} is not supported. Please use a supported IOD.")
 
 
+# Minimum observational arc (in days) generally needed to constrain an orbit.
+# Below this the fit is essentially unconstrained, so a failure is most likely a
+# too-short baseline rather than anything wrong with the data.
+_MIN_ARC_DAYS = 1.0
+
+
+def _warn_if_short_arc(jds, obj_id):
+    """Emit a helpful warning when a failed fit is likely caused by too short an
+    observational arc (less than ~24 hours / a single night).
+
+    See issue #312: an orbit fit needs a baseline of more than 24 hours, so when
+    a fit fails on a sub-day arc we tell the user the likely cause rather than
+    leaving them with an opaque failure.
+    """
+    if jds is None or len(jds) == 0:
+        return
+    arc_days = float(np.max(jds) - np.min(jds))
+    if arc_days < _MIN_ARC_DAYS:
+        logger.warning(
+            f"Orbit fit failed for {obj_id}: the observations span only "
+            f"{arc_days * 24.0:.1f} hours.  Constraining an orbit generally requires "
+            f"a baseline of more than ~24 hours (more than a single night of observations)."
+        )
+
+
 def _orbitfit(
     data,
     cache_dir: str,
@@ -598,6 +623,8 @@ def _orbitfit(
             res = run_from_vector_with_initial_guess(get_ephem(kernels_loc), guess_to_use, observations)
         # Populate our output structured array with the orbit fit results
         success = res.flag == 0
+        if not success:
+            _warn_if_short_arc(jds, data[primary_id_column_name][0])
         cov_matrix = tuple(res.cov[i] for i in range(36)) if success else (np.nan,) * 36
         output = np.array(
             [
diff --git a/tests/layup/test_orbit_fit.py b/tests/layup/test_orbit_fit.py
index 733375dc..94a164e6 100644
--- a/tests/layup/test_orbit_fit.py
+++ b/tests/layup/test_orbit_fit.py
@@ -1,3 +1,4 @@
+import logging
 import os
 
 import numpy as np
@@ -5,7 +6,7 @@
 import pytest
 from numpy.testing import assert_equal
 
-from layup.orbitfit import orbitfit, orbitfit_cli
+from layup.orbitfit import _MIN_ARC_DAYS, _warn_if_short_arc, orbitfit, orbitfit_cli
 from layup.routines import Observation, get_ephem, run_from_vector_with_initial_guess, FitResult, gauss
 from layup.utilities.data_processing_utilities import get_cov_columns, parse_cov, parse_fit_result
 from layup.utilities.data_utilities_for_tests import get_test_filepath
@@ -263,3 +264,30 @@ def __init__(self, g=None):
         )
 
         assert "The IOD, bad_iod is not supported" in str(e.value)
+
+
+def test_warn_if_short_arc_warns_for_subday_arc(caplog):
+    """Issue #312: a failed fit on a sub-24h arc should log a clear warning
+    naming the object and the (too-short) arc length."""
+    jds = np.array([2460000.0, 2460000.2, 2460000.30])  # ~7.2 hour baseline
+    with caplog.at_level(logging.WARNING, logger="layup.orbitfit"):
+        _warn_if_short_arc(jds, "obj_short")
+    msgs = [r.getMessage() for r in caplog.records]
+    assert any("obj_short" in m and "hours" in m for m in msgs), msgs
+
+
+def test_warn_if_short_arc_silent_for_multiday_arc(caplog):
+    """A multi-day arc that fails for some other reason should not trigger the
+    short-arc warning (avoid misleading the user)."""
+    jds = np.array([2460000.0, 2460002.0, 2460005.0])  # 5 day baseline
+    with caplog.at_level(logging.WARNING, logger="layup.orbitfit"):
+        _warn_if_short_arc(jds, "obj_long")
+    assert _MIN_ARC_DAYS == 1.0
+    assert not caplog.records
+
+
+def test_warn_if_short_arc_handles_empty(caplog):
+    """No observations -> no crash, no message."""
+    with caplog.at_level(logging.WARNING, logger="layup.orbitfit"):
+        _warn_if_short_arc(np.array([]), "obj_empty")
+    assert not caplog.records