fix(timestamp): compute control dt in the integer domain (precision; attempted synth dodge) - #319
Merged
Merged
Conversation
…attempted synth dodge)
relay-rate, relay-ekf and relay-pos all computed their control period as
`time.as_secs_f32() - last_time.as_secs_f32()` — differencing two ABSOLUTE f32
epoch times. Replace with `Timestamp::secs_since_f32(earlier)`, which subtracts
in the integer domain and converts once.
PRIMARY WIN — precision. Subtracting two large absolute f32 times catastrophically
cancels: at t ~ 1e6 s an f32 ULP is ~0.06 s, which swamps a 1 ms control period.
Differencing in integers and converting the small result is strictly more
accurate, and the semantics ("elapsed since") are clearer than the subtraction.
Saturates at zero for non-monotonic input rather than wrapping/panicking (u64
underflow); callers clamp to their dt band regardless.
SECONDARY, AND IT DID NOT WORK — the synth GI-FPU-001 dodge. jess's per-stage
lowering sweep (jess#167) showed `f32.convert_i64_u` skipping the public entry
point of exactly the three stages that call this helper (rate#tick,
position#tick, ekf#estimate), and suggested narrowing the u64 to u32 to unblock
them without waiting on synth. The correlation was exact (relay-att defines
Timestamp but never calls as_secs_f32 — and attitude is blocked by the OTHER
defect, GI-FPU-002). But the op SURVIVES: four formulations (if/else cap,
u64::min, mask-truncate, early narrowing) all leave LLVM converting the u64 and
clamping in float domain. Verified by disassembling the built component after
each attempt, and by deleting as_secs_f32 entirely to rule it out — the op
persisted, so it is this expression, not the old helper. This commit therefore
does NOT unblock those stages; GI-FPU-001 looks genuinely synth-side.
Tests: 16 + 13 pass across the three crates, including
rate_p03_step_response_drives_error_to_zero (the closed-loop convergence test) —
behaviour preserved.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HvusAXYbHLyv3uTzfBcMbG
avrabe
enabled auto-merge (squash)
July 29, 2026 03:50
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.
What
relay-rate,relay-ekfandrelay-poseach computed their control period astime.as_secs_f32() - last_time.as_secs_f32()— differencing two absolute f32 epoch times. Replaced withTimestamp::secs_since_f32(earlier), which subtracts in the integer domain and converts once.Primary win — precision (this is why it should land)
Subtracting two large absolute f32 times catastrophically cancels: at t ≈ 1e6 s an f32 ULP is ~0.06 s, which swamps a 1 ms control period. Differencing in integers and converting the small result is strictly more accurate, and "elapsed since" states the intent better than the subtraction. Saturates at zero on non-monotonic input rather than wrapping (u64 underflow) or panicking; callers clamp to their dt band anyway.
Secondary — and it did not work: the synth GI-FPU-001 dodge
jess's per-stage lowering sweep (jess#167) found
f32.convert_i64_uskipping the public entry point of exactly the three stages that call this helper (rate#tick,position#tick,ekf#estimate), and suggested narrowing the u64→u32 to unblock them without waiting on synth.The diagnosis correlated exactly —
relay-attdefinesTimestampbut never callsas_secs_f32, and attitude is blocked by the other defect (GI-FPU-002, VFP exhaustion). But the op survives:if/elsecap atu32::MAXu64::min(..) as u32select+f32.convert_i64_u(whole & 0xFFFF_FFFF)Verified by disassembling the built component after each attempt (
wasm-tools print | grep f32.convert_i64), and by deletingas_secs_f32entirely to rule it out — the op persisted, proving it's this expression, not the old helper.So this PR does NOT unblock those stages. GI-FPU-001 looks genuinely synth-side. Reporting the negative result to jess so they don't wait on a source-level fix that isn't coming.
Tests
16 + 13 pass across the three crates, including
rate_p03_step_response_drives_error_to_zero(closed-loop convergence) — behaviour preserved.Refs: jess#167 (per-stage skip report, GI-FPU-001 / synth#369).
🤖 Generated with Claude Code