docs(pipeline): record the yaw-leak localization and R-IEKF validation#427
Merged
Conversation
Updates the #212 candidate decomposition with this session's real-data findings from obs_inspect (#212/#337): - Yaw null-space leak: confirmed per-update on real V2_03 AND V1_01 (translation structurally protected, yaw leaks on 100% of updates, grows with clone drift); R-IEKF validated as the fix (yaw leak 0.39 -> 2.3e-16 on the same real clone-window perturbations). - Update algebra: cleared on real data too (consistent leak ~1e-15); notes the s6 "non-PSD" was an inspector-predicate artifact, since fixed. - Measurement noise R: reframed as innovation-level only — accepted-update NIS is healthy (0.28); the over-confidence is a state-NEES property pointing back at the yaw gauge. - Isotropic P0 / init scale: ruled out as the e2e lever (init error washes out; ATE unchanged when the misfiring dynamic init is suppressed). Adds the obs_inspect diagnostic to the program section and an R-IEKF (#347/#348) tracking-index row. Relates to #212, #337, #347, #348 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughUpdates ChangesEngineering Status Documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs-site/src/content/docs/pipeline/engineering-status.mdx`:
- Line 42: The yaw leak measurements (0.39, 619, 2.3e-16) and translation leak
assertions in the engineering-status.mdx document lack unit or dimension
specifications, making it unclear whether they represent dimensionless norms,
radians, degrees, or another quantity. Add clarification either inline or via a
footnote that explains the physical interpretation of these leak
values—specifically, confirm whether they are the norm of the null-space leak
(‖H·N‖) computed as `sqrt(sum ...)` on matrix products, and specify the units
(e.g., dimensionless, radians, degrees). Apply this same clarification to both
line 42 and line 81 to ensure consistent context throughout the document.
- Line 42: The yaw leak value reporting is inconsistent between line 42 and line
81 in the engineering-status.mdx file. Line 42 explicitly states "mean 0.39 /
max 619" with clear qualifiers, but line 81 presents the same finding as "yaw
leak 0.39 → 2.3e-16" without specifying that 0.39 is the mean value, creating
potential confusion about whether it represents the average or peak. Update line
81 to explicitly include the "mean" qualifier by changing "yaw leak 0.39 →
2.3e-16" to "mean yaw leak 0.39 → 2.3e-16" to maintain consistency and precision
with line 42's reporting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 08d86914-07ed-417a-93cf-852768d2f004
📒 Files selected for processing (1)
docs-site/src/content/docs/pipeline/engineering-status.mdx
- State that all leak figures are the dimensionless gauge-leak norm ‖H·N‖. - Line ~81: "mean yaw leak 0.39 → 2.3e-16" to match the candidate table's mean/max. Relates to #212 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs-site/src/content/docs/pipeline/engineering-status.mdx`:
- Line 42: The phrase "leak ≡ 0" on line 42 in the translation-leak description
overstates the guarantee as mathematically exact, but the actual test assertions
in observability_inspect.cpp use a tolerance of 1e-9 and the real-data
measurements show values around 1e-15. Change the text "translation is
structurally protected (leak ≡ 0)" to use approximate equality like "leak ≈ 0"
or a tolerance bound "leak < 1e-9" to accurately reflect that the leak is
negligible within measurement precision rather than claiming a hard algebraic
invariant.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: dbeb8d09-6cf4-4c55-8837-00795cd162ac
📒 Files selected for processing (1)
docs-site/src/content/docs/pipeline/engineering-status.mdx
| | Candidate | Stage | Status | | ||
| |---|---|---| | ||
| | **Yaw null-space leak** (observability) | [S2](/pipeline/stages/s2-imu-propagation/)/[S6](/pipeline/stages/s6-msckf-update/) | **Leading, measured** — the `observability_probe` (#337) localizes the structural NEES over-confidence to the **yaw** gauge: the camera Jacobian annihilates the 4-DoF null space at one consistent linearization (‖H·N‖_yaw = 2e-16) but leaks (0.15 at σ=2 cm) at the evolving estimate. Matches the measured **attitude-NEES ≈ 993** on slow V1_01 (NIS a healthy 1.5) | | ||
| | **Yaw null-space leak** (observability) | [S2](/pipeline/stages/s2-imu-propagation/)/[S6](/pipeline/stages/s6-msckf-update/) | **Confirmed on real data; R-IEKF validated as the fix.** The `observability_probe` (#337) localized this synthetically; `obs_inspect` (#212) now measures it **per-update on real EuRoC**, driving the *shipped* Jacobian over the live clone window. (All leak figures here are the **dimensionless gauge-leak norm ‖H·N‖** — the residual measurement information the stacked Jacobian projects onto the unobservable gauge; 0 = perfectly annihilated.) On both **V2_03** and **V1_01** the consistent leak is ~1e-15 (the production Jacobian is correct), **translation is structurally protected** (leak ≡ 0), and the **yaw leak is nonzero on 100% of updates and grows with the clone window's attitude drift** (σθ → 5.6°) — exactly the structural NEES over-confidence (attitude-NEES ≈ 993 on V1_01, NIS a healthy 1.5). At the filter's *own* claimed clone σ the standard yaw leak is mean 0.39 / max 619; the **right-invariant (R-IEKF) parameterization on the same perturbations leaks 2.3e-16** (estimate-independent gauge) — the real-data evidence that R-IEKF is the fix | |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify translation leak values from obs_inspect runs on real EuRoC.
# Expected: Either strict zero (≡ 0) or negligible value range (< 1e-10).
# Search for obs_inspect output or test assertions on real data translation leak
rg -i 'translation.*leak' --type=cpp --type=json -C2 tools/src/obs_inspect.cpp tools/Repository: branes-ai/cortex
Length of output: 1525
🏁 Script executed:
cat -n docs-site/src/content/docs/pipeline/engineering-status.mdx | sed -n '35,50p'Repository: branes-ai/cortex
Length of output: 5497
🏁 Script executed:
rg -i 'translation leak' --type=cpp -A5 -B5 tools/Repository: branes-ai/cortex
Length of output: 2094
🏁 Script executed:
fd -i "obs_inspect" -type f | head -20Repository: branes-ai/cortex
Length of output: 231
🏁 Script executed:
find . -type f \( -name "*.json" -o -name "*.jsonl" -o -name "*obs*" -o -name "*leak*" \) | grep -E "(test|out|result|example)" | head -20Repository: branes-ai/cortex
Length of output: 136
🏁 Script executed:
rg 'translation.*leak|leak.*translation' --type=hpp --type=cpp -B2 -A2 | head -50Repository: branes-ai/cortex
Length of output: 88
🏁 Script executed:
rg -i 'obs_inspect.*output\|translation.*value\|leak.*[0-9]' tools/ docs/ --max-count=20Repository: branes-ai/cortex
Length of output: 42
🏁 Script executed:
wc -l tests/tools/observability_inspect.cpp tests/sdk/observability_probe.cppRepository: branes-ai/cortex
Length of output: 153
🏁 Script executed:
cat -n tests/tools/observability_inspect.cpp | head -100Repository: branes-ai/cortex
Length of output: 5150
Soften the translation-leak phrasing to match measurement tolerance.
Line 42 uses "leak ≡ 0" (mathematically exact), but test assertions (tests/tools/observability_inspect.cpp lines 64, 95–96) pin translation leak to WithinAbs(0.0, 1e-9), and the real-data obs_inspect measurement (documented on line 49 as ~1e-15 on V2_03/V1_01) confirms the leak is negligible, not structurally zero by construction. Change "translation is structurally protected (leak ≡ 0)" to "leak ≈ 0" or "leak < 1e-9" to align with the measured tolerance and avoid overstating an unobservable-by-measurement-precision guarantee as a hard algebraic invariant.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs-site/src/content/docs/pipeline/engineering-status.mdx` at line 42, The
phrase "leak ≡ 0" on line 42 in the translation-leak description overstates the
guarantee as mathematically exact, but the actual test assertions in
observability_inspect.cpp use a tolerance of 1e-9 and the real-data measurements
show values around 1e-15. Change the text "translation is structurally protected
(leak ≡ 0)" to use approximate equality like "leak ≈ 0" or a tolerance bound
"leak < 1e-9" to accurately reflect that the leak is negligible within
measurement precision rather than claiming a hard algebraic invariant.
Contributor
Author
|
Addressed both CodeRabbit nitpicks in 201e7c5:
|
…deRabbit) CodeRabbit read "leak ≡ 0" as overstated vs the test's 1e-9 tolerance, but that tolerance / the ~1e-15 figure is the YAW consistent leak. The TRANSLATION leak is exactly 0 by bit-for-bit ±Hf cancellation (obs_inspect prints 0, not 1e-15). Spell that out so the exact-zero (translation) vs ~1e-15 (yaw) distinction is unambiguous. Relates to #212 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Author
|
Addressed in a191463: clarified the wording. To be precise — the |
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
1 participant
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.
Summary
Records this session's real-data findings on the Engineering Status page — the #212 candidate decomposition now reflects what
obs_inspectmeasured on real EuRoC.Changes to the candidate table
R→ reframed as innovation-level only; accepted-update NIS is healthy (0.28). Over-confidence is a state-NEES property → points back at the yaw gauge.obs_inspectto the diagnostic-program section + an R-IEKF (epic(sdk): Right-Invariant EKF (R-IEKF) backend — fix the #212 yaw observability leak by construction #347/spike(sdk): R-IEKF Phase A — invariant propagation + observability probe goes flat (#347) #348) tracking-index row.Test plan
Relates to #212, #337, #347, #348
🤖 Generated with Claude Code
Summary by CodeRabbit