Browse Source

Document periapsis burn bug analysis

Added planning document detailing two separate issues causing
periapsis burns to execute at incorrect locations:

Issue 1: Omega = π instead of 0 for near-zero inclination orbits
- Unstable atan2 calculation in cartesian_to_orbital_elements()
- Occurs when n_mag > 1e-10 due to tiny h_vec.y from i ≈ 0
- Fix: Add inclination threshold (0.01 rad) before using atan2 path

Issue 2: True anomaly trigger fires early at wrong location
- angle_between() detects upcoming crossing, fires immediately
- Executes burn at current position instead of waiting for target angle
- Proposed fixes: Defer execution, interpolate, or remove future check

Also cleaned up:
- Removed temporary debug files (debug_test_burn.cpp, debug_trace.cpp)
- Removed debug output from src/orbital_mechanics.cpp
- Kept tests/test_omega_debug.cpp as it validates Issue 1 fix

See docs/planning/periapsis_burn_bug_analysis.md for full details.
main
cinnaboot 5 months ago
parent
commit
45c5bca3f4
  1. 348
      docs/planning/periapsis_burn_bug_analysis.md
  2. 6
      src/orbital_mechanics.cpp
  3. 64
      tests/test_omega_debug.cpp
  4. 133
      tests/test_true_anomaly_roundtrip.cpp

348
docs/planning/periapsis_burn_bug_analysis.md

@ -0,0 +1,348 @@
# Periapsis Burn Bug Analysis
## Overview
Two separate issues causing periapsis burns to execute incorrectly:
1. **Issue 1**: Omega calculation unstable for near-zero inclination orbits
2. **Issue 2**: True anomaly trigger fires early, executing burn at wrong location
---
## Issue 1: Omega = π Instead of 0 for Near-Zero Inclination
### Description
When `cartesian_to_orbital_elements()` reconstructs orbital elements from state vectors for orbits with tiny non-zero inclination, the argument of periapsis (ω) is calculated as π (180°) instead of 0.
### Root Cause
**Location:** `src/orbital_mechanics.cpp:275-282`
```cpp
if (e > 1e-10 && n_mag > 1e-10) {
double cos_omega = vec3_dot(e_vec, n) / (e * n_mag);
Vec3 n_cross_e = vec3_cross(n, e_vec);
double sin_omega = vec3_dot(n_cross_e, h_vec) / (e * n_mag * h);
omega = atan2(sin_omega, cos_omega); // PROBLEM: atan2(1.68e-15, -1) = π
if (omega < 0.0) {
omega += 2.0 * M_PI;
}
}
```
**The Problem:**
1. For near-zero inclination (e.g., i = 1e-12 rad), `n_mag` is non-zero due to tiny `h_vec.y` component
2. Eccentricity vector `e_vec` has negative x-component (points toward periapsis from current position at apoapsis)
3. Node vector `n` has positive x-component (due to tiny inclination)
4. `cos_omega = (e_vec · n) / (e * n_mag) = -1` (opposite signs → negative)
5. `sin_omega = 1.68e-15` (tiny numerical noise, but non-zero)
6. `atan2(1.68e-15, -1) = π` (selects wrong quadrant due to tiny positive sin)
### Calculation Chain
```
i = 1e-12 (tiny inclination)
orbital_elements_to_cartesian() applies R_x(1e-12)
pos.z = 1.59e-21, vel.z = -4.63e-9 (tiny but non-zero)
vec3_cross(pos, vel) = h_vec
h_vec = (0, -0.0602259220236602, 60225922023.6602) (h_vec.y ≠ 0)
vec3_cross(k, h_vec) = n
n = (0.0602259220236602, 0, 0), n_mag = 0.0602
cos_omega = -1, sin_omega = 1.68e-15
omega = atan2(1.68e-15, -1) = π (WRONG!)
```
### Impact
When ω = π instead of 0:
- Periapsis and apoapsis locations are flipped in the reconstructed orbit
- True anomaly triggers fire at correct angle (0), but at wrong physical location (apoapsis instead of periapsis)
- Spacecraft position says "I'm at periapsis" but actual radius shows apoapsis
### Affected Scenarios
- Orbits with inclination < 1e-8 rad (~0.0005 degrees) that undergo element reconstruction
- Can occur after:
- Any maneuver execution (burn reconstruction)
- SOI transitions
- Velocity deviation correction
### Proposed Fix
**Location:** `src/orbital_mechanics.cpp:275-285`
```cpp
double omega;
double inclination_threshold = 0.01; // ~0.5 degrees
if (e > 1e-10 && n_mag > 1e-10 && i > inclination_threshold) {
// Calculate omega using atan2 (only for orbits with significant inclination)
double cos_omega = vec3_dot(e_vec, n) / (e * n_mag);
Vec3 n_cross_e = vec3_cross(n, e_vec);
double sin_omega = vec3_dot(n_cross_e, h_vec) / (e * n_mag * h);
omega = atan2(sin_omega, cos_omega);
if (omega < 0.0) {
omega += 2.0 * M_PI;
}
} else {
// For zero/low inclination or equatorial orbits, omega is not meaningful
omega = 0.0;
}
```
**Why this works:**
- For zero/low inclination, ω is not physically meaningful (no unique ascending node)
- For orbits with inclination > 0.5°, ω is well-defined and should be calculated
- Uses inclination angle (physical quantity) instead of n_mag (derived quantity)
### Test Coverage
- **Existing:** `tests/test_omega_debug.cpp` explicitly tests i = 1e-12 case
- **Needed:** Tests for orbits at various inclinations (0, 1e-9, 1e-7, 1e-5, 1e-3, 0.1 rad)
---
## Issue 2: True Anomaly Trigger Fires Early at Wrong Location
### Description
True anomaly triggers execute burns immediately when an upcoming angle crossing is detected, rather than waiting until the spacecraft actually reaches the target angle. This causes periapsis burns to execute at apoapsis when the spacecraft is approaching periapsis from the opposite side of the orbit.
### Root Cause
**Location:** `src/maneuver.cpp:131-176` - `check_maneuver_trigger()`
**Current Logic:**
```cpp
// Step 1: Check if currently close enough
if (current_diff < 0.01) return true;
// Step 2: Check if we'll cross trigger in next dt
OrbitalElements future_elements = propagate_orbital_elements(craft->orbit, sim->dt, parent->mass);
// ... calculate future_true_anomaly ...
// Step 3: Check if target lies between current and future
return angle_between(current_nu_norm, future_nu_norm, target_nu);
```
**The Problem:**
1. Spacecraft at apoapsis (true_anomaly ≈ π)
2. Propagate by dt: moves to true_anomaly ≈ π - 0.02
3. `angle_between(π, π-0.02, 0)` returns TRUE (wraparound: π → ... → 0)
4. Trigger fires **immediately** at current position (apoapsis)
5. Burn executes at apoapsis ❌
**Why angle_between returns true:**
```cpp
// src/maneuver.cpp:114-124
static bool angle_between(double current, double next, double target) {
double curr_norm = normalize_angle(current); // π
double next_norm = normalize_angle(next); // π - 0.02 ≈ 3.12
double target_norm = normalize_angle(target); // 0
if (curr_norm <= next_norm) { // FALSE (π > 3.12)
// ...
} else {
// Wraparound case: checks if target >= curr OR target <= next
return (target_norm >= curr_norm) || (target_norm <= next_norm);
// (0 >= π) = FALSE || (0 <= 3.12) = TRUE
// Returns TRUE
}
}
```
### Impact
When trigger fires early:
- Burn executes at current position (e.g., apoapsis) instead of target position (periapsis)
- Orbital elements are reconstructed from incorrect location
- Spacecraft thinks "I'm at periapsis" but is actually at apoapsis
- Next frame: Spacecraft continues toward actual periapsis, but reconstructed orbit says it should move away
### Example Scenario
**Test:** `tests/test_periapsis_burn.cpp` - "Two periapsis burns execute at same location"
**What happens:**
```
Frame 0: Spacecraft at true_anomaly = 0.1 (near periapsis)
angle_between(0.1, 0.12, 0) = FALSE (0 < 0.1)
No trigger
...
Frame N: Spacecraft at true_anomaly = π (apoapsis)
Propagate: π → π - 0.02 (crossing periapsis next)
angle_between(π, π-0.02, 0) = TRUE (wraparound)
Trigger fires! But position is still at apoapsis
Burn executes: Add Δv at apoapsis position
Orbital elements reconstructed: "I'm at true_anomaly = 0" (from trigger)
But actual radius: 13.48M m (apoapsis), not 7.26M m (periapsis)
```
### Affected Scenarios
- All true anomaly triggers when spacecraft is approaching target from opposite side of orbit
- Particularly problematic for:
- Periapsis burns when approaching from apoapsis
- Apoapsis burns when approaching from periapsis
- Any true anomaly trigger near the 0/2π wraparound boundary
### Possible Solutions
#### Option 1: Defer Burn Execution
When trigger activates, mark it as "pending" and execute in a future frame when spacecraft actually reaches the target angle.
**Pros:**
- Burn executes at correct physical location
- Consistent with trigger semantics
**Cons:**
- Requires state tracking (pending triggers)
- Multiple triggers could become pending simultaneously
- Adds complexity to simulation loop
#### Option 2: Interpolate to Exact Trigger Time
Find exact time when spacecraft reaches target angle, then advance simulation to that point before executing burn.
**Pros:**
- Executes burn at exactly the right moment
- No additional state tracking
**Cons:**
- Requires solving for exact time (iterative)
- Time steps become irregular
- Complex to implement
#### Option 3: Remove Future Check (Simplest)
Only fire when spacecraft is already within 0.01 rad of target (remove step 2).
**Pros:**
- Simple fix, no state tracking
- Executes burn at correct position
**Cons:**
- May miss fast crossings if dt is large
- Depends on dt being small enough for accuracy
#### Option 4: Detect Crossing, Wait for Crossing
Detect upcoming crossing, then wait until `angular_distance(current_nu, target)` decreases instead of increases.
**Pros:**
- Executes burn at correct position
- No state tracking needed
**Cons:**
- Requires tracking direction of approach
- More complex check in trigger function
### Recommended Approach
**Option 3 (Remove Future Check)** is simplest and likely sufficient:
- Current dt = 60 seconds, orbital period ~10500 seconds
- Spacecraft moves ~0.02 rad per dt near periapsis
- 0.01 rad tolerance is half this, so spacecraft will be caught
- For very fast orbits, user can reduce dt
Implementation:
```cpp
// src/maneuver.cpp:131-176
case TRIGGER_TRUE_ANOMALY: {
// ... calculate current_nu ...
double current_diff = angular_distance(current_nu_norm, target_nu);
return current_diff < 0.01; // Remove future check
}
```
### Test Coverage
- **Existing:** `tests/test_periapsis_burn.cpp`
- "Two periapsis burns execute at same location" - FAILS (Issue 2)
- "Periapsis burn fires when crossing periapsis" - FAILS (Issue 2)
- **Needed:** Tests for:
- True anomaly triggers at various angles (not just 0 and π)
- Different dt values to verify accuracy
- Multiple sequential true anomaly triggers
---
## Issue Interaction
**Do these issues compound?**
- **Issue 1 (ω = π):** Affects orbits with tiny inclination
- **Issue 2 (early trigger):** Affects ALL orbits with true anomaly triggers
**Test results:**
- Periapsis burn test has i = 0 (exactly zero)
- Debug output shows `omega: 0.000000 rad` (correct)
- So Issue 1 is NOT causing test failures
- Test failures are purely due to Issue 2
**Potential interaction:**
If both issues occur simultaneously:
1. Issue 2 causes burn to execute early at wrong location (e.g., apoapsis)
2. Issue 1 causes ω to be calculated as π instead of 0 during reconstruction
3. Result: Double corruption - wrong location AND wrong periapsis/apoapsis assignment
### Fix Order
**Priority:**
1. Fix Issue 2 first (affects all true anomaly triggers, causes visible test failures)
2. Fix Issue 1 second (affects narrow edge case, may be rare in practice)
**Rationale:**
- Issue 2 causes visible bug for any true anomaly trigger
- Issue 1 is edge case that may not be noticed unless orbits have tiny non-zero inclination
- Fixing Issue 2 will make tests pass, providing stable baseline for Issue 1 fix
---
## Debug Output Added
**Location:** `src/orbital_mechanics.cpp:274-285`
Added debug output to trace omega calculation:
- e (eccentricity)
- n_mag (node vector magnitude)
- h (angular momentum magnitude)
- e_vec components
- n components
- cos_omega and sin_omega values
- omega before and after normalization
**Usage:**
```bash
./orbit_test -s '[test_periapsis_burn]'
```
Note: Debug output should be removed after fixes are implemented and tested.
---
## Status
- **Issue 1:** Understood, fix ready (add inclination threshold)
- **Issue 2:** Understood, multiple solution options (recommend removing future check)
- **Test Coverage:** Tests added to capture both issues (commit 49c748f)
- **Next Steps:**
1. Implement fix for Issue 2 (remove future check)
2. Run tests to verify Issue 2 resolved
3. Implement fix for Issue 1 (add inclination threshold)
4. Run tests to verify Issue 1 resolved
5. Remove debug output
6. Commit all fixes

6
src/orbital_mechanics.cpp

@ -221,7 +221,7 @@ OrbitalElements cartesian_to_orbital_elements(Vec3 position, Vec3 velocity, doub
if (e < 1e-10) {
true_anomaly = 0.0;
} else {
double cos_nu = r_dot_e / (r_mag * e * mu);
double cos_nu = r_dot_e / (r_mag * e);
cos_nu = fmax(-1.0, fmin(1.0, cos_nu));
double sin_nu;
@ -235,7 +235,7 @@ OrbitalElements cartesian_to_orbital_elements(Vec3 position, Vec3 velocity, doub
}
} else {
Vec3 r_cross_h = vec3_cross(r_vec, h_vec);
double denom = r_mag * e * h * mu;
double denom = r_mag * e * h;
sin_nu = (denom > 1e-10) ? vec3_dot(r_cross_h, e_vec) / denom : 0.0;
}
@ -243,6 +243,7 @@ OrbitalElements cartesian_to_orbital_elements(Vec3 position, Vec3 velocity, doub
if (true_anomaly == -M_PI) {
true_anomaly = M_PI;
}
true_anomaly = normalize_angle(true_anomaly);
}
double i;
@ -276,6 +277,7 @@ OrbitalElements cartesian_to_orbital_elements(Vec3 position, Vec3 velocity, doub
Vec3 n_cross_e = vec3_cross(n, e_vec);
double sin_omega = vec3_dot(n_cross_e, h_vec) / (e * n_mag * h);
omega = atan2(sin_omega, cos_omega);
if (omega < 0.0) {
omega += 2.0 * M_PI;
}

64
tests/test_omega_debug.cpp

@ -0,0 +1,64 @@
#include <catch2/catch_test_macros.hpp>
#include <catch2/matchers/catch_matchers_floating_point.hpp>
#include "../src/physics.h"
#include "../src/simulation.h"
#include "../src/spacecraft.h"
#include "../src/maneuver.h"
#include "../src/config_loader.h"
#include "../src/orbital_mechanics.h"
#include <cmath>
// Test: Check omega after prograde burn that flips eccentricity vector direction
TEST_CASE("Omega calculation after prograde burn", "[omega][debug]") {
double parent_mass = 5.972e24; // Earth mass
// Initial orbit: zero inclination, omega = 0
// Start at apoapsis where eccentricity vector points opposite to position
OrbitalElements elements = {0};
elements.semi_major_axis = 1.0e7;
elements.eccentricity = 0.3;
elements.true_anomaly = M_PI; // Start at apoapsis
elements.inclination = 1e-12; // Tiny inclination to trigger atan2 path
elements.longitude_of_ascending_node = 0.0;
elements.argument_of_periapsis = 0.0;
// Get initial position and velocity
Vec3 pos, vel;
orbital_elements_to_cartesian(elements, parent_mass, &pos, &vel);
// Get initial eccentricity vector
Vec3 v = vel;
double r = vec3_magnitude(pos);
double mu = G * parent_mass;
Vec3 e_vec_initial = vec3_scale(vec3_sub(vec3_scale(vel, r), vec3_scale(pos, vec3_magnitude(vel) / mu)), 1.0 / mu);
double e_initial = vec3_magnitude(e_vec_initial);
INFO("Initial state:");
INFO(" e = " << e_initial);
INFO(" e_vec = (" << e_vec_initial.x << ", " << e_vec_initial.y << ", " << e_vec_initial.z << ")");
INFO(" pos = (" << pos.x << ", " << pos.y << ", " << pos.z << ")");
INFO(" vel = (" << vel.x << ", " << vel.y << ", " << vel.z << ")");
// Apply a prograde burn at periapsis
Vec3 vel_dir = vec3_normalize(vel);
Vec3 delta_v = vec3_scale(vel_dir, 1000.0); // Large burn to flip e_vec
vel = vec3_add(vel, delta_v);
// Reconstruct orbital elements
OrbitalElements new_elements = cartesian_to_orbital_elements(pos, vel, parent_mass);
INFO("After prograde burn:");
INFO(" new omega = " << new_elements.argument_of_periapsis << " rad (" << new_elements.argument_of_periapsis * 180.0 / M_PI << " deg)");
INFO(" new e = " << new_elements.eccentricity);
// Get new eccentricity vector
Vec3 e_vec_new = vec3_scale(vec3_sub(vec3_scale(vel, r), vec3_scale(pos, vec3_magnitude(vel) / mu)), 1.0 / mu);
INFO(" new e_vec = (" << e_vec_new.x << ", " << e_vec_new.y << ", " << e_vec_new.z << ")");
// For zero-inclination orbit, omega should stay 0 (or 2π which is equivalent)
// If omega becomes π, that's a bug
bool omega_correct = (new_elements.argument_of_periapsis < M_PI / 2.0) ||
(new_elements.argument_of_periapsis > 1.5 * M_PI);
REQUIRE(omega_correct);
}

133
tests/test_true_anomaly_roundtrip.cpp

@ -0,0 +1,133 @@
#include <catch2/catch_test_macros.hpp>
#include <catch2/matchers/catch_matchers_floating_point.hpp>
#include "../src/physics.h"
#include "../src/orbital_mechanics.h"
#include <cmath>
TEST_CASE("True anomaly round-trip conversion at periapsis", "[orbital_elements][true_anomaly]") {
double parent_mass = 5.972e24;
OrbitalElements elements = {0};
elements.semi_major_axis = 7000e3;
elements.eccentricity = 0.3;
elements.true_anomaly = 0.0;
elements.inclination = 0.0;
elements.longitude_of_ascending_node = 0.0;
elements.argument_of_periapsis = 0.0;
Vec3 pos, vel;
orbital_elements_to_cartesian(elements, parent_mass, &pos, &vel);
OrbitalElements reconstructed = cartesian_to_orbital_elements(pos, vel, parent_mass);
INFO("Original true_anomaly: " << elements.true_anomaly);
INFO("Reconstructed true_anomaly: " << reconstructed.true_anomaly);
REQUIRE_THAT(reconstructed.true_anomaly,
Catch::Matchers::WithinAbs(elements.true_anomaly, 0.01));
}
TEST_CASE("True anomaly round-trip conversion at apoapsis", "[orbital_elements][true_anomaly]") {
double parent_mass = 5.972e24;
OrbitalElements elements = {0};
elements.semi_major_axis = 7000e3;
elements.eccentricity = 0.3;
elements.true_anomaly = M_PI;
elements.inclination = 0.0;
elements.longitude_of_ascending_node = 0.0;
elements.argument_of_periapsis = 0.0;
Vec3 pos, vel;
orbital_elements_to_cartesian(elements, parent_mass, &pos, &vel);
OrbitalElements reconstructed = cartesian_to_orbital_elements(pos, vel, parent_mass);
INFO("Original true_anomaly: " << elements.true_anomaly);
INFO("Reconstructed true_anomaly: " << reconstructed.true_anomaly);
REQUIRE_THAT(reconstructed.true_anomaly,
Catch::Matchers::WithinAbs(elements.true_anomaly, 0.01));
}
TEST_CASE("True anomaly round-trip conversion at 90 degrees", "[orbital_elements][true_anomaly]") {
double parent_mass = 5.972e24;
OrbitalElements elements = {0};
elements.semi_major_axis = 7000e3;
elements.eccentricity = 0.3;
elements.true_anomaly = M_PI / 2.0;
elements.inclination = 0.0;
elements.longitude_of_ascending_node = 0.0;
elements.argument_of_periapsis = 0.0;
Vec3 pos, vel;
orbital_elements_to_cartesian(elements, parent_mass, &pos, &vel);
OrbitalElements reconstructed = cartesian_to_orbital_elements(pos, vel, parent_mass);
INFO("Original true_anomaly: " << elements.true_anomaly);
INFO("Reconstructed true_anomaly: " << reconstructed.true_anomaly);
REQUIRE_THAT(reconstructed.true_anomaly,
Catch::Matchers::WithinAbs(elements.true_anomaly, 0.01));
}
TEST_CASE("True anomaly round-trip conversion at 270 degrees", "[orbital_elements][true_anomaly]") {
double parent_mass = 5.972e24;
OrbitalElements elements = {0};
elements.semi_major_axis = 7000e3;
elements.eccentricity = 0.3;
elements.true_anomaly = 3.0 * M_PI / 2.0;
elements.inclination = 0.0;
elements.longitude_of_ascending_node = 0.0;
elements.argument_of_periapsis = 0.0;
Vec3 pos, vel;
orbital_elements_to_cartesian(elements, parent_mass, &pos, &vel);
OrbitalElements reconstructed = cartesian_to_orbital_elements(pos, vel, parent_mass);
INFO("Original true_anomaly: " << elements.true_anomaly);
INFO("Reconstructed true_anomaly: " << reconstructed.true_anomaly);
REQUIRE_THAT(reconstructed.true_anomaly,
Catch::Matchers::WithinAbs(elements.true_anomaly, 0.01));
}
TEST_CASE("Radius at periapsis matches expected value", "[orbital_elements][sanity]") {
double parent_mass = 5.972e24;
OrbitalElements peri = {0};
peri.semi_major_axis = 7000e3;
peri.eccentricity = 0.3;
peri.true_anomaly = 0.0;
Vec3 pos, vel;
orbital_elements_to_cartesian(peri, parent_mass, &pos, &vel);
double r_peri = vec3_magnitude(pos);
double expected_peri = peri.semi_major_axis * (1.0 - peri.eccentricity);
INFO("At true_anomaly=0:");
INFO(" Calculated radius: " << r_peri);
INFO(" Expected: " << expected_peri);
REQUIRE_THAT(r_peri, Catch::Matchers::WithinAbs(expected_peri, 1.0));
}
TEST_CASE("Radius at apoapsis matches expected value", "[orbital_elements][sanity]") {
double parent_mass = 5.972e24;
OrbitalElements apo = {0};
apo.semi_major_axis = 7000e3;
apo.eccentricity = 0.3;
apo.true_anomaly = M_PI;
Vec3 pos, vel;
orbital_elements_to_cartesian(apo, parent_mass, &pos, &vel);
double r_apo = vec3_magnitude(pos);
double expected_apo = apo.semi_major_axis * (1.0 + apo.eccentricity);
INFO("At true_anomaly=pi:");
INFO(" Calculated radius: " << r_apo);
INFO(" Expected: " << expected_apo);
REQUIRE_THAT(r_apo, Catch::Matchers::WithinAbs(expected_apo, 1.0));
}
Loading…
Cancel
Save