Skip to content

Add deterministic multi-worker evaluation and seeded failure replay - #533

Open
Victorbares wants to merge 86 commits into
3.0from
vb/better_eval
Open

Add deterministic multi-worker evaluation and seeded failure replay#533
Victorbares wants to merge 86 commits into
3.0from
vb/better_eval

Conversation

@Victorbares

@Victorbares Victorbares commented Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Eval PR:

  1. Removes previous eval code and eval managers (notably EGL renderer and WOSAC eval will need a subsequent PR)

  2. Add seeding for deterministic replay.

  3. Works with multiprocessing.

  4. Works with mid-training evals, and nightly runs and other stuff

For example you can do the following:

Eval 1000 scenarios on CARLA and render the ones with an offroad

puffer eval puffer_drive carla_fast load_model_path=my_model.pt eval.render-filter=offroad

This will first run a evaluation on 1000 episodes, generate a CSV and then use the csv to know which episodes to render and thanks to seeding will replay them and render them.

WaelDLZ and others added 30 commits July 6, 2026 15:58
- Simplified file handling in `make_client` function in `render.h`.
- Removed deprecated `compare_episode_metrics.py` script.
- Added `benchmark_comparison.py` to aggregate benchmark results.
- Created `run_all_eval.sh` for streamlined evaluation of experiments.
- Introduced `run_all_latest_eval.py` to execute evaluations on the latest models.
- Implemented `run_failure_scenarios.py` to rerun failed scenarios based on metrics.
- Updated `launch_nightly_best.sh` script comments for clarity.
- Modified `nightly_report.py` to reflect new evaluation metrics.
- Enhanced unit tests in `test_drive_config.py` to validate configuration files.
- Added new test in `test_eval_zlib.py` to ensure agent capacity constraints are enforced.
@WaelDLZ
WaelDLZ marked this pull request as ready for review July 29, 2026 19:48
Comment thread .github/workflows/ci.yml
Comment on lines -124 to -157
smoke-replay:
name: Smoke Replay HTML
runs-on: ubuntu-latest
env:
PUFFER_CPU: 1
steps:
- name: Checkout code
uses: actions/checkout@v4

- name: Setup Python
uses: actions/setup-python@v5
with:
python-version: "3.11"

- name: Free up disk space
run: |
sudo rm -rf /usr/share/dotnet /usr/local/lib/android /opt/ghc
sudo apt-get clean
sudo rm -rf ~/.cache/pip /tmp/* /var/tmp/*

- name: Install pufferlib
env:
TMPDIR: ${{ runner.temp }}/build
PIP_NO_CACHE_DIR: 1
run: |
sudo apt-get update && sudo apt-get install -y build-essential cmake
python -m pip install -U pip pytest
pip install -e . --no-cache-dir
python setup.py build_ext --inplace --force

- name: Replay HTML test
run: python -m pytest -v tests/smoke_tests/test_validation_replay_html.py
timeout-minutes: 20

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand deleting this, but why not have a replacement test for the htmls?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I deleted a lot of tests to focus only on testing the 4 most important mechanisms of the PR:

  • multiprocessed eval dispatches map correctly and gather the correct number of scenarios
  • Seeding gets you exactly the same numbers for all metrics
  • Running a mid-training eval doesn't affect training
  • Capturing replays work (complex interaction between C and Python)

My idea was to focus on the most complex mechanisms that rely on both C and Python code, and could be broken by a coming PR that touches drive.h or binding.c, and thus avoid having 2k lines of tests for things that are "less complex"

But I'm very open to advice on it, if you think we should have tests for every mechanism because it's the best way to make the codebase robust for the future, we'll add them !

Comment thread notebooks/01_observations.py
Comment thread pufferlib/config/evaluation/benchmark.yaml Outdated
num_scenarios: 1000
num_maps: 8
max_agents_per_env: 50
scenario_length: 6000

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3600? Why 6000?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that's the setup Valentin used for CARLA eval.
The benchmarks in benchmark.yaml are a first draft, open to changing them to more appropriate values.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@eugenevinitsky I think a separate PR should fix the values in this file, and thiese values should come from a discussion between all of us to align our eval objectives

Comment on lines +56 to +65
- name: womd_multi
seed: 42
simulation_mode: replay
num_scenarios: 1000
num_maps: 1000
max_agents_per_env: 128
scenario_length: 91
control_mode: control_vehicles
map_dir: pufferlib/resources/drive/binaries/womd

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do these binaries actually exist?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're not part of the github repo, but it would be up to people to download them and put them at the correct location, maybe we can rename womd to womd_valid

seed: 42
# Null disables evaluation during training.
evaluation_interval_epochs: null
evaluation_benchmarks: carla_fast

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how do we ask for multiple evals? a list?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, why does this arg not live under eval?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, why does this arg not live under eval?

Made this choice because it's dedicated to mid-training evals, so I felt like it should be configured alongside other training args

how do we ask for multiple evals? a list?

Puttting: evaluation_benchmarks: carla_fast,womd_single,nuplan_multi for example, Victor is adding a comment to explain it

Comment thread pufferlib/config/puffer_drive.yaml Outdated
output_name: null
# Capture and render every scenario during the standard benchmark pass.
render_scenarios: false
# Render scenarios where any configured metric is positive; use all for every failure type; null disables it.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all for every failure type suggests that there is an alternative to all or null?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exact, the comments aren't clear:

render_filter can accept any list metrics from the csv (even non-sense like seed), and select rows where at least one of the selected metrics is positive.

For example: render_filter=offroad,at_fault_collisions would render every offroad and every at-fault collision.

render_filter=all is a shortcut for the 3 infractions: offroad+collision+traffic_light, it could be renamed all_infractions for example.

Ideally we wanted to have it working for any sets of requests like "reward>0.2 AND lane_align<-0.1" but it would have make the PR a bit messy so we tried to find a compromise here.

max_rendered_failures: null
# Existing episode metrics CSV to replay without running the benchmark first.
failure_replay_csv: null
capture_observations: false

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does this do?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The goal is to be able to replay failures from a csv directly without executing the bench before.
Like this command :

puffer eval puffer_drive carla \
    load_model_path=experiments/mimolette/models/model_puffer_drive_003815.pt \
    eval.failure_replay_csv=experiments/mimolette/eval/carla/<timestamp>/episode_metrics.csv \
    eval.render_filter=all \
    eval.max_rendered_failures=10

# Existing episode metrics CSV to replay without running the benchmark first.
failure_replay_csv: null
capture_observations: false
observation_replay_wave_size: 16

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is a wave size?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Observation capture uses substantial RAM, especially for CARLA scenarios with 6,000 timesteps. To limit peak memory usage, failure scenarios are processed in sequential waves.
observation_replay_wave_size sets the maximum number of scenarios whose observations are captured concurrently.

This is active when eval.capture_observations=true

May be rename "chunk_size" instead of "wave_size" ?

failure_replay_csv: null
capture_observations: false
observation_replay_wave_size: 16
observation_replay_writer_count: 4

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what is this number?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's also something to limit RAM Usage during zlib serialization.
eval.observation_replay_writer_count controls how many replay files are serialized and compressed concurrently after observation capture.
This is also only active when eval.capture_observations=true

PyList_SetItem(agent_offsets, env_count, PyLong_FromLong(total_agent_count));
total_agent_count += env->active_agent_count;
env_count++;
// In eval, keep whole scenes: a scene that would overflow the buffer is

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a little confused by this comment, what is a partial scene?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue is, each worker has a budget of agents it can handle (e.g. num_agents=1000), and we want to fill it with scenarios.

If for examples the maps all have 400 agents (like huge replay maps) we should only assign the two first maps i.e. 800 agents, and not try to fit the 3rd map by throwing away the 200 exceeding agents.

So like the last map is never complete (except the very rare case where your maps add up perfectly to your num_agents), and we prefer to have like 0 padding than a non-complete ("partial") scene.

I agree the comment could be clearer

@eugenevinitsky

Copy link
Copy Markdown

Okay working through this bit by bit but my first concern is that a lot of eval logic has migrated into eval.py so if you wanted to later change the evaluator, you would have to hunt down the corresponding changes in drive.py. It feels like breaking some abstraction barriers? I'm raising the issue but I don't have a solution yet

@riccardosavorgnan

Copy link
Copy Markdown
Collaborator

[non-blocking] are all scripts in scripts/eval now dead code? should we remove them?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants