Skip to content

Add an accepted column so a usable fit reads as a predicate - #527

Open
matthewholman wants to merge 1 commit into
mainfrom
feat/498-accepted-column
Open

Add an accepted column so a usable fit reads as a predicate#527
matthewholman wants to merge 1 commit into
mainfrom
feat/498-accepted-column

Conversation

@matthewholman

Copy link
Copy Markdown
Collaborator

Closes #498.

Reviewers on #495 asked for a status column that reads as a predicate -- if (!error) or if (success) -- rather than an integer 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 -- seven fixture CSVs carry it as a column header, so output already written stops loading.

This is the alternative from #498 instead: keep flag, add a derived boolean. accepted is 1 exactly when flag == 0. 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 carries rather than tracked beside the other outcome facts, so the two cannot disagree, and it is taken after any stage marker has overwritten the fitter's own verdict -- that overwritten value is what a reader sees.

Two things came along with it:

  • create_empty_result hand-copied the outcome tuple, a second copy of the column layout that would have silently desynced. It now calls as_row.
  • Corrects the failed_cov entry in docs/fit_flags.rst. Both of its assignments sit inside the non-gravitational weak-constraint guard, so a gravity-only fit is never marked there. The page implied the covariance was examined for every fit, which would let a reader take a column of zeros as reassurance.

Reviewers on #495 asked for a status column that reads as `if (!error)` or
`if (success)` rather than an integer 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: seven fixture CSVs carry
it as a column header, so output already written stops loading.

This is #498's alternative instead: keep `flag`, add a derived boolean.
`accepted` is 1 exactly when `flag == FLAG_CONVERGED`. 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 carries rather than tracked beside the other
outcome facts, so the two cannot disagree, and it is taken after any stage marker
has overwritten the fitter's own verdict -- that overwritten value is what a
reader sees.

Also replaces the hand-copied outcome tuple in create_empty_result with a real
as_row call. It was a second copy of the column layout and would have silently
desynced; the existing tests caught it as a length mismatch.

Separately, corrects the failed_cov entry in docs/fit_flags.rst. Both of its
assignments sit inside the non-gravitational weak-constraint guard, so a
gravity-only fit is never marked there -- the page implied the covariance was
examined for every fit.
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.

Decide what the orbit fit status column is called before v1.0

1 participant