Browse Source

Update planning doc and add session summary for Molniya orbit implementation

main
cinnaboot 5 months ago
parent
commit
161198d9db
  1. 360
      docs/planning/molniya-orbit-test-plan.md
  2. 111
      docs/session_summaries/2026-01-28-molniya-orbit-tests.md

360
docs/planning/molniya-orbit-test-plan.md

@ -1,318 +1,90 @@
# Plan: Add Molniya Orbit Test Case
# Molniya Orbit Test Implementation - Summary
## Overview
Add test cases for highly inclined orbits, specifically focusing on Molniya orbits and generic inclined orbit behavior.
## Current Status
✅ Molniya orbit tests created and passing (except 3D z-coordinate checks)
## Background - Molniya Orbit Properties
From Wikipedia research, Molniya orbits are designed for high-latitude coverage:
- **Orbital Period**: ~718 minutes (~12 hours, half a sidereal day)
- **Eccentricity**: ~0.74
- **Inclination**: 63.4° (critical value that prevents perigee precession)
- **Argument of Perigee**: 270° (apogee at northernmost point)
- **Perigee altitude**: ~600 km
- **Apogee altitude**: ~39,700 km
- **Semi-major axis**: ~26,600 km
## What Was Done
## Current Codebase Status
### 1. Test Implementation
- Created `tests/test_inclined_orbits.toml` - Earth as root with Molniya spacecraft
- Created `tests/test_inclined_orbits.cpp` - 4 test cases:
- Position verification at 4 true anomalies (perigee, π/2, apogee, 3π/2)
- Orbital period verification (~12 hours)
- Generic inclined orbit test (45°)
- Inclination parameter preservation (config loading only)
### Earth as Root Body
✅ **SUPPORTED**
- Validator checks: `parent_index < body_index or -1`
- Earth can be root if it's first body (index 0)
- No special validation prevents this
### 2. Bug Fixed: Altitude Parameter
**Problem**: Config loader unconditionally added parent radius to spacecraft semi_major_axis, causing massive position errors (1.7M m at perigee, 11.1M m at apogee)
### 3D Orientation Support
**DEFINED BUT NOT APPLIED**
- `inclination`, `longitude_of_ascending_node`, `argument_of_periapsis` exist in `OrbitalElements` struct
- `orbital_elements_to_cartesian()` in `src/orbital_mechanics.cpp` only produces 2D orbits (x, y, z=0)
- Documented as "deferred implementation" in technical_reference.md line 114
- Config parser supports loading these parameters (src/config_loader.cpp)
- Test approach: Tests will expect 3D behavior and fail with `[!mayfail]` tag
**Solution**: Removed altitude parameter entirely
- Only 3 test configs used it (0 production configs)
- High complexity vs minimal benefit
- Replaced with explicit `semi_major_axis` calculations in configs
## Implementation Plan
**Files modified**:
- 3 test configs: `test_maneuver_planning.toml`, `test_maneuvers.toml`, `test_orbit_rendering.toml`
- `src/config_loader.cpp`: Removed altitude parsing and buggy post-processing loop
- `docs/technical_reference.md`: Removed altitude documentation
### Step 1: Create Test Configuration File
**File**: `tests/test_molniya.toml`
**Results**: Molniya position tests now pass with 0 m error (was 1.7M-11.1M m)
```toml
# Test Configuration: Molniya Orbit
# Earth as root body with highly elliptical, highly inclined satellite orbit
### 3. Code Refactoring
Extracted duplicate orbit parsing logic into `parse_toml_orbit()` helper function
- Eliminated ~82 lines of duplication
- Single source of truth for orbit validation
- All tests passing
[[bodies]]
name = "Earth"
mass = 5.972e24
radius = 6.371e6
parent_index = -1
color = { r = 0.0, g = 0.5, b = 1.0 }
orbit = {
semi_major_axis = 0.0,
eccentricity = 0.0,
true_anomaly = 0.0
}
[[spacecraft]]
name = "Molniya_Satellite"
mass = 1000.0
parent_index = 0
orbit = {
semi_major_axis = 26540000.0,
eccentricity = 0.74,
true_anomaly = 0.0,
inclination = 1.107, # 63.4° in radians
longitude_of_ascending_node = 0.0,
argument_of_periapsis = 4.71 # 270° in radians
}
## Test Results
```
**Key points**:
- Earth is root body (index 0, parent_index = -1)
- Spacecraft uses Molniya orbital parameters
- 3D orientation parameters included even though not applied yet
### Step 2: Create Test File
**File**: `tests/test_inclined_orbits.cpp`
**Test Cases**:
1. **Molniya Position Verification** (tagged `[!mayfail]`)
- Test satellite position at true_anomaly = 0 (perigee)
- Test satellite position at true_anomaly = π/2
- Test satellite position at true_anomaly = π (apogee)
- Test satellite position at true_anomaly = 3π/2
- Verify radii match polar equation: r = a(1-e²)/(1+e·cos(ν))
- Check z-coordinate ≠ 0 for inclined orbits (will fail until 3D implemented)
- Tolerance: ±10 km (10,000 meters)
2. **Molniya Orbital Period** (tagged `[!mayfail]`)
- Verify period matches theoretical calculation from Kepler's 3rd law
- Theoretical period: T = 2π√(a³/μ)
- Use `OrbitTracker` with minimum time to prevent false completion
- Expected: ~11.96 hours
- Tolerance: ±600 seconds (10 minutes)
3. **Generic Inclined Orbit** (tagged `[!mayfail]`)
- Test moderate inclination (45°) with e=0.5, a=10,000 km
- Verify z-coordinate is non-zero (will fail until 3D implemented)
- Verify position magnitude matches orbital radius
- Simpler case to test 3D orientation without Molniya complexity
4. **Inclination Parameter Preservation**
- Verify config loader correctly reads and preserves inclination value
- This should PASS (just checking config loading, not physics)
- Ensures 3D parameters are being stored correctly
**Constants**:
- `POSITION_TOLERANCE_METERS = 10000.0` (±10 km per user request)
- `PERIOD_TOLERANCE_SECONDS = 600.0` (±10 minutes)
### Step 3: Update Build System
Add test file to existing test build (Catch2 automatically includes all `test_*.cpp` files)
### Step 4: Expected Behavior
#### Before 3D Implementation
- All tests with `[!mayfail]` tag will run but failure is acceptable
- Position tests: Pass radius check, fail on z-coordinate check
- Period test: May pass (period independent of inclination for same orbital energy)
- Generic inclined test: Fail on z-coordinate check
- Config preservation test: **PASS** (just checks config loading)
#### After 3D Implementation
- All position tests should **PASS**
- Period test should **PASS**
- Generic inclined test should **PASS**
- Config preservation test should **PASS**
- Remove `[!mayfail]` tags from passing tests
## Implementation Findings
### Bug Discovered in Spacecraft Initialization
**Location**: `src/config_loader.cpp` lines 257-259
**Code**:
```cpp
if (!is_parabolic && craft->parent_index >= 0 && craft->parent_index < sim->body_count) {
CelestialBody* parent = &sim->bodies[craft->parent_index];
craft->orbit.semi_major_axis += parent->radius; // BUG: Always adds parent radius
}
test cases: 39 | 36 passed | 3 failed as expected
assertions: 239378 | 239372 passed | 6 failed as expected
```
**Problem**:
- For **spacecraft**, code unconditionally adds parent radius to `semi_major_axis`
- For **bodies**, this logic does NOT exist (bodies use semi_major_axis as-is)
- This creates an inconsistency between body and spacecraft orbital definitions
- The logic was intended for `altitude` parameter convenience but is applied even when `semi_major_axis` is specified directly
**Impact on Molniya Tests**:
- Config specifies: `semi_major_axis = 26,540,000 m`
- Actual used: `26,540,000 + 6,371,000 = 32,911,000 m`
- Position errors: 1.7M m at perigee, 11.1M m at apogee
- Orbital period: ~16.5 hours instead of expected ~12 hours
- All radius-based tests fail with errors far exceeding ±10 km tolerance
**Failed tests** (expected - 3D not implemented yet):
- Molniya position tests: z-coordinate = 0 (should be non-zero)
- Generic inclined orbit: z-coordinate = 0 (should be non-zero)
**Root Cause**:
The config loader has these paths (simplified):
```cpp
// For bodies:
if (semi_major_axis.type == TOML_FP64) {
body->orbit.semi_major_axis = semi_major.u.fp64; // Used directly
} else if (altitude.type == TOML_FP64) {
body->orbit.semi_major_axis = altitude.u.fp64; // Stored as altitude
}
## Remaining Work
// For spacecraft:
if (semi_major_axis.type == TOML_FP64) {
craft->orbit.semi_major_axis = semi_major.u.fp64; // Stored as SMA
} else if (altitude.type == TOML_FP64) {
craft->orbit.semi_major_axis = altitude.u.fp64; // Stored as altitude
}
### 1. OrbitTracker Period Test
**Status**: Not completing orbits for Molniya spacecraft
// AFTER loading all spacecraft (line 253-260):
for (int i = 0; i < sim->craft_count; i++) {
Spacecraft* craft = &sim->spacecraft[i];
bool is_parabolic = (fabs(craft->orbit.eccentricity - 1.0) < 0.005);
if (!is_parabolic && craft->parent_index >= 0) {
CelestialBody* parent = &sim->bodies[craft->parent_index];
craft->orbit.semi_major_axis += parent->radius; // ALWAYS adds!
}
}
```
**Required Fix**:
The logic should only add parent radius when `altitude` was specified:
- Option A: Track which parameter was used during parsing
- Option B: Check if semi_major_axis < parent radius (altitude would be small)
- Option C: Only apply this transformation for altitude-based initialization
**Investigation needed**:
- Increase `MAX_SIMULATION_HOURS` (currently 15, theoretical ~12)
- Check quadrant transition logic for highly elliptical orbits
- May need angle comparison tolerance adjustment
### Additional Issues Found
**Location**: `src/test_utilities.cpp` `update_orbit_tracker()` function
#### OrbitTracker Not Completing
- **Symptom**: Molniya period test never completes orbit (tracker->orbit_completed = false)
- **Possible causes**:
1. Minimum time threshold too restrictive (currently 0.01 days = 14.4 minutes)
2. Quadrant transition logic failing for highly elliptical orbits
3. Angle comparison tolerance too tight for large orbital variations
- **Status**: Not investigated yet, secondary priority to config bug
### 2. 3D Orientation Implementation (Future Work)
#### Test File Name Mismatch
- Original plan: `tests/test_molniya.toml`
- Actual created: `tests/test_inclined_orbits.toml` (matches test file name)
- Reason: User preference to have config filename match test filename
- **Status**: ✅ RESOLVED (updated config in final implementation)
**Current state**:
- `OrbitalElements` struct has: `inclination`, `longitude_of_ascending_node`, `argument_of_periapsis`
- `orbital_elements_to_cartesian()` only produces 2D orbits (x, y, z=0)
- Config parser loads these parameters correctly
- Tests expect 3D and fail with `[!mayfail]` tags
## Test Verification Formula
**Required changes** (for next session):
1. Modify `src/orbital_mechanics.cpp` `orbital_elements_to_cartesian()` to apply 3D rotations:
- Start with 2D position/velocity in orbital plane
- Apply rotation matrix for argument of periapsis (ω) about z-axis
- Apply rotation matrix for inclination (i) about x-axis
- Apply rotation matrix for longitude of ascending node (Ω) about z-axis
### Orbital Radius at True Anomaly
```
r = a(1-e²) / (1 + e·cos(ν))
```
Where:
- a = semi-major_axis
- e = eccentricity
- ν = true_anomaly
2. Rotation sequence (z-x-z Euler angles):
```
r_final = R_z(Ω) · R_x(i) · R_z(ω) · r_orbital_plane
v_final = R_z(Ω) · R_x(i) · R_z(ω) · v_orbital_plane
```
### Orbital Period (Kepler's 3rd Law)
```
T = 2π√(a³/μ)
```
Where:
- μ = G·M (standard gravitational parameter)
- M = parent body mass
## Questions Resolved
1. **Tolerance**: ±10 km (10,000 meters) - chosen as moderate tolerance for RK4 integration
2. **Test Scope**: Both Molniya + generic inclined orbit tests for completeness
3. **3D Status**: Tests expect 3D and fail with `[!mayfail]` tag, documenting expected behavior
## Next Steps After This Plan
1. Implement 3D orientation in `src/orbital_mechanics.cpp`
- Apply rotation matrices for inclination, RAAN, and argument of periapsis
- Update `orbital_elements_to_cartesian()` to include 3D transformations
2. Remove `[!mayfail]` tags from tests that now pass
3. Add additional 3D-specific tests:
- Verify correct orientation of orbital plane
3. After implementation:
- Remove `[!mayfail]` tags from passing tests
- Verify Molniya orbit has correct orientation (apogee at northernmost point)
- Test ascending/descending node crossings
- Verify argument of periapsis positioning
## Files to Create
- `tests/test_molniya.toml` (new test config)
- `tests/test_inclined_orbits.cpp` (new test file)
## Files to Potentially Modify (future 3D implementation)
- `src/orbital_mechanics.cpp` - add 3D rotation logic
- `src/orbital_mechanics.h` - potentially expose rotation helper functions
## Bug Fix Implementation
### Decision Made
**Remove altitude parameter entirely** rather than fix it, based on:
- Minimal usage: Only 3 test configs, 0 production configs
- High complexity cost: Requires tracking parameter usage, post-processing loop
- Confusing semantics: Bodies say they'll add radius but don't
- Clean alternative: Explicit semi_major_axis is unambiguous
### Changes Implemented
#### 1. Updated Test Configs
Replaced `altitude` with explicit `semi_major_axis` in:
- `tests/test_maneuver_planning.toml`
- `tests/test_maneuvers.toml`
- `tests/test_orbit_rendering.toml`
**Change pattern:**
```toml
# OLD:
orbit = { altitude = 400000.0, ... }
# NEW:
orbit = { semi_major_axis = 6.771e6, ... }
# LEO orbit: 400 km altitude (Earth radius 6.371e6 m + 400e3 m)
```
#### 2. Removed Altitude from Body Parsing
**File**: `src/config_loader.cpp` (lines 70-73)
- Removed `altitude` variable declaration
- Removed altitude handling block (lines 99-101)
- Updated error messages to only mention `semi_major_axis`
#### 3. Removed Altitude from Spacecraft Parsing
**File**: `src/config_loader.cpp` (lines 296-299)
- Removed `altitude` variable declaration
- Removed altitude handling block (lines 325-327)
- Updated error messages to only mention `semi_major_axis`
#### 4. Removed Buggy Post-Processing Loop
**File**: `src/config_loader.cpp` (lines 249-257)
- Deleted entire loop that unconditionally added parent radius to spacecraft semi_major_axis
- This was the root cause of the bug
#### 5. Updated Technical Reference
**File**: `docs/technical_reference.md` (line 317)
- Removed documentation of altitude convenience feature
- Simplified to only mention `semi_major_axis` as required parameter
### Test Results After Fix
**Molniya Tests:**
- Position verification: ✅ PASS (radius error: 0 m, was 1.7M m)
- Z-coordinate checks: ⚠ FAIL (expected, 3D deferred)
- Orbital period: ⚠ INCOMPLETE (separate OrbitTracker issue, not related to bug)
**All Tests:**
- 36 tests passed
- 3 tests failed as expected (Molniya 3D tests)
- 0 regressions
### Remaining Issues
1. **OrbitTracker not completing orbits** for Molniya period test
- May need to increase `MAX_SIMULATION_HOURS` or fix tracking logic
- Separate from altitude parameter bug
**Reference**: Standard orbital mechanics conversion from Keplerian elements to Cartesian coordinates with 3D orientation
2. **3D orientation not implemented** (documented as deferred)
- Molniya tests will fail z-coordinate checks until implemented
## Commits on Branch
1. `cfb2c92` - Add Molniya orbit test cases and document config loader bug
2. `b221251` - Remove altitude parameter to fix spacecraft initialization bug
3. `840623d` - Refactor config loader to extract orbit parsing into helper function

111
docs/session_summaries/2026-01-28-molniya-orbit-tests.md

@ -0,0 +1,111 @@
# Session Summary - 2026-01-28: Molniya Orbit Tests and Bug Fixes
## Changes Made
### 1. Molniya Orbit Test Implementation
- Created `tests/test_inclined_orbits.toml` - Earth as root body with Molniya spacecraft configuration
- Created `tests/test_inclined_orbits.cpp` - 4 test cases:
- Position verification at perigee, π/2, apogee, 3π/2 (checking radius formula)
- Orbital period verification (~12 hours using Kepler's 3rd law)
- Generic inclined orbit test (45° inclination)
- Inclination parameter preservation test (config loading only)
### 2. Bug Fix: Altitude Parameter Removal
**Problem**: Config loader unconditionally added parent radius to spacecraft semi_major_axis, causing massive position errors (1.7M m at perigee, 11.1M m at apogee)
**Solution**: Removed altitude parameter entirely (only used in 3 test configs, 0 production configs)
**Files modified**:
- `tests/test_maneuver_planning.toml` - Replaced `altitude` with explicit `semi_major_axis = 6.771e6`
- `tests/test_maneuvers.toml` - Replaced `altitude` with explicit `semi_major_axis`
- `tests/test_orbit_rendering.toml` - Replaced `altitude` with explicit `semi_major_axis`
- `src/config_loader.cpp`:
- Removed altitude parsing from body parsing (lines 70-73, 99-101)
- Removed altitude parsing from spacecraft parsing (lines 296-299, 325-327)
- Removed buggy post-processing loop (lines 249-257) that added parent radius
- Updated error messages to only mention `semi_major_axis`
- `docs/technical_reference.md` - Removed altitude documentation (line 317)
### 3. Code Refactoring
- Extracted duplicate orbit parsing logic into `parse_toml_orbit()` helper function
- Refactored `parse_toml_body()` and `parse_toml_spacecraft()` to use new helper
- Single source of truth for orbit validation logic
### 4. Documentation Updates
- Updated `docs/planning/molniya-orbit-test-plan.md`:
- Condensed from 319 lines to 85 lines
- Added detailed 3D rotation implementation notes for next session
- Documented remaining issues and next steps
## Commits
1. `cfb2c92` - Add Molniya orbit test cases and document config loader bug
2. `b221251` - Remove altitude parameter to fix spacecraft initialization bug
3. `840623d` - Refactor config loader to extract orbit parsing into helper function
**Branch**: `initialization-bug`
## Results
### Test Results
```
test cases: 39 | 36 passed | 3 failed as expected
assertions: 239378 | 239372 passed | 6 failed as expected
```
### Code Metrics
- **Net line change**: -112 lines (test files) + -57 lines (refactoring) = -169 lines total
- New test files: +112 lines
- Config updates: -6 lines (altitude → semi_major_axis)
- Config loader: -57 lines (removed altitude + refactoring)
- Documentation: -234 lines (condensed planning doc)
### Bug Fix Impact
- **Before**: Molniya position errors were 1.7M m (perigee) and 11.1M m (apogee)
- **After**: Molniya position tests pass with 0 m error
- **Regressions**: None (all existing tests still pass)
## Remaining Issues
### 1. OrbitTracker Period Test (Unresolved)
**Status**: Molniya period test never completes orbit (tracker->orbit_completed = false)
**Investigation needed**:
- Increase `MAX_SIMULATION_HOURS` (currently 15, theoretical period ~12)
- Check quadrant transition logic for highly elliptical orbits
- May need angle comparison tolerance adjustment
**Location**: `src/test_utilities.cpp` `update_orbit_tracker()` function
### 2. 3D Orientation Not Implemented (Expected)
**Status**: Documented as deferred, tests fail with `[!mayfail]` tags
**Current behavior**:
- Molniya tests fail z-coordinate checks (z = 0, should be non-zero)
- Generic inclined orbit test fails z-coordinate check
- Inclination parameter preservation test passes (config loading only)
**Required implementation**:
- Modify `src/orbital_mechanics.cpp` `orbital_elements_to_cartesian()`
- Apply rotation matrices for:
- Argument of periapsis (ω) - about z-axis
- Inclination (i) - about x-axis
- Longitude of ascending node (Ω) - about z-axis
- Use z-x-z Euler angle sequence
## Next Steps for Next Session
1. **Investigate OrbitTracker failure**:
- Debug why period test doesn't complete orbit
- Try increasing `MAX_SIMULATION_HOURS` from 15 to 20
- Check if tracker works differently for circular vs highly elliptical orbits
2. **Implement 3D orientation** (if desired):
- Add rotation matrix functions in `orbital_mechanics.cpp`
- Update `orbital_elements_to_cartesian()` to apply 3D rotations
- Test with Molniya configuration
- Remove `[!mayfail]` tags from tests that now pass
3. **Verify no regressions**:
- Run full test suite: `make test`
- Ensure all existing tests still pass
Loading…
Cancel
Save