Bring OU-II to parity with OU-III's adaptation and startup policy - #298
Conversation
OU-II and OU-III differ in one thing that matters: the translational state
structure. OU-III carries an extra integral-displacement state and regularizes
it with a single r_S; OU-II regularizes p and v separately. The auto-tuner, the
wave-period estimator, the private attitude observer and the magnetic
acquisition are shared machinery that does not know which estimator it is
driving, and that machinery had drifted by four changes.
The variance channel now runs in a JONSWAP-similar band. sigma_aw is estimated
from the same exogenous levelled acceleration the wave-period estimator uses,
after a band-pass whose corners are fixed multiples of the tuner's own wave
frequency, so the transfer shape is fixed in f/f_tune away from the safety
clamps. The bench noise floor is referred through that band's own time-varying
coefficients rather than subtracted as a broadband constant.
The pseudo-measurement cadence is self-similar in tau. One pseudo update has
covariance r^2, so at one update per T_S the information rate goes as
1/(r^2 T_S), and scaling T_S with tau while holding r fixed would change the
regularization strength with sea state as a side effect. T_S = (0.015/1.1) tau
clamped to [5, 250] ms, with both filter inputs renormalized by sqrt(T_0/T_S).
p and v fire on one tick, so one cadence and one factor serve both. The
renormalization is deliberately not re-clamped.
Startup defaults to MahonyProxy. The measurement-only front end runs from the
first sample through updateFrontEnd() with the MEKF untouched, the private
Mahony observer supplies the tilt that gates the magnetometer and frames the
world-reference average, and goLive() seeds the MEKF so it starts live in one
step. Magnetic acquisition runs twice: a provisional lock on the old schedule
and a refinement at 90 s framed on the observer, with accelerometer-bias
learning held until it lands. StagedMekf restores the previous path.
Three things that were not obvious. The proxy needs its integral term on --
measured, a 0.05 deg/s bias settles at 0.711 deg of tilt at two_ki = 0 and at
zero with the deployed 0.02, and nothing high-passes an attitude seed. The
handoff needs a timeout, held clear of the magnetic acquisition it would
otherwise cut short. And the seed needs an anisotropic attitude covariance,
which is a new Kalman3D_Wave_OU_II::initialize_from_attitude().
Two of OU-III's r_S changes are not carried over, for stated reasons.
The floor change has no counterpart. OU-III's 0.4 floor was the binding
constraint on every low-motion sea; OU-II's are not. At the smallest calibrated
operating point the schedule asks for r_p0 = 0.314 m against a 0.05 floor and
r_v0 = 0.497 m/s against 0.01, clear by 6.3x and 50x, and still clear at the
near-still Hs = 0.05 m stress case. Lowering them would weaken a guard that
costs nothing. regularizer_floor-test pins that against the operating points the
scored records actually produce, and OU_II_R_P0_MIN/OU_II_R_V0_MIN now expose
the clamps so the question can be re-measured rather than re-argued.
The RSAdaptationLaw ablation is not ported: it exists to resolve an amplitude
exponent by measurement, it confirmed the deployed law, and there is no OU-II
analogue of the pole-placement derivation the Riccati laws come from.
Separately, the residual accelerometer-bias model was already at parity -- same
first-order Gauss-Markov residual about a temperature-calibrated mean, same
5000 s correlation time, same exact discrete covariance and the same setters.
What was missing is the other half of that argument: set_accel_bias_limit() and
the projection onto a ball after every state injection. Mean reversion bounds
the bias in distribution, not pathwise, and the bias enters the ISS bound only
as an input, so that input has to be bounded for the bound to say anything.
Measured on five IMU seeds across the eight scored records, n = 40 paired
records per metric, 900 s window, deployed against the pre-change filter:
roll RMS 0.354 -> 0.286 deg -19.3% (2 SE: 0.044)
accel-bias 3D % 82.94 -> 70.14 -15.4% (2 SE: 12.0)
accel-bias 3D RMS 0.0595 -> 0.0520 -12.7%
yaw RMS 2.488 -> 2.411 deg -3.1% (2 SE: 0.037)
3D RMS % 18.997 -> 19.042 +0.24% n.s.
Z RMS %Hs 6.318 -> 6.350 +0.50% n.s.
pitch RMS 0.269 -> 0.300 deg +11.8% (2 SE: 0.015)
Displacement accuracy does not move, which is the expected shape for a change
to the attitude front end and to how sigma_aw is measured. The attitude and
bias gains come almost entirely from the startup policy. The cadence is a
measurable no-op -- every metric moves by under 0.05% -- and by construction,
since the renormalization holds r^2 T_S fixed; it is ported for consistency of
policy, not for a gain it does not produce.
Pitch is the one real cost and it comes from the sigma band. Roll falls further
than pitch rises in absolute terms, so combined tilt RMS improves 6.7%, from
0.444 to 0.414 deg, with yaw 3.1% better on top. The redistribution between the
horizontal axes is not symmetric because the records are not: all eight carry a
fixed +/-30 deg wave direction. Flagged rather than averaged away.
The regression sentinels are re-derived on the filter that now ships. Three
tighten, two loosen. Both that loosen are realization moves rather than quality
regressions and the ensemble is what establishes it: the accelerometer-bias
aggregate improves 15% while its single-realization sentinel gets 8% worse, and
3D displacement error does not move at all. Said out loud at FAIL_LIMITS,
because raising a sentinel to admit one's own change is how these stop meaning
anything.
Full record in docs/ou-ii-ou-iii-parity.md.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FRGdQMgrMXko3rVh4SCQY9
|
Warning Review limit reached
Next review available in: 53 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a2a786de8d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (proxy_init || stage_ != Stage::Uninitialized) { | ||
| if (proxy_init && stage_ != Stage::Live) { | ||
| // Bootstrap: front end only, MEKF held. | ||
| impl_.updateFrontEnd(dt, gyro_body_ned, acc_body_ned); |
There was a problem hiding this comment.
Reset the proxy observer when beginning again
When begin() is called a second time on the same wrapper, the outer startup state and MEKF are reset, but initialize() does not reset vertical_accel_comp_, its learned integral gyro bias, or the tuner/front-end state. The default proxy path therefore continues the previous run's observer here, reports it initialized immediately, and can hand that stale attitude and operating point to the new MEKF. Reset the measurement-only tracking state as part of begin()/initialization before advancing the proxy.
Useful? React with 👍 / 👎.
| ? cfg_.proxy_mag_settle_sec + | ||
| 2.0f * std::max(cfg_.mag_min_window_sec, 1.0f) + | ||
| cfg_.mag_tilt_fallback_sec |
There was a problem hiding this comment.
Include the magnetometer delay in the handoff deadline
When mag_delay_sec exceeds this computed deadline and proxy_startup_timeout_sec is configured lower, updateMag() rejects every sample until the delay expires, but the timeout still hands off at this earlier deadline without a reference or yaw gauge. For example, a 100 s mag delay and 20 s proxy timeout with the other defaults goes live around 60 s even though magnetic acquisition cannot start until 100 s, contradicting the stated guarantee that the timeout cannot cut acquisition short. Base the deadline on at least max(mag_delay_sec, proxy_mag_settle_sec) before adding the acquisition window.
Useful? React with 👍 / 👎.
OU-II and OU-III differ in one thing that matters: the translational state
structure. OU-III carries an extra integral-displacement state and regularizes
it with a single r_S; OU-II regularizes p and v separately. The auto-tuner, the
wave-period estimator, the private attitude observer and the magnetic
acquisition are shared machinery that does not know which estimator it is
driving, and that machinery had drifted by four changes.
The variance channel now runs in a JONSWAP-similar band. sigma_aw is estimated
from the same exogenous levelled acceleration the wave-period estimator uses,
after a band-pass whose corners are fixed multiples of the tuner's own wave
frequency, so the transfer shape is fixed in f/f_tune away from the safety
clamps. The bench noise floor is referred through that band's own time-varying
coefficients rather than subtracted as a broadband constant.
The pseudo-measurement cadence is self-similar in tau. One pseudo update has
covariance r^2, so at one update per T_S the information rate goes as
1/(r^2 T_S), and scaling T_S with tau while holding r fixed would change the
regularization strength with sea state as a side effect. T_S = (0.015/1.1) tau
clamped to [5, 250] ms, with both filter inputs renormalized by sqrt(T_0/T_S).
p and v fire on one tick, so one cadence and one factor serve both. The
renormalization is deliberately not re-clamped.
Startup defaults to MahonyProxy. The measurement-only front end runs from the
first sample through updateFrontEnd() with the MEKF untouched, the private
Mahony observer supplies the tilt that gates the magnetometer and frames the
world-reference average, and goLive() seeds the MEKF so it starts live in one
step. Magnetic acquisition runs twice: a provisional lock on the old schedule
and a refinement at 90 s framed on the observer, with accelerometer-bias
learning held until it lands. StagedMekf restores the previous path.
Three things that were not obvious. The proxy needs its integral term on --
measured, a 0.05 deg/s bias settles at 0.711 deg of tilt at two_ki = 0 and at
zero with the deployed 0.02, and nothing high-passes an attitude seed. The
handoff needs a timeout, held clear of the magnetic acquisition it would
otherwise cut short. And the seed needs an anisotropic attitude covariance,
which is a new Kalman3D_Wave_OU_II::initialize_from_attitude().
Two of OU-III's r_S changes are not carried over, for stated reasons.
The floor change has no counterpart. OU-III's 0.4 floor was the binding
constraint on every low-motion sea; OU-II's are not. At the smallest calibrated
operating point the schedule asks for r_p0 = 0.314 m against a 0.05 floor and
r_v0 = 0.497 m/s against 0.01, clear by 6.3x and 50x, and still clear at the
near-still Hs = 0.05 m stress case. Lowering them would weaken a guard that
costs nothing. regularizer_floor-test pins that against the operating points the
scored records actually produce, and OU_II_R_P0_MIN/OU_II_R_V0_MIN now expose
the clamps so the question can be re-measured rather than re-argued.
The RSAdaptationLaw ablation is not ported: it exists to resolve an amplitude
exponent by measurement, it confirmed the deployed law, and there is no OU-II
analogue of the pole-placement derivation the Riccati laws come from.
Separately, the residual accelerometer-bias model was already at parity -- same
first-order Gauss-Markov residual about a temperature-calibrated mean, same
5000 s correlation time, same exact discrete covariance and the same setters.
What was missing is the other half of that argument: set_accel_bias_limit() and
the projection onto a ball after every state injection. Mean reversion bounds
the bias in distribution, not pathwise, and the bias enters the ISS bound only
as an input, so that input has to be bounded for the bound to say anything.
Measured on five IMU seeds across the eight scored records, n = 40 paired
records per metric, 900 s window, deployed against the pre-change filter:
Displacement accuracy does not move, which is the expected shape for a change
to the attitude front end and to how sigma_aw is measured. The attitude and
bias gains come almost entirely from the startup policy. The cadence is a
measurable no-op -- every metric moves by under 0.05% -- and by construction,
since the renormalization holds r^2 T_S fixed; it is ported for consistency of
policy, not for a gain it does not produce.
Pitch is the one real cost and it comes from the sigma band. Roll falls further
than pitch rises in absolute terms, so combined tilt RMS improves 6.7%, from
0.444 to 0.414 deg, with yaw 3.1% better on top. The redistribution between the
horizontal axes is not symmetric because the records are not: all eight carry a
fixed +/-30 deg wave direction. Flagged rather than averaged away.
The regression sentinels are re-derived on the filter that now ships. Three
tighten, two loosen. Both that loosen are realization moves rather than quality
regressions and the ensemble is what establishes it: the accelerometer-bias
aggregate improves 15% while its single-realization sentinel gets 8% worse, and
3D displacement error does not move at all. Said out loud at FAIL_LIMITS,
because raising a sentinel to admit one's own change is how these stop meaning
anything.
Full record in docs/ou-ii-ou-iii-parity.md.
Co-Authored-By: Claude Opus 5 noreply@anthropic.com
Claude-Session: https://claude.ai/code/session_01FRGdQMgrMXko3rVh4SCQY9