Repository navigation
Chapter 8's accelerometer is body-frame, and its covariance stays positive - #125
Merged
Merged
Conversation
…itive
Two defects, both found by the ION GNSS+ pre-conference audit, and one of
them moved every number in the chapter.
**The shipped accelerometer was map-frame.** `imu.npz/accel_xy` was
`np.gradient(v_xy)/dt` written out unrotated, while `core/fusion/tc_models.py`
rotates `u = [ax, ay, .]` by the state yaw before adding it to the map-frame
velocity -- so every Chapter 8 filter rotated it a second time. This is
precisely the defect Chapter 6's strapdown generator had, fixed in an earlier
session; `tests/ch6_dead_reckoning/test_imu_is_body_frame.py` explained the
signature and its hand-written `DATASETS` tuple had never covered Chapter 8.
The tell was the same one that file records: the residual against map-frame
acceleration was [0.1002, 0.1001] m/s^2, indistinguishable from the declared
0.1 noise, while against body frame it carried a systematic [-0.017, -0.096].
The generator now rotates into body frame. All three datasets were
regenerated from the recipe `tests/test_datasets_reproduce_from_their_recipe.py`
records, and **only `accel_xy` moved** -- `truth`, `gyro_z`, `uwb_ranges` and
`uwb_anchors` are identical to the last bit, which is the check that the RNG
stream was not disturbed. Attribution was established first, per CLAUDE.md:
with the edit stashed, a clean-tree regeneration reproduced the shipped bytes
exactly, so the diff is provably this change.
One config field also moved and is **pre-existing**, verified the same way:
`ch8_fusion_2d_imu_uwb_timeoffset/config.json` recorded
`uwb.nlos_bias_m: 0.5` where `--preset time_offset_50ms` produces 0.0. It was
inert (`nlos_anchors` is empty, so no range ever carried it) and invisible
because the reproduction test compares arrays only -- `_arrays()` reads
`.txt/.npy/.npz` and never `.json`. A sweep of all 20 datasets found no other
substantive instance; the four other diffs are last-ulp floats, a `nan != nan`
artifact, and a cosmetic `preset` field, so no repo-wide config guard is added
here.
**P inflation after S made the covariance indefinite.**
`AdaptiveGatingManager` doubles P after S has been formed for the chi-square
gate, and both runners then built the gain from that stale S and applied the
short form `(I - K H) P`, which equals the Joseph form only at the optimal
gain. Off it the variance along H scales by `1 - lambda*HPH'/(HPH' + R)`,
negative once `lambda*HPH' > HPH' + R`. Measured on the corrected data with
the short form still in place: LC on the NLOS dataset reached
min trace(P) = -0.0123 (11/6587 samples) and TC -2.10e+07 (600/8271). It was
drawn in the covariance panel of `lc_uwb_imu_results.png` and nothing objected.
`core.fusion.tuning.kalman_update` is the fix and is deliberately shaped so the
defect cannot recur: it derives S from the covariance it is handed, so a stale
S is not expressible, and it uses the Joseph form, which is a sum of two
congruence transforms and therefore PSD at any gain. All five hand-rolled
update sites now share it -- both runners plus `example_robust_tuning` and
`example_temporal_calibration`, which had correct copies (no P inflation, so
the gain was optimal) and were the fourth and fifth copies of the idiom that
had just failed twice. Verified inert on those two: the LOS baseline is
0.020130 m and the clean residual tail is unchanged to every digit.
Guards, both shown red before green:
- The frame test now discovers its datasets by scanning `data/sim` for an
accelerometer rather than listing them, so the next one is covered without
anyone remembering. Red on the old bytes at 0.3208 std against a 0.15 bound.
It also gained a second trajectory assertion, because the existing one is
**blind here**: the broken rectangular walk integrates to an extent ratio of
1.00: it was the circle in Chapter 6 that made the extent check bite. Median
deviation from the true path separates them, 1.02 against 0.14 normalised.
- `tests/ch8_sensor_fusion/test_covariance_stays_positive_semidefinite.py`
covers both runners on both datasets, asserts the inflation branch still
fires on NLOS (it fires zero times on the baseline now, so a baseline-only
guard would be vacuous), and pins the mechanism without a dataset.
Everything downstream was re-measured rather than adjusted. Both sides of each
comparison were run this session:
before after
TC RMSE 0.167 m 0.025 m
LC RMSE 0.143 m 0.027 m
TC mean |yaw err| 70.89 deg 2.23 deg
TC mean NIS 1.71 1.08 (expected 1)
TC norm. innov std 1.307 1.037
TC innov autocorr -0.52 / +0.63 / +0.46 -> -0.01 / +0.08 / +0.07 (lags 2/4/8)
Three headline claims reversed, and each is recorded in place rather than
quietly replaced:
- **The anchor-outage mirror-branch flip does not happen.** TC was reported
jumping to (30.1, -35.5) against a truth of (20.0, 7.2), a 43.9 m peak that
put its whole-run RMSE above LC's. Two ranges to two collinear anchors really
are ambiguous; what broke the tie was the IMU prediction, which was wrong
too. TC now peaks at 0.06 m and wins at all five windows checked. The test
that pinned the flip is rewritten to fail if it returns.
- **Chi-square gating is the best method, not the worst.** It was written up as
"collapsing" to 24-26 m in every scenario through a starvation feedback loop.
The gate is the only strategy here that tests its input against a
distributional assumption, so it failed loudest under bad data and looked
like the culprit. It now accepts 95% of a clean run -- exactly its nominal
confidence -- and takes the NLOS run from 0.680 m to 0.033 m.
- **The "LC NIS 1.93 vs Q-tuning" open item is re-attributed.** TC's mean NIS
is 1.14; roughly four-fifths of the excess was the frame, not the process
noise.
The robust thresholds were re-decided by measurement, keeping the three-scenario
acceptance frame. The clean-data tail collapsed (99th percentile 14.51 -> 2.73
sigma, maximum 17.23 -> 3.76), so the old justification for delta = 20 -- "just
above the measured maximum" -- now gives 4. Swept against all three scenarios,
delta = 4 costs 10.0% on persistent NLOS; delta = 10 is the smallest value that
leaves clean data untouched, improves sporadic by 81%, and does no harm on
NLOS. Cauchy moves 50 -> 20 by the same rule. Why an aggressive threshold hurts
is now measured rather than asserted: with no inlier majority the filter is
dragged to a compromise where every anchor's residual inflates, and the loss
down-weights honest links at 35.7% and 37.5% against 39.0% and 28.3% for the
biased pair.
Also corrected while re-pasting: the TC mode table (all four rows, and a "Best"
bold on a row that is not), the extrinsic-alignment residual (0.05 m against
this same README's own correct 0.10 m derivation two sections above), the
temporal-calibration gain (12.5% -> 62.7%, with the kinematic bound unchanged
and now explained as the durable claim), "TC ~10-20% better" (measured 8%),
the NLOS gating table, and `example_comparison`'s trade-off sentence, whose
guard tested `max_error` while the sentence also asserted something about
`mean_error` -- true today, and false on the data shipped when it was written.
`docs/ch8_comparison_tool_summary.md` was an empty 3-byte file that
`docs/README.md` advertised as documentation; both it and its references are
gone.
Nine README transcripts re-pasted from real runs and checked by
`tests/docs/test_readme_example_output.py`. Six figure sets regenerated and
every changed PNG opened: the covariance panels now decay monotonically and
never cross zero, and the anchor-outage caption -- which asserted the branch
flip unconditionally -- is derived from the data so it can stop asserting it.
`example_observability` and `example_calibration` figures are untouched, which
is correct: neither reads the accelerometer.
Review fixes on the kalman_update helper the blocker fix introduced. Three guards caught three real debts: the docstring cited Eq. (3.31), which errata E-03 records as absent from the book -- the Joseph form is the correction of Eq. (3.19), and the citation now says so and points at E-01. The semantic-API ratchet counted the single-letter parameters and the bare tuple return; the signature is now descriptive with the book symbols mapped inside per docs/api_naming_conventions.md, and the result is a NamedTuple that unpacks exactly as before, so the five call sites are untouched. The lint ratchet then caught the import my NamedTuple addition left unsorted. Also on the record: two of my own edit steps failed silently because their replacements carried no assertion, and the fix shipped in pieces until the asserted version landed. The rule that every mechanical replace asserts its anchor exists is not decoration.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes the two Chapter 8 BLOCKERs from the ION GNSS+ pre-conference audit.
BLOCKER 1 — the shipped accelerometer was map-frame
imu.npz/accel_xywasnp.gradient(v_xy)/dtwritten out unrotated, whilecore/fusion/tc_models.pyrotatesu = [ax, ay, .]by the state yaw beforeadding it to map-frame velocity — so every Chapter 8 filter rotated it twice.
Same defect Chapter 6's strapdown generator had; that chapter's guard listed
its two datasets by hand and had never covered Chapter 8.
accel_xyvsAll three datasets regenerated from their recorded recipes. Only
accel_xymoved —
truth,gyro_z,uwb_ranges,uwb_anchorsbit-identical, so theRNG stream is undisturbed. Attribution established first per CLAUDE.md: with
the edit stashed, a clean-tree regeneration reproduced the shipped bytes
exactly.
BLOCKER 2 — P inflation after S made the covariance indefinite
AdaptiveGatingManagerdoubles P after S is formed for the gate; bothrunners then built the gain from that stale S and applied the short form
(I − K H) P, valid only at the optimal gain. Measured on the corrected datawith the short form still in place:
core.fusion.tuning.kalman_updatederives S from the covariance it is handed(a stale S is not expressible) and uses the Joseph form (PSD at any gain). All
five hand-rolled sites now share it; the two in examples were correct copies
and are verified numerically inert.
Downstream — everything re-measured, both sides run this session
Three headline claims reversed, each recorded in place rather than quietly
replaced:
excursion was real, but what should have broken the two-fold ambiguity was
the IMU prediction — which was also wrong. TC now peaks at 0.06 m and wins
at all five windows checked. The test that pinned the flip now fails if it
returns.
as "collapsing" to 24–26 m through a starvation loop. It is the only
strategy here that tests its input against a distributional assumption, so
it failed loudest under bad data and looked like the culprit. It now accepts
95% of a clean run — its nominal confidence — and takes NLOS 0.680 → 0.033 m.
is 1.14, so ~4/5 of the excess was the frame, not the process noise.
Robust thresholds re-decided by measurement, keeping the three-scenario
acceptance frame (clean not degraded / sporadic improved / persistent-NLOS
untouched). The clean tail collapsed (p99 14.51 → 2.73σ, max 17.23 → 3.76σ),
so the old "just above the measured maximum" rule now gives 4 — but δ=4 costs
10.0% on NLOS. δ=10 is the smallest value meeting all three criteria; Cauchy
50 → 20 by the same rule.
Guards, red before green
rather than listing them. Red on the old bytes (std 0.3208 vs 0.15 bound).
It also gained a second trajectory assertion, because the existing one is
blind here — the broken rectangular walk integrates to an extent ratio of
1.00; only the circle in Chapter 6 made that check bite.
test_covariance_stays_positive_semidefinite.py: both runners, bothdatasets, plus a guard-the-guard that the inflation branch still fires on
NLOS (it fires zero times on the baseline now, so a baseline-only test
would be vacuous).
Verification (all foreground)
Nine transcripts re-pasted from real runs. Six figure sets regenerated and
every changed PNG opened — covariance panels now decay monotonically and never
cross zero.
example_observabilityandexample_calibrationfigures areuntouched, which is correct: neither reads the accelerometer.
Also fixed
The audit's MAJOR falsehoods, each re-measured on the fixed data: the TC mode
table (all four rows and a wrong "Best" bold), the extrinsic-alignment
residual (0.05 m against this same README's own correct 0.10 m derivation two
sections above), temporal-calibration gain 12.5% → 62.7% (kinematic bound
unchanged), "TC ~10-20% better" → measured 8%, the NLOS gating table, and
example_comparison's trade-off sentence — whose guard testedmax_errorwhile the sentence also asserted something about
mean_error.docs/ch8_comparison_tool_summary.mdwas an empty 3-byte file thatdocs/README.mdadvertised as documentation; it and its references are gone.Suspected issues, not fixed here
AdaptiveGatingManagerharms persistent NLOS.example_tc_fusionmeasures 1.469 m with adaptive gating against 0.690 m ungated, because the
manager reads permanent bias as its own mis-tuning: it inflates R on 97.5%
of updates to its 5× ceiling, widening the gate until the biased ranges pass
again. A plain 95% gate reaches 0.033 m on the same data. Design question,
not a blocker.
Metrics Comparisonbar panels inlc_tc_comparisonandtemporal_calibration_demoplot metres beside percentages on one linearaxis, so the RMSE bars render at zero height. Pre-existing.
core/fusion/tuning.pyis not in the doctest allowlist; two of itsdocstrings (
cauchy_R_scale,huber_weight) fail as pre-existing drift.The new
kalman_updatedoctest passes.