diff --git a/ANALYSIS_SUMMARY.md b/ANALYSIS_SUMMARY.md new file mode 100644 index 00000000..e3e0b9d9 --- /dev/null +++ b/ANALYSIS_SUMMARY.md @@ -0,0 +1,148 @@ +# Analysis Summary: Code Slowdown After Upstream Sync + +## Question Asked +> "Look at the differences before and after I synced with upstream (ci-group), and tell me what possible reasons for code slowdown there could be for my examples/A3_modified.py" + +## Answer + +### Primary Cause: Duration Parameter Ambiguity in `continue_simple_runner()` + +The upstream sync added a new function `continue_simple_runner()` to `src/ariel/utils/runners.py` that has an **ambiguous duration parameter**. + +#### The Problem + +```python +def continue_simple_runner(model, data, duration=10.0, steps_per_loop=100): + while data.time < duration: # ⚠️ Is this absolute time or incremental? + mujoco.mj_step(model, data, nstep=steps_per_loop) +``` + +Your configuration has: +```python +duration_flat: int = 15 # First segment +duration_rugged: int = 30 # Second segment +duration_elevated: int = 55 # Third segment +``` + +#### Two Possible Interpretations + +**Interpretation 1: Absolute Times** (current implementation) +- Simulation runs until `data.time` reaches the specified value +- Total time: 15 + (30-15) + (55-30) = **55 seconds** + +**Interpretation 2: Incremental Durations** (possible original intent) +- Simulation adds the specified duration to current time +- Total time: 15 + 30 + 55 = **100 seconds** + +#### Impact: 82% Difference! + +``` +Current behavior: [====55 seconds====] +Possible intent: [==============100 seconds==============] +Missing time: [====45 seconds====] +``` + +### Why This Causes "Slowdown" + +If your code expects 100s simulations but gets 55s: + +1. **Robots travel shorter distances** → appear less capable +2. **Fitness scores are lower** → different selection in EA +3. **Fewer robots pass checkpoints** → different evolutionary pressure +4. **EA makes different progress** → appears to "slow down" or stagnate +5. **Results don't match expectations** → confusion about performance + +### How to Fix + +**Option 1: Make Duration Incremental** (if 100s is correct) + +Edit `src/ariel/utils/runners.py`, line 64: + +```python +def continue_simple_runner(model, data, duration=10.0, steps_per_loop=100): + """Continue a simple headless simulation for an additional duration.""" + end_time = data.time + duration # ← Add this line + while data.time < end_time: # ← Change this line + mujoco.mj_step(model, data, nstep=steps_per_loop) +``` + +**Option 2: Update Config** (if 55s is correct) + +Rename variables in `examples/A3_modified.py` for clarity: + +```python +duration_initial: int = 15 # 0 → 15s +total_time_after_rugged: int = 30 # 0 → 30s total +total_time_after_elevated: int = 55 # 0 → 55s total +``` + +### How to Verify Which Is Correct + +1. **Run the test:** + ```bash + python test_duration_semantics.py + ``` + +2. **Check your expectations:** + - Should simulations be 55s or 100s total? + - Are checkpoint locations reasonable for 55s vs 100s? + - What are historical fitness value ranges? + +3. **Monitor actual behavior:** + - Add logging to see actual simulation times + - Check checkpoint pass rates + - Compare fitness distributions + +### Files Created for This Analysis + +| File | Purpose | +|------|---------| +| `README_ANALYSIS.md` | Master index and overview | +| `QUICKSTART_ANALYSIS.md` | Quick summary with fixes | +| `SLOWDOWN_ANALYSIS.md` | Detailed technical analysis | +| `TIMELINE_VISUALIZATION.txt` | Visual timeline comparison | +| `test_duration_semantics.py` | Runnable demonstration | +| `ANALYSIS_SUMMARY.md` | This file - executive summary | + +### Recommendation + +**Most Likely:** The duration parameters were meant to be incremental (100s total), so: + +1. ✅ Apply Option 1 fix to `src/ariel/utils/runners.py` +2. ✅ Test with a few EA runs +3. ✅ Verify fitness values return to expected ranges +4. ✅ Confirm checkpoint pass rates improve + +### Additional Minor Issues + +While analyzing, I also found these minor issues (not primary causes of slowdown): + +1. **No controller state reset** in `continue_simple_runner` + - Impact: Minimal, controller stays active + - Urgency: Low + +2. **Checkpoint timing assumptions** + - Impact: Depends on duration interpretation + - Fix: Will be correct once duration is fixed + +### Upstream Changes + +The merge commit `b465e55` ("Merge remote-tracking branch 'upstream/main'") added: +- New `continue_simple_runner()` function +- Many infrastructure files (`.devcontainer/`, `.typings/`, etc.) +- No direct changes to your `A3_modified.py` (it's your custom file) + +### Root Cause Analysis + +This issue is a classic example of: +- **Ambiguous parameter naming**: `duration` could mean different things +- **Missing documentation**: No clear spec of absolute vs incremental +- **Silent behavior change**: No error, just different behavior + +Better API design would use: +- `target_time` for absolute times +- `additional_duration` for incremental times + +--- + +**Bottom Line:** The most likely cause of your code "slowdown" is that simulations are running for 55 seconds instead of the expected 100 seconds, due to how the new `continue_simple_runner()` function interprets duration parameters. The fix is a simple 2-line change to make durations incremental instead of absolute. diff --git a/QUICKSTART_ANALYSIS.md b/QUICKSTART_ANALYSIS.md new file mode 100644 index 00000000..bd4fed99 --- /dev/null +++ b/QUICKSTART_ANALYSIS.md @@ -0,0 +1,129 @@ +# Summary: Possible Reasons for Code Slowdown in A3_modified.py + +After analyzing the differences before and after syncing with upstream (ci-group), here are the **possible reasons for code slowdown** in `examples/A3_modified.py`: + +## 🎯 Primary Issue: Duration Semantics Ambiguity + +The upstream sync added a new `continue_simple_runner()` function that has an **ambiguous duration parameter**: + +```python +def continue_simple_runner(model, data, duration=10.0, steps_per_loop=100): + while data.time < duration: # ⚠️ Treats duration as ABSOLUTE time + mujoco.mj_step(model, data, nstep=steps_per_loop) +``` + +### The Problem + +Your configuration has these values: +```python +duration_flat: int = 15 +duration_rugged: int = 30 +duration_elevated: int = 55 +``` + +**Question**: Are these meant to be: +1. **Absolute times** (current behavior): Run until simulation reaches this time +2. **Incremental durations** (possible original intent): Add this much time + +### Impact on Simulation Time + +| Interpretation | Flat | Rugged (added) | Elevated (added) | **TOTAL** | +|----------------|------|----------------|------------------|-----------| +| **Absolute** (current) | 0→15s (15s) | 15→30s (15s) | 30→55s (25s) | **55 seconds** | +| **Incremental** (possible) | 0→15s (15s) | 15→45s (30s) | 45→100s (55s) | **100 seconds** | + +**Difference: 45 seconds (82% increase!)** + +## 🔍 Why This Causes "Slowdown" + +If your original implementation (before sync) used **incremental durations**, and the upstream version uses **absolute times**, then: + +1. ⏱️ **Simulations run for less time**: 55s instead of 100s per robot +2. 🏃 **Robots travel shorter distances**: Less time to reach checkpoints/targets +3. 📉 **Lower fitness scores**: Less distance covered = worse fitness +4. 🎯 **Fewer checkpoint passes**: Robots might not reach rugged/elevated terrains +5. 🔄 **Different EA behavior**: Selection pressure changes, different solutions emerge +6. ⚡ **Perceived "slowdown"**: EA not making progress as expected + +## 🧪 How to Verify + +Run the test script to see the difference: + +```bash +python test_duration_semantics.py +``` + +This will show you exactly how the two interpretations differ. + +## ✅ Recommended Fix + +### Option 1: Change to Incremental Duration (Recommended if original behavior was incremental) + +**File**: `src/ariel/utils/runners.py` + +```python +def continue_simple_runner( + model: mujoco.MjModel, + data: mujoco.MjData, + duration: float = 10.0, + steps_per_loop: int = 100, +) -> None: + """Continue a simple headless simulation for an additional duration.""" + end_time = data.time + duration # ✓ Make it incremental + while data.time < end_time: + mujoco.mj_step(model, data, nstep=steps_per_loop) +``` + +**And update config to be clear**: +```python +duration_flat: int = 15 # Initial simulation time +duration_rugged_extra: int = 30 # Additional time for rugged (renamed for clarity) +duration_elevated_extra: int = 55 # Additional time for elevated (renamed for clarity) +``` + +### Option 2: Keep Absolute Times but Fix Config Values + +If absolute times are correct, update config to match: + +```python +duration_flat: int = 15 # Run until time = 15 +total_time_rugged: int = 30 # Run until time = 30 (15s added) +total_time_elevated: int = 55 # Run until time = 55 (25s added) +``` + +### Option 3: Restore Original Behavior + +If you know what the original implementation was before the sync, you could restore that behavior. + +## 📋 Quick Check + +To determine which interpretation is correct, check: + +1. **Are robots reaching checkpoints?** + - If very few robots pass checkpoints → likely running for too short + +2. **What are typical fitness values?** + - Compare with results before the sync + +3. **How long should simulations actually run?** + - Do you expect robots to have 55s or 100s to complete the course? + +## 📄 Full Details + +See `SLOWDOWN_ANALYSIS.md` for complete technical analysis including: +- Detailed code comparison +- Additional potential issues +- Multiple fix options +- Testing recommendations + +## 🚀 Next Steps + +1. **Determine intended behavior**: Should durations be incremental or absolute? +2. **Choose a fix option**: Based on your determination +3. **Update code/config**: Implement the chosen fix +4. **Test**: Verify robots behave as expected +5. **Compare results**: Check if EA performance is restored + +--- + +**Bottom Line**: The duration parameter ambiguity in `continue_simple_runner` is the most likely cause of unexpected behavior after syncing with upstream. The fix is simple once you determine the intended semantics. diff --git a/README_ANALYSIS.md b/README_ANALYSIS.md new file mode 100644 index 00000000..0c3f09d4 --- /dev/null +++ b/README_ANALYSIS.md @@ -0,0 +1,191 @@ +# Slowdown Analysis Results + +This directory contains a comprehensive analysis of potential code slowdown issues in `examples/A3_modified.py` after syncing with upstream (ci-group/ariel). + +## 📁 Files in This Analysis + +### 1. **QUICKSTART_ANALYSIS.md** ⭐ START HERE + - **Purpose**: User-friendly summary with quick recommendations + - **Best for**: Getting a fast overview and actionable fixes + - **Contains**: + - Simple explanation of the issue + - Comparison table showing 55s vs 100s difference + - Three fix options with code examples + - Quick verification steps + +### 2. **SLOWDOWN_ANALYSIS.md** + - **Purpose**: Comprehensive technical analysis + - **Best for**: Understanding all technical details + - **Contains**: + - Complete code comparisons + - Detailed explanation of duration semantics + - Analysis of all potential performance issues + - Multiple solution approaches + - Testing recommendations + +### 3. **TIMELINE_VISUALIZATION.txt** + - **Purpose**: Visual timeline comparison + - **Best for**: Seeing the difference at a glance + - **Contains**: + - ASCII timeline showing 55s vs 100s + - Checkpoint locations + - Code implementation comparison + - EA impact analysis + +### 4. **test_duration_semantics.py** + - **Purpose**: Executable demonstration + - **Best for**: Seeing the issue in action + - **Run with**: `python test_duration_semantics.py` + - **Shows**: How duration parameters are interpreted differently + +## 🎯 TL;DR - The Main Issue + +The `continue_simple_runner()` function added from upstream interprets `duration` as an **absolute time** instead of an **incremental duration**. + +**Your config:** +```python +duration_flat: int = 15 # ✓ Clear: initial simulation +duration_rugged: int = 30 # ⚠️ Ambiguous: total time (30s) or added time (30s)? +duration_elevated: int = 55 # ⚠️ Ambiguous: total time (55s) or added time (55s)? +``` + +**Current behavior:** +- Total simulation time: **55 seconds** +- Breakdown: 15s + 15s + 25s + +**Possible original intent:** +- Total simulation time: **100 seconds** +- Breakdown: 15s + 30s + 55s + +**Impact:** 82% difference in simulation time! + +## ✅ Quick Fix + +**File:** `src/ariel/utils/runners.py` + +**Change this:** +```python +def continue_simple_runner(model, data, duration=10.0, steps_per_loop=100): + while data.time < duration: # ⚠️ Absolute time + mujoco.mj_step(model, data, nstep=steps_per_loop) +``` + +**To this:** +```python +def continue_simple_runner(model, data, duration=10.0, steps_per_loop=100): + end_time = data.time + duration # ✓ Incremental time + while data.time < end_time: + mujoco.mj_step(model, data, nstep=steps_per_loop) +``` + +## 🔍 How to Verify + +### Step 1: Check Current Behavior +```bash +python test_duration_semantics.py +``` + +### Step 2: Check Your Results +Look at your simulation logs: +- How long do simulations actually run? +- Are robots reaching checkpoints? +- What are typical fitness values? + +### Step 3: Compare with Expectations +- Do you expect 55s or 100s total simulation time? +- Are checkpoint locations reasonable for the actual simulation time? + +## 📊 Impact on Your Code + +### If simulations should be 100s (not 55s): + +**Symptoms you might see:** +- ❌ Robots not traveling far enough +- ❌ Lower fitness scores than expected +- ❌ EA making slower progress +- ❌ Fewer robots passing checkpoints +- ❌ Different solutions being selected + +**Why it feels like "slowdown":** +- EA appears to make less progress per generation +- Takes more generations to achieve same results +- Selection pressure is different + +### If simulations should be 55s (current behavior is correct): + +Then you might need to: +- ✓ Adjust checkpoint locations +- ✓ Adjust fitness function expectations +- ✓ Verify duration values match intent + +## 🚀 Recommended Action Plan + +1. **Determine Intent** (5 minutes) + - Decide: Should total simulation be 55s or 100s? + - Check if robots are reaching checkpoints as expected + - Review fitness value ranges + +2. **Choose Fix** (2 minutes) + - If 100s → Apply the code fix above + - If 55s → Update config comments for clarity + +3. **Test** (10 minutes) + - Run test script + - Run a few EA generations + - Verify checkpoint pass rates + +4. **Validate** (30 minutes) + - Check fitness distributions + - Compare with previous results + - Ensure EA is progressing as expected + +## 📝 Additional Notes + +### Other Potential Issues Found + +While the duration semantics is the PRIMARY issue, the analysis also identified: + +1. **Missing controller reset**: `continue_simple_runner` doesn't reset control state + - Impact: Probably minor, controller is still active + - Fix: Not urgent unless you see unexpected behavior + +2. **Checkpoint logic timing**: Relies on absolute time values + - Impact: Works correctly if durations are interpreted correctly + - Fix: Ensure duration semantics match checkpoint expectations + +### Files Modified by Upstream Sync + +The merge commit `b465e55` added: +- `src/ariel/utils/runners.py` - Contains the `continue_simple_runner` function +- Many other upstream changes (see `.devcontainer/`, `.typings/`, etc.) + +Your `examples/A3_modified.py` is a custom file that uses the upstream runners. + +## 📞 Need Help? + +1. **Read**: Start with `QUICKSTART_ANALYSIS.md` +2. **Visualize**: Check `TIMELINE_VISUALIZATION.txt` +3. **Test**: Run `test_duration_semantics.py` +4. **Deep Dive**: Read `SLOWDOWN_ANALYSIS.md` for all details + +## 🎓 Understanding the Root Cause + +The issue stems from an **ambiguous parameter name**: + +```python +def continue_simple_runner(duration): + # Is 'duration' the END TIME or the AMOUNT OF TIME to add? +``` + +Better parameter names would be: +- `target_time` (for absolute) or +- `additional_duration` (for incremental) + +This is a classic API design issue that can lead to subtle bugs! + +--- + +**Generated by:** Automated analysis of upstream sync changes +**Date:** 2025-10-10 +**Repository:** coopa33/ariel +**Upstream:** ci-group/ariel diff --git a/SLOWDOWN_ANALYSIS.md b/SLOWDOWN_ANALYSIS.md new file mode 100644 index 00000000..686bf5b3 --- /dev/null +++ b/SLOWDOWN_ANALYSIS.md @@ -0,0 +1,161 @@ +# Code Slowdown Analysis: A3_modified.py + +## Summary +After syncing with upstream (ci-group/ariel), there are several potential reasons for code slowdown in `examples/A3_modified.py`. + +## Changes from Upstream Sync + +The merge commit `b465e55` added the following new function to `src/ariel/utils/runners.py`: + +```python +def continue_simple_runner( + model: mujoco.MjModel, + data: mujoco.MjData, + duration: float = 10.0, + steps_per_loop: int = 100, +) -> None: + """Continue a simple headless simulation for a given duration.""" + while data.time < duration: + mujoco.mj_step(model, data, nstep=steps_per_loop) +``` + +## Potential Slowdown Issues + +### 1. **Duration Parameter Semantic Issue** ⚠️ CRITICAL + +**Problem**: The `continue_simple_runner` function interprets `duration` as an **absolute time** to run until, not an **incremental duration** to add. + +**Current Behavior**: +```python +# In examples/A3_modified.py, line 536-555: +simple_runner(model, data, duration=15) # Runs until data.time = 15 + +# Then if checkpoint passed: +continue_simple_runner(model, data, duration=30) # Runs until data.time = 30 (only 15 more seconds!) + +# Then if next checkpoint passed: +continue_simple_runner(model, data, duration=55) # Runs until data.time = 55 (only 25 more seconds!) +``` + +**Configuration Values** (lines 60-62): +```python +duration_flat: int = 15 # Initial simulation time +duration_rugged: int = 30 # TOTAL time including rugged terrain +duration_elevated: int = 55 # TOTAL time including elevated terrain +``` + +**Potential Issue**: If these values were previously interpreted as **incremental durations**, your robots would have run for: +- Before: 15 + 30 + 55 = 100 seconds total +- After sync: 15 + 15 + 25 = 55 seconds total + +This would explain significant performance differences! + +### 2. **Missing State Reset or Controller Updates** + +**Problem**: `continue_simple_runner` doesn't reset any controller state or perform any setup that `simple_runner` does. + +**Comparison**: +```python +# simple_runner (lines 34-41): +def simple_runner(...): + mujoco.mj_resetData(model, data) # ✓ Resets simulation + data.ctrl = RNG.normal(scale=0.1, size=model.nu) # ✓ Sets control + while data.time < duration: + mujoco.mj_step(model, data, nstep=steps_per_loop) + +# continue_simple_runner (lines 64-65): +def continue_simple_runner(...): + while data.time < duration: # ✗ No reset, no control setup + mujoco.mj_step(model, data, nstep=steps_per_loop) +``` + +**Impact**: The controller behavior might differ between the initial and continuation phases, affecting robot performance. + +### 2. **Checkpoint Evaluation Logic** + +**Problem**: Checkpoints are evaluated AFTER the initial simulation completes, using the final position. + +**Code** (lines 542-555): +```python +simple_runner(model, data, duration=duration) +# Simulation completes, robot position is now fixed + +# Check if checkpoint was passed during simulation: +if passed_checkpoint(sim_config.checkpoint_rugged, controller.tracker.history["xpos"][0]): + continue_simple_runner(...) # Continue from current position +``` + +**Potential Issues**: +- If the robot's position at time=15 doesn't meet the checkpoint, no additional simulation runs +- Fitness evaluation might be based on incomplete simulations +- The checkpoint positions might need adjustment based on the actual duration semantics + +### 3. **Performance Implications for Evolutionary Algorithm** + +If simulations are running for shorter durations than expected: + +1. **Fitness Evaluation**: Robots evaluated for less time → different fitness scores +2. **Selection Pressure**: Different individuals selected as "best" +3. **Convergence**: EA might converge to different solutions +4. **Computational Time**: Shorter simulations = faster, but potentially less accurate results + +## Recommendations + +### Option 1: Fix the Duration Semantics (RECOMMENDED) + +Change `continue_simple_runner` to accept incremental duration: + +```python +def continue_simple_runner( + model: mujoco.MjModel, + data: mujoco.MjData, + duration: float = 10.0, + steps_per_loop: int = 100, +) -> None: + """Continue a simple headless simulation for an additional duration.""" + end_time = data.time + duration # Make it incremental + while data.time < end_time: + mujoco.mj_step(model, data, nstep=steps_per_loop) +``` + +And update the config to reflect incremental values: + +```python +@dataclass +class EAConfig: + duration_flat: int = 15 # Initial simulation time + duration_rugged: int = 15 # Additional time for rugged (was 30) + duration_elevated: int = 25 # Additional time for elevated (was 55) +``` + +### Option 2: Update Configuration to Match New Semantics + +Keep the function as-is, but rename config values for clarity: + +```python +@dataclass +class EAConfig: + duration_flat: int = 15 # Initial simulation time + total_time_after_rugged: int = 30 # Total time including rugged checkpoint + total_time_after_elevated: int = 55 # Total time including elevated checkpoint +``` + +### Option 3: Verify Current Behavior Matches Intent + +If the current behavior IS correct: +1. Verify that robots are reaching expected checkpoints +2. Confirm fitness values are in expected ranges +3. Check that evolutionary progress is occurring as expected + +## Testing Recommendations + +1. **Add timing assertions** to verify actual simulation durations +2. **Log checkpoint pass rates** to ensure robots are performing as expected +3. **Compare fitness distributions** before and after sync +4. **Profile simulation runs** to measure actual computational time + +## Conclusion + +The most likely cause of slowdown or performance change is the **ambiguous duration parameter semantics** in `continue_simple_runner`. The function treats `duration` as an absolute time rather than an increment, which may not match the original intention of the configuration values. + +**Recommended Action**: Modify `continue_simple_runner` to use incremental durations (Option 1) and update configuration accordingly. diff --git a/TIMELINE_VISUALIZATION.txt b/TIMELINE_VISUALIZATION.txt new file mode 100644 index 00000000..db472308 --- /dev/null +++ b/TIMELINE_VISUALIZATION.txt @@ -0,0 +1,106 @@ +``` +SIMULATION TIMELINE COMPARISON +================================ + +Configuration Values: + duration_flat: 15 + duration_rugged: 30 + duration_elevated: 55 + +───────────────────────────────────────────────────────────────────────────────── + +CURRENT BEHAVIOR (Absolute Time Interpretation): +───────────────────────────────────────────────────────────────────────────────── + + 0 5 10 15 20 25 30 35 40 45 50 55 + |--------|--------|--------|--------|--------|--------|--------|--------|--------|--------|--------| + [========FLAT TERRAIN======] + [======RUGGED TERRAIN=====] + [========ELEVATED TERRAIN========] + + └─ simple_runner(15) └─ continue(30) └─ continue(55) + Runs: 0→15 (15s) Runs: 15→30 (15s) Runs: 30→55 (25s) + + TOTAL TIME: 55 seconds + +───────────────────────────────────────────────────────────────────────────────── + +POSSIBLE ORIGINAL INTENT (Incremental Duration Interpretation): +───────────────────────────────────────────────────────────────────────────────── + + 0 10 20 30 40 50 60 70 80 90 100 + |--------|--------|--------|--------|--------|--------|--------|--------|--------|--------| + [========FLAT=======] + [==============RUGGED TERRAIN==============] + [====================ELEVATED====================] + + └─ simple_runner(15) └─ continue(+30) └─ continue(+55) + Runs: 0→15 (15s) Runs: 15→45 (30s more) Runs: 45→100 (55s more) + + TOTAL TIME: 100 seconds + +───────────────────────────────────────────────────────────────────────────────── + +DIFFERENCE: 45 seconds (82% more simulation time in incremental version) + +───────────────────────────────────────────────────────────────────────────────── + +CHECKPOINT LOCATIONS: + checkpoint_rugged: x = 0.6 (should be reached around 15s mark) + checkpoint_elevated: x = 2.4 (should be reached around 30s mark) + +With ABSOLUTE times (55s total): + - Robot has 15s to reach x=0.6 ✓ (if moving ~0.04 units/s) + - Robot has 30s total to reach x=2.4 ✓ (if moving ~0.08 units/s) + +With INCREMENTAL times (100s total): + - Robot has 15s to reach x=0.6 ✓ (same) + - Robot has 45s total to reach x=2.4 ✓ (more time, easier to reach) + - Robot has 100s total to reach x=5.0 target ✓ (much more time) + +───────────────────────────────────────────────────────────────────────────────── + +CODE IMPLEMENTATION: +───────────────────────────────────────────────────────────────────────────────── + +Current (Absolute): | Proposed (Incremental): + | +def continue_simple_runner( | def continue_simple_runner( + model, data, | model, data, + duration=10.0, | duration=10.0, + steps_per_loop=100): | steps_per_loop=100): + | + while data.time < duration: | end_time = data.time + duration + mujoco.mj_step(...) | while data.time < end_time: + | mujoco.mj_step(...) + | +Example with duration=30: | Example with duration=30: +- If data.time = 15 | - If data.time = 15 +- Runs until: data.time = 30 | - Runs until: data.time = 45 +- Duration added: 15 seconds | - Duration added: 30 seconds + +───────────────────────────────────────────────────────────────────────────────── + +IMPACT ON EVOLUTIONARY ALGORITHM: +───────────────────────────────────────────────────────────────────────────────── + +With 55s simulations (current): + • Less time to evolve effective locomotion + • Fitness based on shorter distances + • Fewer generations needed per run (faster, but less accurate) + • May select for "sprinters" rather than "marathon runners" + +With 100s simulations (incremental): + • More time for robots to demonstrate capabilities + • Fitness based on longer distances + • More generations needed per run (slower, but more accurate) + • May select for sustained, efficient locomotion + +If you expected 100s but got 55s, the EA would appear to "slow down" because: + ✗ Robots don't travel as far as expected + ✗ Fitness scores are lower than expected + ✗ EA makes slower progress toward goals + ✗ Fewer robots pass checkpoints + +───────────────────────────────────────────────────────────────────────────────── +``` diff --git a/test_duration_semantics.py b/test_duration_semantics.py new file mode 100644 index 00000000..78ab68a3 --- /dev/null +++ b/test_duration_semantics.py @@ -0,0 +1,156 @@ +#!/usr/bin/env python3 +""" +Test script to demonstrate the duration semantics issue in continue_simple_runner. + +This script shows how the current implementation interprets durations as absolute times +rather than incremental additions, which could lead to unexpected simulation durations. +""" + +class MockData: + """Mock MuJoCo data object for testing.""" + def __init__(self): + self.time = 0.0 + + def step(self, amount=1): + """Simulate a time step.""" + self.time += amount + + +def simple_runner_behavior(data, duration): + """Simulates the behavior of simple_runner.""" + # Resets data.time to 0 + data.time = 0.0 + + # Run until duration + while data.time < duration: + data.step(1) # Each step adds 1 second + + print(f"After simple_runner(duration={duration}): data.time = {data.time}") + + +def continue_simple_runner_current_behavior(data, duration): + """Simulates CURRENT behavior of continue_simple_runner (absolute time).""" + # Does NOT reset data.time + # Runs until absolute time reaches duration + + start_time = data.time + while data.time < duration: + data.step(1) + + elapsed = data.time - start_time + print(f"After continue_simple_runner(duration={duration}): " + f"data.time = {data.time}, elapsed = {elapsed}") + + +def continue_simple_runner_expected_behavior(data, duration): + """Simulates EXPECTED behavior of continue_simple_runner (incremental time).""" + # Does NOT reset data.time + # Runs for an ADDITIONAL duration + + start_time = data.time + end_time = data.time + duration + + while data.time < end_time: + data.step(1) + + elapsed = data.time - start_time + print(f"After continue_simple_runner_incremental(duration={duration}): " + f"data.time = {data.time}, elapsed = {elapsed}") + + +def test_current_implementation(): + """Test the current implementation behavior.""" + print("=" * 70) + print("CURRENT IMPLEMENTATION (Absolute Time Interpretation)") + print("=" * 70) + + data = MockData() + + # Simulate the A3_modified.py behavior + print("\n1. Initial flat terrain simulation:") + simple_runner_behavior(data, duration=15) + + print("\n2. Continue for rugged terrain (if checkpoint passed):") + continue_simple_runner_current_behavior(data, duration=30) + + print("\n3. Continue for elevated terrain (if checkpoint passed):") + continue_simple_runner_current_behavior(data, duration=55) + + print(f"\n📊 TOTAL SIMULATION TIME: {data.time} seconds") + print() + + +def test_expected_implementation(): + """Test the expected incremental implementation behavior.""" + print("=" * 70) + print("EXPECTED IMPLEMENTATION (Incremental Time Interpretation)") + print("=" * 70) + + data = MockData() + + # If durations were meant to be incremental + print("\n1. Initial flat terrain simulation:") + simple_runner_behavior(data, duration=15) + + print("\n2. Continue for rugged terrain (if checkpoint passed):") + continue_simple_runner_expected_behavior(data, duration=30) + + print("\n3. Continue for elevated terrain (if checkpoint passed):") + continue_simple_runner_expected_behavior(data, duration=55) + + print(f"\n📊 TOTAL SIMULATION TIME: {data.time} seconds") + print() + + +def test_configuration_interpretation(): + """Show different interpretations of the configuration.""" + print("=" * 70) + print("CONFIGURATION VALUE INTERPRETATIONS") + print("=" * 70) + + print("\nFrom EAConfig:") + print(" duration_flat: int = 15") + print(" duration_rugged: int = 30") + print(" duration_elevated: int = 55") + + print("\n📌 Interpretation 1: ABSOLUTE TIMES (current behavior)") + print(" - Flat terrain: 0 → 15 seconds (15 seconds total)") + print(" - Rugged terrain: 15 → 30 seconds (15 seconds added)") + print(" - Elevated terrain: 30 → 55 seconds (25 seconds added)") + print(" - TOTAL: 55 seconds") + + print("\n📌 Interpretation 2: INCREMENTAL DURATIONS (possible original intent)") + print(" - Flat terrain: 0 → 15 seconds (15 seconds)") + print(" - Rugged terrain: 15 → 45 seconds (30 seconds added)") + print(" - Elevated terrain: 45 → 100 seconds (55 seconds added)") + print(" - TOTAL: 100 seconds") + + print("\n⚠️ DIFFERENCE: 45 seconds (82% increase in simulation time!)") + print() + + +def main(): + """Run all tests.""" + test_configuration_interpretation() + print() + test_current_implementation() + print() + test_expected_implementation() + + print("=" * 70) + print("CONCLUSION") + print("=" * 70) + print(""" +The difference in interpretation could cause: +1. ⏱️ Simulations to run for different durations (55s vs 100s) +2. 🎯 Different checkpoint pass rates +3. 📊 Different fitness evaluations +4. 🔄 Different EA convergence behavior +5. ⚡ Perceived "slowdown" if expectations were based on longer runs + +Recommendation: Clarify the intended behavior and update code/config accordingly. +""") + + +if __name__ == "__main__": + main()