From eade44a245409a337ccdf0bfdff8f67c0d5aff10 Mon Sep 17 00:00:00 2001 From: cinnaboot Date: Sun, 26 Apr 2026 12:34:11 -0400 Subject: [PATCH] resolve burn timing quantization: sub-step interpolation for TRIGGER_TIME, update docs - Remove TODO from simulation.cpp (interpolated burns now implemented) - Update technical_reference.md: both trigger types now use sub-step propagation to exact trigger time, not step boundary - Update hohmann-rendezvous-quantization-fix.md: mark Option A as resolved, remove outdated quantization fix suggestions - Update propagation_refactor.md: resolve Issues 3 and 4, replace interpolated time triggers TODO with code path unification TODO --- .../hohmann-rendezvous-quantization-fix.md | 54 +++++++------------ docs/planning/propagation_refactor.md | 40 +++++++------- docs/technical_reference.md | 17 +++--- src/simulation.cpp | 5 -- 4 files changed, 47 insertions(+), 69 deletions(-) diff --git a/docs/planning/hohmann-rendezvous-quantization-fix.md b/docs/planning/hohmann-rendezvous-quantization-fix.md index 3f50995..c1c0322 100644 --- a/docs/planning/hohmann-rendezvous-quantization-fix.md +++ b/docs/planning/hohmann-rendezvous-quantization-fix.md @@ -88,52 +88,44 @@ original measurement exactly. - `rendezvous_hohmann` was renamed to `rendezvous` (CW module removed, only Hohmann remains) - All 154 remaining test cases pass (240,445 assertions) -## Current Code Path (After 2026-04-20 Refactoring) +## Current Code Path (After 2026-04-26 Sub-step Interpolation) -The maneuver trigger check and execution are now merged into `update_spacecraft_physics()`: +The maneuver trigger check and execution are merged into `update_spacecraft_physics()`. Both trigger types now use sub-step interpolation: ```cpp // In update_spacecraft_physics(), per spacecraft: check_maneuver_trigger(maneuver, craft, sim); -// → For TRIGGER_TIME: returns true if sim->time >= trigger_value -// → Sets scheduled_dt = 0 (never set for time triggers) +// → For TRIGGER_TIME: computes dt_to_burn = trigger_value - sim->time +// → For TRIGGER_TRUE_ANOMALY: computes dt_needed from mean anomaly delta +// → Both set scheduled_dt = dt_to_burn (0 to sim->dt) if (maneuver_fired) { craft->orbit = propagate_orbital_elements(craft->orbit, burn_dt, ...); - // burn_dt = 0 for TRIGGER_TIME → no-propagate execute_maneuver(fired_maneuver, ...); craft->orbit = propagate_orbital_elements(craft->orbit, remaining_dt, ...); - // remaining_dt = sim->dt } else { craft->orbit = propagate_orbital_elements(craft->orbit, sim->dt, ...); } ``` -**For `TRIGGER_TIME`:** `burn_dt` is always 0, so the sequence is: -1. `propagate_orbital_elements(craft->orbit, 0, ...)` → no-op -2. `execute_maneuver()` → burn applies at current position -3. `propagate_orbital_elements(craft->orbit, sim->dt, ...)` → propagate full step +**For `TRIGGER_TIME`:** `burn_dt` is the exact sub-step offset from step start to trigger time. The spacecraft propagates to the precise trigger position before the burn, then continues for `sim->dt - burn_dt`. **For `TRIGGER_TRUE_ANOMALY`:** `burn_dt` is set to exact seconds-to-target by `check_maneuver_trigger()`, so the spacecraft propagates to the exact burn position before the burn executes. -## Suggested Fixes for Burn Timing Quantization +**Edge case:** If `sim->time > trigger_value` (trigger passed in a previous step), `scheduled_dt` is clamped to 0 and the burn fires immediately at the current position. -### Option A: Sub-step Interpolation (Recommended) -**Approach:** When a burn trigger is detected between steps, propagate to the exact trigger time before executing. +## Burn Timing Quantization — RESOLVED (2026-04-26) -**Changes needed:** -1. In `check_maneuver_trigger()` for `TRIGGER_TIME`: - - When `sim->time >= trigger_value`, calculate `dt_to_burn = trigger_value - (sim->time - sim->dt)` - - Set `maneuver->scheduled_dt = dt_to_burn` - - Return `true` +**Option A (Sub-step Interpolation)** is now implemented for both `TRIGGER_TIME` and `TRIGGER_TRUE_ANOMALY`. + +**Implementation:** +- `check_maneuver_trigger()` computes `dt_to_burn = trigger_value - sim->time` for time triggers +- `update_spacecraft_physics()` propagates to exact burn time, executes burn, propagates remainder +- Quantization error is eliminated: burns execute at the precise trigger time -2. In `update_spacecraft_physics()`: - - When `burn_dt > 0`, propagate the spacecraft to the exact burn time - - Execute the burn - - Propagate the remaining `sim->dt - burn_dt` +**Edge case:** When `sim->time > trigger_value` (trigger passed in a previous step), `scheduled_dt` is clamped to 0 and the burn fires immediately at the current position. -**Pros:** Exact timing, no analytical drift -**Cons:** More complex, requires careful handling of edge cases +### Remaining Options (No Longer Needed) ### Option B: Snap Trigger Times to Step Boundaries **Approach:** In `calculate_next_hohmann_wait_time()`, snap the calculated wait time to the nearest step boundary. @@ -235,11 +227,6 @@ Based on Phase 2 & 3 results, recommend: ## Remaining Work -### Burn Timing Quantization (Optional - future) -1. **Option A**: Implement sub-step interpolation in `check_maneuver_trigger()` for `TRIGGER_TIME` — set `scheduled_dt = trigger_value - (sim->time - sim->dt)` for exact placement -2. **Option B**: Snap trigger times to step boundaries in `calculate_next_hohmann_wait_time()` -3. **Option C**: Accept current behavior and set realistic thresholds based on DT - ### DT Sweep Tests (COMPLETED - 2026-04-20) Measured in `test_maneuver_planning.cpp` via `DT sweep: Hohmann transfer separation vs time step`: @@ -263,8 +250,7 @@ Additional tests: ### Threshold Recommendation -### Threshold Recommendation -Based on DT sweep results, recommend: -- Maximum DT for rendezvous operations -- Separation threshold as a function of DT -- Whether sub-step interpolation is necessary for production use +With sub-step interpolation implemented, burn timing quantization is eliminated. DT sweep results now reflect integration error rather than quantization error. Recommend: +- Maximum DT for rendezvous operations based on integration accuracy +- Separation threshold set by orbital dynamics, not quantization bounds +- Sub-step interpolation is now active for both trigger types diff --git a/docs/planning/propagation_refactor.md b/docs/planning/propagation_refactor.md index 7480116..04031f0 100644 --- a/docs/planning/propagation_refactor.md +++ b/docs/planning/propagation_refactor.md @@ -116,11 +116,11 @@ The function previously performed two distinct responsibilities: ### Issue 3: Ambiguous `scheduled_dt` Semantics -**Status:** PARTIALLY RESOLVED — semantics are now clearer but still differ by trigger type. +**Status:** RESOLVED — both trigger types now set `scheduled_dt` to the exact sub-step offset. | Trigger Type | `scheduled_dt` meaning | Set by | |-------------|----------------------|--------| -| `TRIGGER_TIME` | Always 0.0 (never set) | Never | +| `TRIGGER_TIME` | `trigger_value - sim->time` (step start) | `check_maneuver_trigger()` | | `TRIGGER_TRUE_ANOMALY` | Seconds until exact burn position | `check_maneuver_trigger()` | In `update_spacecraft_physics()`: @@ -133,26 +133,28 @@ if (maneuver_fired) { } ``` -For `TRIGGER_TIME`, `burn_dt == 0` is still a coincidence — the field is never set. The branching is now explicit via `maneuver_fired` flag, which is clearer. +Both trigger types now produce the same `scheduled_dt` semantics: seconds from step start until the burn executes. The only difference is how that value is computed (simple subtraction vs. mean anomaly propagation). ### Issue 4: Time-Triggered Burns Propagate from Wrong State -**Status:** UNRESOLVED — root cause of burn timing quantization. +**Status:** RESOLVED — sub-step interpolation implemented (2026-04-26). -For time triggers, the sequence is: +For time triggers, the sequence is now: ``` Frame N: sim->time = 310.0 (trigger_value = 305.0) - check: 310 >= 305 → true - burn_dt = 0 - remaining_dt = 10 - execute_maneuver() → burn applies at sim->time=310 - propagate craft by 10s starting from sim->time=310 + check: sim->time >= trigger_value → true + dt_to_burn = trigger_value - (sim->time - sim->dt) = 305 - 300 = 5.0 + burn_dt = 5.0 + remaining_dt = 5.0 + propagate(5.0) → craft at trigger position + execute_maneuver() → burn applies at sim->time=305 + propagate(5.0) → continue simulation ``` -The craft's orbit state is at `sim->time=310`, not at the trigger time `305`. The burn fires 5s late and the post-burn propagation starts from the wrong orbital position. This is the root cause of the burn timing quantization problem documented in `docs/planning/hohmann-rendezvous-quantization-fix.md`. +The spacecraft propagates to the exact trigger position before the burn, then continues for the remaining timestep. This eliminates the burn timing quantization problem documented in `docs/planning/hohmann-rendezvous-quantization-fix.md`. -**TODO in code:** `update_spacecraft_physics()` has a TODO suggesting `dt_to_burn = trigger_value - (sim->time - sim->dt)` for exact placement. +**Edge case:** When `sim->time > trigger_value` (trigger passed in a previous step), `scheduled_dt` is clamped to 0 and the burn fires immediately at the current position. ### Issue 5: Hardcoded Array Size @@ -200,18 +202,12 @@ Remaining: 4 call sites (1 for bodies, 3 for spacecraft), 2 distinct contexts fo ## Remaining Work -### Interpolated Time Triggers (TODO in code) - -For `TRIGGER_TIME`, `scheduled_dt` is always 0, so the burn applies at `sim->time` (step boundary). To achieve exact placement: - -1. In `check_maneuver_trigger()` for `TRIGGER_TIME`: compute `dt_to_burn = trigger_value - (sim->time - sim->dt)` when `sim->time >= trigger_value` -2. Set `scheduled_dt = dt_to_burn` (positive value, 0 to `sim->dt`) -3. The propagation loop then does: `propagate(dt_to_burn)` → burn → `propagate(remaining)` - -This would place the burn at the exact trigger time, not the step boundary. See `docs/planning/hohmann-rendezvous-quantization-fix.md` for quantization analysis. - ### Parabolic/Hyperbolic Orbit Support (TODO in code) The analytical `dt_needed` calculation in `check_maneuver_trigger()` only handles elliptical orbits. TODO comment added for: - Parabolic: Barker's equation `D + D³/3 = M`, where `D = tan(ν/2)` - Hyperbolic: `e·sinh(H) - H = M`, using hyperbolic anomaly `H` + +### Code Path Unification (TODO in maneuver.cpp) + +A TODO was added to combine the TRIGGER_TIME and TRIGGER_TRUE_ANOMALY branches in `check_maneuver_trigger()` into a single code path. Both now compute `scheduled_dt` the same way (sub-step offset), so the logic can be unified. The main complication is maneuver array ordering: when an earlier maneuver fires and changes the orbit, cached trigger times for later maneuvers become stale and must be recomputed. diff --git a/docs/technical_reference.md b/docs/technical_reference.md index c00ab18..ea2b277 100644 --- a/docs/technical_reference.md +++ b/docs/technical_reference.md @@ -131,16 +131,17 @@ Sequence: argument_of_periapsis (ω) → inclination (i) → longitude_of_ascend - BURN_CUSTOM: user-specified vector ### Exact Position Execution -True anomaly triggers use analytical mean anomaly delta to compute exact time to target, eliminating per-frame propagation probes: +Both trigger types use sub-step propagation to execute burns at the exact trigger time, eliminating quantization error: -1. `check_maneuver_trigger()` converts current and target true anomaly to mean anomaly, computes delta-M, divides by mean motion to get `dt_needed` -2. If `0 < dt_needed <= sim->dt`, trigger fires and `scheduled_dt` is set -3. In `update_spacecraft_physics()`, for each spacecraft: check all pending maneuvers for that craft -4. If a maneuver fires: propagate by `burn_dt` (scheduled_dt), execute burn, propagate remaining (`sim->dt - burn_dt`) -5. No separate maneuver execution step — all inline in the spacecraft propagation loop +1. **TRIGGER_TIME**: `check_maneuver_trigger()` computes `dt_to_burn = trigger_value - sim->time` (sim->time is step start). If the trigger falls within `[sim->time, sim->time + sim->dt]`, `scheduled_dt` is set to this offset. +2. **TRIGGER_TRUE_ANOMALY**: `check_maneuver_trigger()` converts current and target true anomaly to mean anomaly, computes delta-M, divides by mean motion to get `dt_needed`. +3. If `0 < scheduled_dt <= sim->dt`, trigger fires and `scheduled_dt` is set. +4. In `update_spacecraft_physics()`, for each spacecraft: check all pending maneuvers for that craft. +5. If a maneuver fires: propagate by `burn_dt` (scheduled_dt), execute burn, propagate remaining (`sim->dt - burn_dt`). +6. No separate maneuver execution step — all inline in the spacecraft propagation loop. -**TRIGGER_TIME**: `scheduled_dt` is always 0 (burn at step boundary, quantization error in [0, DT)). -**TRIGGER_TRUE_ANOMALY**: Sub-step timing supported via analytical mean anomaly calculation. +**TRIGGER_TIME**: `scheduled_dt` is 0 only when the trigger time has already passed (clamped to fire immediately). Otherwise it is the exact sub-step offset from step start. +**TRIGGER_TRUE_ANOMALY**: Sub-step timing via analytical mean anomaly calculation. **Future TODO**: Parabolic (Barker's equation) and hyperbolic branches for `check_maneuver_trigger()`. diff --git a/src/simulation.cpp b/src/simulation.cpp index 146cb96..d10ce13 100644 --- a/src/simulation.cpp +++ b/src/simulation.cpp @@ -301,11 +301,6 @@ void update_spacecraft_physics(SimulationState* sim) { // to the burn time, execute the burn, then propagate the remaining // timestep. This enables sub-step interpolation for true-anomaly // triggers and sets up future interpolated time triggers. - // TODO: Test interpolated burns for TRIGGER_TIME — currently only - // TRIGGER_TRUE_ANOMALY provides sub-step timing via scheduled_dt. - // For time triggers, scheduled_dt is always 0, so the burn applies - // at sim->time (step boundary). Could compute dt_to_burn = - // trigger_value - (sim->time - sim->dt) for exact placement. bool maneuver_fired = false; double burn_dt = 0.0; Maneuver* fired_maneuver = NULL;