Add an accepted column so a usable fit reads as a predicate

docs/fit_flags.rst

@@ -1,16 +1,20 @@
 Orbit fit status
 ========================================================================================
 
-Every fit that ``layup orbitfit`` produces reports its outcome in six columns: a
-single-value summary, ``flag``, and five columns that report the individual facts
-behind it.
+Every fit that ``layup orbitfit`` produces reports its outcome in seven columns: a
+single-value summary, ``flag``, a plain ``accepted`` predicate, and five columns
+that report the individual facts behind them.
 
 .. list-table::
    :header-rows: 1
    :widths: 22 78
 
    * - Column
      - Meaning
+   * - ``accepted``
+     - ``1`` if the fit is usable: it converged and passed every check. The same
+       thing ``flag == 0`` says, as a predicate you can filter on without knowing
+       the flag values.
    * - ``flag``
      - Summary. ``0`` if and only if the fit converged and passed every check.
    * - ``converged``
@@ -23,7 +27,11 @@ behind it.
        the acceptance threshold.
    * - ``failed_cov``
      - ``1`` if the fit converged but its covariance is degenerate, or a variance is
-       non-positive.
+       non-positive. This check runs **only when non-gravitational parameters are
+       being fitted**, where it detects a non-gravitational term that has become
+       collinear with the state. A gravity-only fit is never marked here, so a
+       column of zeros does not mean the covariances were examined and found
+       sound.
    * - ``failed_physical``
      - ``1`` if the fit converged but describes an orbit no real object could occupy:
        its hyperbolic excess speed is implausibly large.
@@ -35,9 +43,9 @@ matches ``flag == 0``.
 Which column should I filter on?
 ----------------------------------------------------------------------------------------
 
-**For orbits you intend to use, filter on** ``flag == 0``. That is the summary, it
-means the fit converged and every check passed, and it is what the rest of ``layup``
-uses internally.
+**For orbits you intend to use, filter on** ``accepted == 1``, or equivalently
+``flag == 0``. Both mean the fit converged and every check passed, and ``flag == 0``
+is what the rest of ``layup`` uses internally.
 
 Read the other columns when you need to know *why* something was rejected — for
 triage, for diagnosing a survey's failure modes, or to accept fits that failed a

src/layup/constants.py

@@ -100,7 +100,13 @@
 # is zero across all of them, matching ``flag == FLAG_CONVERGED``. A ``passed_*``
 # convention would make a never-attempted fit (all zero) indistinguishable from
 # one that failed everything.
-OUTCOME_COLUMNS = ("converged", "stage", "failed_csq", "failed_cov", "failed_physical")
+#
+# ``accepted`` is the exception, and the one to filter on: it is the summary
+# verdict as a predicate, 1 exactly when ``flag == FLAG_CONVERGED``. It reads
+# without knowing the flag taxonomy, and unlike an ``error``/``success`` rename
+# it does not label -1 (never attempted) or 8 (incremental bookkeeping) as
+# errors, which they are not (issue #498).
+OUTCOME_COLUMNS = ("accepted", "converged", "stage", "failed_csq", "failed_cov", "failed_physical")
 
 # Which check each of the fitter's own post-convergence verdicts reports as
 # failed. Both are set *after* the Levenberg-Marquardt loop converges, so each

src/layup/orbitfit.py

@@ -742,9 +742,16 @@ def record(self, fit):
         if gate is not None:
             setattr(self, gate, True)
 
-    def as_row(self):
-        """The output columns, in ``OUTCOME_COLUMNS`` order."""
+    def as_row(self, flag):
+        """The output columns, in ``OUTCOME_COLUMNS`` order.
+
+        ``accepted`` is derived from the flag the row actually carries rather
+        than tracked alongside these facts, so the two cannot disagree. It is
+        taken last, after any stage marker has overwritten the fitter's own
+        verdict, because that overwritten value is what a reader will see.
+        """
         return (
+            int(flag == FLAG_CONVERGED),
             int(self.converged),
             int(self.stage),
             int(self.failed_csq),
@@ -807,7 +814,7 @@ def create_empty_result(id, dtypes):
             )
             + (np.nan,) * 36  # Flat covariance matrix
             # never attempted: not converged, no stage reached, no gate applied
-            + ((0, STAGE_NOT_ATTEMPTED, 0, 0, 0) if "converged" in dtypes.names else ())
+            + (FitOutcome().as_row(FLAG_NOT_ATTEMPTED) if "converged" in dtypes.names else ())
             # non-grav columns (issue #351): NaN per a1/a2/a3 (+ _unc) that is present
             + tuple(np.nan for n in ("a1", "a2", "a3") if n in dtypes.names for _ in (0, 1))
             # obs fingerprint (issue #419): empty hash never matches, so a failed
@@ -1571,7 +1578,7 @@ def _orbitfit(
                     ("BCART_EQ" if success else "NONE"),  # The base format returned by the C++ code
                 )
                 + cov_matrix  # Flat covariance matrix
-                + outcome.as_row()  # the outcome columns
+                + outcome.as_row(res.flag)  # the outcome columns
                 + nongrav_cols  # non-grav params + uncertainties (issue #351), when fit_nongrav
                 + per_arc_cols  # later-arc amplitudes (comet linkage), when per_arc
                 + (obs_hash, nobs_fit)  # obs fingerprint (issue #419)
@@ -1951,7 +1958,7 @@ def _fitresult_to_row(fit, obj_id, obs_hash, nobs_fit, dtypes):
         )
         + cov
         # The sequential update does not run the staged pipeline either.
-        + (FitOutcome.from_flag(fit.flag).as_row() if "converged" in dtypes.names else ())
+        + (FitOutcome.from_flag(fit.flag).as_row(fit.flag) if "converged" in dtypes.names else ())
         + (obs_hash, nobs_fit)
     )
     return np.array([row], dtype=dtypes)

tests/layup/test_accepted_column.py

@@ -0,0 +1,123 @@
+"""One column that says whether a fit is usable, without reading the taxonomy.
+
+Reviewers on #495 asked for a status column that reads as a predicate --
+``if (!error)`` or ``if (success)`` -- rather than an integer to be compared
+against a documented list. The two are not interchangeable: ``0`` means
+converged today, so ``!error`` works with the values unchanged while ``success``
+is false for every good fit unless the values also invert. And renaming ``flag``
+is breaking: it appears across three modules, sixteen test files and seven
+fixture CSVs that carry it as a column header, so output already written stops
+loading.
+
+``accepted`` is the alternative from #498: keep ``flag``, add a derived boolean.
+It reads the same, inverts nothing, and does not label ``-1`` (never attempted)
+or ``8`` (incremental bookkeeping) as errors, which they are not.
+
+It is derived from the flag the row actually carries, not tracked beside the
+other outcome facts, so the two cannot disagree -- and it is read *after* any
+stage marker has overwritten the fitter's own verdict, because the overwritten
+value is what a reader sees.
+"""
+
+import numpy as np
+import pytest
+
+import layup.orbitfit as orbitfit
+from layup.constants import (
+    FLAG_BUILDUP_FAILED,
+    FLAG_CONVERGED,
+    FLAG_CSQ_TOO_LARGE,
+    FLAG_DEGENERATE_COV,
+    FLAG_DID_NOT_CONVERGE,
+    FLAG_IMPLAUSIBLE_ORBIT,
+    FLAG_INCREMENTAL_NO_FULL_OBS,
+    FLAG_NO_ROOT_CONVERGED,
+    FLAG_NO_SOLUTION,
+    FLAG_NOT_ATTEMPTED,
+    OUTCOME_COLUMNS,
+    STAGE_COMPLETE,
+)
+from layup.orbitfit import FitOutcome
+
+ACCEPTED = OUTCOME_COLUMNS.index("accepted")
+
+# Every documented flag other than the accepting one.
+REJECTING_FLAGS = [
+    FLAG_NOT_ATTEMPTED,
+    FLAG_DID_NOT_CONVERGE,
+    FLAG_CSQ_TOO_LARGE,
+    FLAG_NO_ROOT_CONVERGED,
+    FLAG_BUILDUP_FAILED,
+    FLAG_NO_SOLUTION,
+    FLAG_DEGENERATE_COV,
+    FLAG_INCREMENTAL_NO_FULL_OBS,
+    FLAG_IMPLAUSIBLE_ORBIT,
+]
+
+
+def test_accepted_is_the_first_outcome_column():
+    """It is the one to filter on, so it leads the group rather than sitting
+    among the individual verdicts."""
+    assert OUTCOME_COLUMNS[0] == "accepted"
+
+
+def test_accepted_is_one_exactly_for_the_converged_flag():
+    row = FitOutcome(converged=True, stage=STAGE_COMPLETE).as_row(FLAG_CONVERGED)
+    assert row[ACCEPTED] == 1
+
+
+@pytest.mark.parametrize("flag", REJECTING_FLAGS)
+def test_every_other_flag_is_not_accepted(flag):
+    """A single value means usable and it is the only one. Anything else -- a
+    rejection, a stage marker, or the never-attempted sentinel -- is not."""
+    row = FitOutcome.from_flag(flag).as_row(flag)
+    assert row[ACCEPTED] == 0, f"flag {flag} should not read as accepted"
+
+
+def test_accepted_follows_the_flag_the_row_carries_not_the_fitters_verdict():
+    """The case #499 is about: a fit converged, was rejected on chi-square, and
+    then had its flag overwritten by a stage marker. The row carries 4, so it is
+    not accepted -- even though the outcome facts still record that it converged.
+    Deriving from the flag is what keeps the two consistent."""
+    outcome = FitOutcome(converged=True, stage=STAGE_COMPLETE, failed_csq=True)
+    row = outcome.as_row(FLAG_BUILDUP_FAILED)
+    assert row[ACCEPTED] == 0
+    assert row[OUTCOME_COLUMNS.index("converged")] == 1
+    assert row[OUTCOME_COLUMNS.index("failed_csq")] == 1
+
+
+def test_accepted_is_a_plain_int_like_the_other_columns():
+    """The row goes straight into an i1 column; a bool would be stored without
+    complaint."""
+    row = FitOutcome(converged=True).as_row(FLAG_CONVERGED)
+    assert all(isinstance(v, int) for v in row)
+    assert len(row) == len(OUTCOME_COLUMNS)
+
+
+def test_never_attempted_is_not_accepted_and_not_a_failure():
+    """The reason #498 preferred this over renaming flag to ``error``: a row that
+    was never fit is not an error, it is an absence. It reads as not accepted,
+    with no check marked failed."""
+    row = orbitfit.create_empty_result("x", orbitfit._get_result_dtypes("ObjID", []))
+    assert row["accepted"][0] == 0
+    assert row["flag"][0] == FLAG_NOT_ATTEMPTED
+    assert row["failed_csq"][0] == 0
+    assert row["failed_cov"][0] == 0
+    assert row["failed_physical"][0] == 0
+
+
+def test_accepted_agrees_with_the_flag_across_the_whole_taxonomy():
+    """The column exists so that callers stop writing ``flag == 0`` by hand. If
+    the two ever disagree, the column is worse than nothing."""
+    for flag in [FLAG_CONVERGED] + REJECTING_FLAGS:
+        row = FitOutcome.from_flag(flag).as_row(flag)
+        assert row[ACCEPTED] == int(flag == FLAG_CONVERGED), f"disagreement at flag {flag}"
+
+
+def test_the_column_is_in_the_output_dtype():
+    dt = orbitfit._get_result_dtypes("ObjID", [])
+    assert "accepted" in dt.names
+    assert np.dtype(dt["accepted"]).kind == "i"
+    # It stays inside the outcome group, ahead of the fingerprint columns.
+    assert dt.names.index("flag") < dt.names.index("accepted")
+    assert dt.names[-2:] == ("obs_hash", "nobs_fit")

tests/layup/test_fit_outcome.py

@@ -176,10 +176,11 @@ def test_as_row_is_ints_in_column_order():
     plain ints in OUTCOME_COLUMNS order. A bool or a reordering would be stored
     without complaint and silently mislabel every fit."""
     outcome = FitOutcome(converged=True, stage=STAGE_COMPLETE, failed_csq=True)
-    row = outcome.as_row()
+    row = outcome.as_row(FLAG_CSQ_TOO_LARGE)
     assert len(row) == len(OUTCOME_COLUMNS)
     assert all(isinstance(v, int) for v in row)
-    assert row == (1, STAGE_COMPLETE, 1, 0, 0)
+    # converged, then rejected on chi-square: accepted is 0 though converged is 1.
+    assert row == (0, 1, STAGE_COMPLETE, 1, 0, 0)
 
 
 # --------------------------------------------------------------------------