diff --git a/docs/planning/propagation_refactor.md b/docs/planning/propagation_refactor.md index 4103e49..2234e0b 100644 --- a/docs/planning/propagation_refactor.md +++ b/docs/planning/propagation_refactor.md @@ -3,6 +3,27 @@ ## Session Date 2026-04-20 +## Recent Changes + +### 2026-04-20: Eliminated Redundant Propagation in `check_maneuver_trigger()` + +Replaced the per-frame propagation probe in `check_maneuver_trigger()` (true-anomaly branch) with a direct analytical calculation using mean anomaly delta. + +**What changed:** +- Removed `propagate_orbital_elements()` call from `check_maneuver_trigger()` — eliminates ~24k redundant Kepler solves per Hohmann transfer wait period +- Removed dead `angle_between()` static helper +- Replaced the `angle_between`, `future_diff`, and `wraparound_crossing` checks with a cleaner `dt_needed <= 0.0` guard +- Added TODO comment noting elliptical-orbits-only limitation + +**Impact:** +- Same correctness, fewer function calls, simpler logic +- All 154 tests pass (240,445 assertions) +- For a Hohmann transfer with ~244,000s wait time and DT=10s: ~24,400 fewer `propagate_orbital_elements()` calls + +**Remaining:** See Issue 1 below — still has a TODO for parabolic/hyperbolic orbit support. + +--- + ## Objective Audit all call sites of `propagate_orbital_elements()` for spacecraft and trace the `update_simulation()` call chain through `execute_pending_maneuvers()` and `update_spacecraft_physics()` to identify inefficiencies and confusing branching. @@ -52,16 +73,20 @@ update_simulation() ### Issue 1: Redundant Propagation for True-Anomaly Triggers -**Location:** `src/maneuver.cpp`, line 147 in `check_maneuver_trigger()` +**Status:** RESOLVED — eliminated in favor of analytical mean anomaly delta calculation. -For every frame that a true-anomaly maneuver is pending, `propagate_orbital_elements()` is called as a "look-ahead" probe to determine if the target angle is approaching. +**Location:** `src/maneuver.cpp`, `check_maneuver_trigger()` (true-anomaly branch) + +Previously, for every frame that a true-anomaly maneuver was pending, `propagate_orbital_elements()` was called as a "look-ahead" probe to determine if the target angle was approaching. ```cpp -// maneuver.cpp:147 +// OLD (maneuver.cpp:147) — REMOVED OrbitalElements future_elements = propagate_orbital_elements(craft->orbit, sim->dt, parent->mass); ``` -**Impact:** For a Hohmann transfer with a ~244,000s wait time and DT=10s, this results in ~24,400 redundant propagations — each one solving Kepler's equation — before the maneuver even fires. The spacecraft's orbit state is not modified, but the computational cost is paid every single frame. +**Resolution:** Replaced with direct analytical computation of `dt_needed` from mean anomaly delta. The analytical solution is more precise (no discretization error) and eliminates ~24k redundant Kepler solves per Hohmann transfer wait period at DT=10s. + +**Remaining:** The current implementation only handles elliptical orbits. TODO comment added for parabolic (Barker's equation) and hyperbolic branches. ### Issue 2: Mixed Concerns in `execute_pending_maneuvers()` @@ -153,6 +178,8 @@ When no sub-step is needed, the maneuver path is: propagate → convert → burn ## Call Sites Summary +### Before fix (5 call sites) + | Location | Function | Context | dt value | |----------|----------|---------|----------| | `simulation.cpp:287` | `update_bodies_physics()` | Normal body propagation | `sim->dt` | @@ -161,4 +188,13 @@ When no sub-step is needed, the maneuver path is: propagate → convert → burn | `simulation.cpp:350` | `execute_pending_maneuvers()` | Post-burn remaining propagation | `sim->dt - dt_to_burn` | | `maneuver.cpp:147` | `check_maneuver_trigger()` | True-anomaly look-ahead probe | `sim->dt` | -Total: 5 call sites, 3 distinct contexts (normal propagation, sub-step execution, probe). +### After fix (4 call sites) + +| Location | Function | Context | dt value | +|----------|----------|---------|----------| +| `simulation.cpp:287` | `update_bodies_physics()` | Normal body propagation | `sim->dt` | +| `simulation.cpp:315` | `update_spacecraft_physics()` | Normal craft propagation | `sim->dt` | +| `simulation.cpp:343` | `execute_pending_maneuvers()` | Pre-burn sub-step propagation | `dt_to_burn` (0 to `sim->dt`) | +| `simulation.cpp:350` | `execute_pending_maneuvers()` | Post-burn remaining propagation | `sim->dt - dt_to_burn` | + +Remaining: 4 call sites, 2 distinct contexts for spacecraft (normal propagation, sub-step execution). Issue 6 (duplicated propagation) remains. diff --git a/src/maneuver.cpp b/src/maneuver.cpp index 325e3da..20a66ff 100644 --- a/src/maneuver.cpp +++ b/src/maneuver.cpp @@ -110,20 +110,9 @@ OrbitalElements preview_burn_result(const Spacecraft* craft, BurnDirection direc return cartesian_to_orbital_elements(pos, new_vel, parent_mass); } -// Check if target angle lies between current and next angles (accounting for wraparound) -static bool angle_between(double current, double next, double target) { - double curr_norm = normalize_angle(current); - double next_norm = normalize_angle(next); - double target_norm = normalize_angle(target); - - if (curr_norm <= next_norm) { - return (target_norm >= curr_norm) && (target_norm <= next_norm); - } else { - return (target_norm >= curr_norm) || (target_norm <= next_norm); - } -} - -// FIXME: this function needs helpers for wraparound detection and Kepler's equation +// Elliptical orbits only: uses analytical mean anomaly delta to compute +// exact time to target true anomaly, eliminating per-frame propagation. +// TODO: add parabolic (Barker's equation) and hyperbolic branches. bool check_maneuver_trigger(Maneuver* maneuver, Spacecraft* craft, SimulationState* sim) { switch (maneuver->trigger_type) { case TRIGGER_TIME: @@ -144,23 +133,6 @@ bool check_maneuver_trigger(Maneuver* maneuver, Spacecraft* craft, SimulationSta return true; } - OrbitalElements future_elements = propagate_orbital_elements(craft->orbit, sim->dt, parent->mass); - double future_nu = normalize_angle(future_elements.true_anomaly); - - bool between = angle_between(current_nu, future_nu, target_nu); - if (!between) { - return false; - } - - double future_diff = angular_distance(future_nu, target_nu); - - bool wraparound_crossing = (current_nu > 5.0 && future_nu < 1.0) || - (current_nu < 1.0 && future_nu > 5.0); - - if (future_diff > current_diff && !wraparound_crossing) { - return false; - } - double a = craft->orbit.semi_major_axis; double e = craft->orbit.eccentricity; double mu = G * parent->mass; @@ -180,7 +152,7 @@ bool check_maneuver_trigger(Maneuver* maneuver, Spacecraft* craft, SimulationSta dt_needed += M_period / n; } - if (dt_needed > sim->dt) { + if (dt_needed <= 0.0 || dt_needed > sim->dt) { return false; }