Skip to content

Comments

Adding Rendering Support for Human Replay Evals#311

Open
Aditya-Gupta26 wants to merge 4 commits into3.0_betafrom
aditya/add_log_replay_evals
Open

Adding Rendering Support for Human Replay Evals#311
Aditya-Gupta26 wants to merge 4 commits into3.0_betafrom
aditya/add_log_replay_evals

Conversation

@Aditya-Gupta26
Copy link

This PR augments rendering support for evals. We can now specify an eval_render_interval, along with number of maps and a map_dir to run visualizations on.

This also fixes some issues with static agents (previously - static agents were not marked as experts while the mode was set to control_sdc_only)

@greptile-apps
Copy link

greptile-apps bot commented Feb 23, 2026

Greptile Summary

This PR adds rendering support for human replay evaluations with configurable intervals and map selection. It also fixes a bug where static agents weren't being marked as experts in control_sdc_only mode.

Key changes:

  • Added eval rendering configuration options (eval_render_enabled, eval_render_interval, eval_render_num_maps)
  • Implemented render_eval_videos function with async support and wandb logging under eval_render/ namespace
  • Fixed static agent expert marking logic in drive.h by checking control_mode == CONTROL_SDC_ONLY
  • Added control mode override support via CLI flag --control-mode
  • Added rendering of both active and expert static agent trajectories in visualization

Issues found:

  • Critical bug in pufferlib/pufferl.py:1777 where --log-trajectories flag is appended to base_cmd instead of cmd, preventing it from being executed
  • Minor typos in debug print statements in visualize.c

Confidence Score: 3/5

  • This PR has one critical logic bug that will prevent a feature from working correctly
  • The bug on line 1777 of pufferlib/pufferl.py prevents the --log-trajectories flag from being passed to the rendering command, which will cause human log trajectories not to render when expected. The rest of the implementation appears solid with proper error handling and async support.
  • pufferlib/pufferl.py requires immediate attention to fix the critical bug on line 1777

Important Files Changed

Filename Overview
pufferlib/ocean/drive/visualize.c Added control mode override, expert trajectory rendering, and --log-trajectories flag - contains typo in debug print
pufferlib/pufferl.py Added eval rendering system with async support and wandb logging - contains critical bug on line 1777
pufferlib/utils.py Added render_eval_videos function with proper error handling and video logging

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[PuffeRL.evaluate] -->|Check eval_render_enabled| B{Enabled & at interval?}
    B -->|Yes| C[Find latest checkpoint]
    B -->|No| Z[Skip]
    C --> D[Export model to .bin]
    D --> E{render_async?}
    E -->|Yes| F[Start multiprocessing.Process]
    E -->|No| G[Call render_eval_videos directly]
    F --> H[render_eval_videos runs in background]
    G --> I[render_eval_videos runs synchronously]
    H --> J[Put results in render_queue]
    I --> K[Log to wandb immediately]
    J --> L[check_render_queue polls queue]
    L --> M[Log to wandb with eval_render namespace]
    
    N[render_eval_videos] --> O[Select random maps from map_dir]
    O --> P[Build command with --control-mode override]
    P --> Q[Call ./visualize for each map]
    Q --> R[Generate topdown & agent videos]
    R --> S[Move videos to eval_videos dir]
Loading

Last reviewed commit: b329856

Copy link

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

5 files reviewed, 3 comments

Edit Code Review Agent Settings | Greptile


Weights *weights = load_weights(policy_name);
printf("Active agents in map: %d\n", env.active_agent_count);
printf("Static expoer agents in the map :%d\n", env.expert_static_agent_count);
Copy link

Choose a reason for hiding this comment

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

Typo: 'expoer' should be 'expert'

Suggested change
printf("Static expoer agents in the map :%d\n", env.expert_static_agent_count);
printf("Static expert agents in the map: %d\n", env.expert_static_agent_count);

cmd.append("--lasers")
if render_configs.get("show_human_logs", False):
cmd.append("--show-human-logs")
base_cmd.append("--log-trajectories")
Copy link

Choose a reason for hiding this comment

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

Appending to base_cmd instead of cmd - flag won't be included in the command that gets executed. Other flags correctly append to cmd.

Suggested change
base_cmd.append("--log-trajectories")
cmd.append("--log-trajectories")

Copy link
Author

Choose a reason for hiding this comment

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

Done

.map_name = (char *)map_name,
};

printf("Control Mode : %.2d", control_mode_int);
Copy link

Choose a reason for hiding this comment

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

Missing newline character in printf

Suggested change
printf("Control Mode : %.2d", control_mode_int);
printf("Control Mode: %d\n", control_mode_int);

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.

1 participant