Skip to content

FitResult.cov is left uninitialised when a Cartesian fit returns flag != 0 #547

Description

@matthewholman

Summary

orbit_fit populates FitResult.cov only when the fit converges cleanly, but cov has no
initialiser, so every fit that returns flag != 0 hands back uninitialised stack memory as
its covariance
. bk_fit already does the right thing (result.cov.fill(0.0)), so the
Cartesian path is the outlier.

This is easy to miss because flag = 2 is not an error — it is a converged fit whose reduced
χ² exceeded thresh = 10.0 — so callers reasonably treat the result as usable and read cov.

Where

  • src/lib/orbit_fit/fit_result.cpp:21std::array<double, 36> cov;, no default member
    initialiser (same for state and root).
  • src/lib/orbit_fit/orbit_fit.cpp:1180FitResult result; is default-initialised, so
    cov is indeterminate.
  • src/lib/orbit_fit/orbit_fit.cpp:1545if (flag == 0) { ... result.cov[...] = cov(i, j); }
    is the only place it is written.

Reachable with flag = 1 (no convergence, orbit_fit.cpp:1180), flag = 2
(orbit_fit.cpp:1315, reduced χ² > 10) and flag = 6 (orbit_fit.cpp:1348, weakly
constrained / non-positive variance).

Contrast src/lib/orbit_fit/bk_fit.cpp:42, which zeroes it up front:

result.cov.fill(0.0);

Observed

Same object, same input, same build, three separate processes — a flag = 2 fit on
(20) Massalia:

### numbered__00020: flag=2 csq=7.73354e+07 ndof=19978 red=3871.03
  cov finite: False   any NaN: True
  diagonal: [0.0e+000 4.9407e-324 6.9527e-310 0.0e+000 0.0e+000  nan]   run 1
  diagonal: [0.0e+000 4.9407e-324 6.9526e-310 0.0e+000 0.0e+000  nan]   run 2
  diagonal: [0.0e+000 4.9410e-324 6.9530e-310 0.0e+000 0.0e+000  nan]   run 3
  symmetric: True / False / True

6.95e-310 is a pointer value reinterpreted as a double and it moves with ASLR between
runs
; 4.94e-324 is the smallest subnormal. The array is not merely wrong, it differs every
execution, so anything downstream of it is irreproducible.

csq, ndof and state are identical across the three runs — the fit itself is
deterministic. Only cov varies.

Why it bites

Any consumer that reads fit.cov after a flag != 0 result. Concretely, an OrbFit-style
outlier-rejection loop (Carpino, Milani & Chesley 2003) computes a per-observation χ² on the
residual covariance Γ_res = Γ_obs ∓ B Γ_x Bᵀ, taking Γ_x from fit.cov. With cov
indeterminate, every reject/recover decision taken while the fit sits at flag = 2 is made on
garbage — and a NaN silently defeats the usual det <= 0 sanity check, because every
comparison against NaN is false.

Measured over a full 1.51 M-object MPC catalogue fit (455 run logs, 1,506,746 object blocks):
5,562 objects (0.37%) took at least one rejection iteration at flag != 0, and 5,311 of
them moved their active observation set on it. Guarding the covariance changes the outcome for
836 of the 1,366 objects that ended at flag = 2 — they converge at median reduced χ² 0.25
instead, and agree with MPCORB at median rel|da| 1.5e-7.

Suggested fix

One line, matching bk_fit:

FitResult result;
result.cov.fill(0.0);   // and result.state, result.root

or give the members default initialisers in fit_result.cpp so every FitResult is
zero-initialised regardless of construction site.

Zeroing is safe for the rejection use above (Γ_res degrades to Γ_obs, i.e. no projection)
and is unambiguous to test for, whereas indeterminate values are not. Worth documenting on the
pybind11 binding too: cov is only meaningful when flag == 0.

Version

Reproduced on main @ b6d7edd, and on the 2026-07 run build (eea7d3b + #411/#416/#412).
Unaffected by #536.

Surfaced while re-fitting the objects that did not reach flag=0 in the full-MPC
catalogue run.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions