From 5e89ec8bb39d3a6b8474eb5052691e57914ca29d Mon Sep 17 00:00:00 2001 From: Theodore Omtzigt Date: Sat, 20 Jun 2026 16:55:25 -0400 Subject: [PATCH] fix(tools): correct s6_inspect PSD check and NIS headline The S6 inspector reported two misleading diagnostics on real EuRoC data, both measurement artifacts rather than filter faults: 1. psd_after used is_positive_definite (bare Cholesky), which fails on any zero eigenvalue. The MSCKF covariance is legitimately rank-deficient: the newest clone is a stochastic copy of the IMU pose (exact zero-covariance directions) and the 4-DoF gauge (yaw + global position) is unobservable, so those eigenvalues sit at 0 +/- machine-eps. Full-window updates nudged them slightly positive (Cholesky passed) while partial-window updates left them at +/-1e-17 (Cholesky tripped) -> a deterministic, spurious "100% non-PSD on partial tracks" signal (eigendecomposition: min eig -1.1e-17, 6 eigenvalues ~0). Switched to is_positive_semidefinite, the invariant the Joseph form actually guarantees; the non-PSD warning drops from 18% to 0 on MH_02. 2. The NIS/dof headline averaged over all valid updates including the chi2-gated outlier tail (rejected, never applied), reading ~4.0 and printing "over-confident (R under-modeled)". The applied (accepted) updates are consistent (accepted NIS/dof 0.28 on MH_02). Now reports accepted-only as the honest per-update metric and keeps all-valid as a labelled secondary. #212 system-level over-confidence is a state-NEES property measured end-to-end, not from these innovation residuals. Inspector gate test passes (gcc + clang); no SDK/backend change. Relates to #371, #380, #212 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../branes/tools/s6_update_inspect.hpp | 11 ++++++++-- tools/src/s6_inspect.cpp | 20 ++++++++++++++++--- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/tools/include/branes/tools/s6_update_inspect.hpp b/tools/include/branes/tools/s6_update_inspect.hpp index 39b742e..d0f6851 100644 --- a/tools/include/branes/tools/s6_update_inspect.hpp +++ b/tools/include/branes/tools/s6_update_inspect.hpp @@ -61,7 +61,8 @@ struct S6UpdateRecord { double cov_trace_before = 0.0; ///< tr(P) before the update double cov_trace_after = 0.0; ///< tr(P) after (== before if rejected) double cov_trace_ratio = 1.0; ///< after / before (<1 ⇒ uncertainty shrank) - bool psd_after = true; ///< P stayed positive-definite (Joseph guarantee) + bool psd_after = true; ///< P stayed positive-SEMI-definite (Joseph guarantee; the + ///< MSCKF gauge + stochastic-clone copy make P legitimately rank-deficient) double residual_rms_px = 0.0; ///< RMS reprojection residual at the landmark std::vector residuals; }; @@ -150,7 +151,13 @@ class S6UpdateInspector { const DynMat cov_after = state_after.covariance(); r.cov_trace_after = trace(cov_after); r.cov_trace_ratio = r.cov_trace_before > 0.0 ? r.cov_trace_after / r.cov_trace_before : 1.0; - r.psd_after = ms::is_positive_definite(cov_after); + // The MSCKF covariance is legitimately rank-deficient: the newest clone is a + // stochastic copy of the IMU pose (exact zero-covariance directions) and the + // 4-DoF gauge (yaw + global position) is unobservable. Those eigenvalues sit at + // 0 ± machine-eps, so a strict positive-DEFINITE (bare Cholesky) test reports + // false breakage on every partial-window update. Positive-SEMI-definite (small + // ridge) is the correct invariant the Joseph form actually guarantees. + r.psd_after = ms::is_positive_semidefinite(cov_after); // Per-observation reprojection residual at the triangulated landmark. branes::math::lie::detail::Vec p_f{}; diff --git a/tools/src/s6_inspect.cpp b/tools/src/s6_inspect.cpp index de49b51..47afc47 100644 --- a/tools/src/s6_inspect.cpp +++ b/tools/src/s6_inspect.cpp @@ -255,8 +255,10 @@ int main(int argc, char** argv) { double frame_t = 0.0; std::uint64_t n_updates = 0, n_accepted = 0, n_gated = 0, n_psd_fail = 0; - double sum_nis_over_dof = 0.0; + double sum_nis_over_dof = 0.0; // all valid updates (incl. χ²-gated outlier tail) std::size_t nis_count = 0; + double sum_nis_accepted = 0.0; // accepted updates only — the honest per-update consistency + std::size_t nis_accepted_count = 0; bool dumped = false; json cov_dump; @@ -281,6 +283,10 @@ int main(int argc, char** argv) { if (rec.valid && rec.dof > 0) { sum_nis_over_dof += rec.nis_over_dof; ++nis_count; + if (rec.accepted) { + sum_nis_accepted += rec.nis_over_dof; + ++nis_accepted_count; + } } // Dump one update's full covariance for the before/after heatmap. @@ -341,13 +347,21 @@ int main(int argc, char** argv) { } } + // Accepted-only NIS is the honest per-update consistency: the all-valid mean is + // dominated by the χ²-gated outlier tail (rejected, never applied), so it reads + // alarmingly high while the corrections the filter actually made are consistent. + // System-level over-confidence (#212) is a STATE-NEES-vs-ground-truth property and + // is measured end-to-end (vio_pipeline), not from these innovation residuals. const double mean_nod = nis_count ? sum_nis_over_dof / static_cast(nis_count) : 0.0; + const double mean_nod_acc = nis_accepted_count ? sum_nis_accepted / static_cast(nis_accepted_count) : 0.0; std::cout << "s6_inspect: processed " << processed << " frames"; if (skipped) std::cout << " (" << skipped << " unreadable, skipped)"; std::cout << "\n updates: " << n_updates << " (" << n_accepted << " accepted, " << n_gated << " χ²-gated)\n" - << " NIS/dof: mean " << mean_nod << " over " << nis_count << " valid updates" - << (mean_nod > 1.2 ? " → over-confident (R under-modeled)" : "") << "\n"; + << " NIS/dof: accepted " << mean_nod_acc << " over " << nis_accepted_count << " applied updates" + << (mean_nod_acc > 1.5 ? " → applied updates over-confident" : " → applied updates consistent") + << "\n all-valid " << mean_nod << " over " << nis_count + << " (incl. gated outlier tail — not the consistency metric)\n"; if (n_psd_fail) std::cout << " WARNING: " << n_psd_fail << " updates left P non-PSD\n"; std::cout << " records: " << upd_path << "\n";