Render videos eval mode #274
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
docs/src/visualizer.md
Outdated
| Adjust the screen size and color depth as needed. The `xvfb-run` wrapper allows Raylib to render without an attached display, which is convenient for servers and CI jobs. | ||
|
|
||
| ## Evaluation Rendering | ||
| You can batch render videos for multiple maps using the evaluation mode. This will render the first `num_maps`(limited by number of maps in the directory) from `map_dir` in parallel using the `visualize` binary. |
There was a problem hiding this comment.
can we randomize over this? It seems bad to always use the exact same maps
Author
There was a problem hiding this comment.
Oh so the PR intends to be able to render a bunch of videos(that the user knows scenarios for) for a given policy, so I figured it be best to be predictable instead of a random set of videos. Maybe you are getting confused with this PR and async render mode PR(#269).
| ; "both", "topdown", "agent"; Other args are passed from train confs | ||
| render_view_mode = "both" | ||
| ; Policy bin file used for rendering videos | ||
| render_policy_path = "resources/drive/puffer_drive_weights.bin" |
There was a problem hiding this comment.
this seems dangerous? Presumably we want to render the policies we are actually training
| void init_goal_positions(Drive *env); | ||
| float clipSpeed(float speed); | ||
| void sample_new_goal(Drive *env, int agent_idx); | ||
| int check_lane_aligned(Entity *car, Entity *lane, int geometry_idx); |
There was a problem hiding this comment.
this seems like it should not be here?
Author
There was a problem hiding this comment.
Oh i added it cuz I was getting compile error on my mac. forward declaration. I can make it a separate PR but it's just a one liner.
Greptile OverviewGreptile SummaryThis PR implements a parallel video rendering evaluation mode using ThreadPool to batch render videos for multiple maps in the drive environment. Key Changes:
Issues Found:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant eval() as eval()
participant Config as Config (drive.ini)
participant ThreadPool
participant Worker as Worker Thread
participant Binary as visualize binary
participant FS as File System
User->>eval(): puffer eval puffer_drive
eval()->>Config: Load render_videos_eval settings
Config-->>eval(): num_maps, map_dir, view_mode, num_workers
eval()->>FS: os.listdir(map_dir)
FS-->>eval(): List of .bin files
eval()->>eval(): Filter and sort .bin files
eval()->>FS: os.makedirs(output_dir)
eval()->>eval(): ensure_drive_binary()
eval()->>ThreadPool: Create ThreadPool(num_workers)
loop For each map in render_maps
ThreadPool->>Worker: Assign render_task(map_path)
Worker->>Worker: Build command with args
Worker->>Binary: subprocess.run(["xvfb-run", "./visualize", ...])
Binary->>FS: Write topdown_{map_name}.mp4
Binary->>FS: Write agent_{map_name}.mp4
Binary-->>Worker: Return code
alt returncode != 0
Worker->>User: Print error message
end
end
ThreadPool-->>eval(): All tasks complete
eval()->>User: Print completion message
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR adds an eval-mode path to batch-render PufferDrive videos in parallel using the visualize binary, plus a small C header update and configuration/docs wiring. It enables puffer eval puffer_drive to render multiple maps concurrently based on config values.
Changes:
- Introduced
render_videos_evalmode inpufferlib.pufferl.eval, using aThreadPoolto invoke thevisualizebinary across multiple.binmaps and write MP4s toresources/drive/render_videos. - Updated
drive.inieval configuration to include switches and options for video rendering (toggle, view mode, and policy path). - Added a forward declaration for
check_lane_alignedindrive.hand extendeddocs/src/visualizer.mdto describe the new evaluation rendering workflow.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
pufferlib/pufferl.py |
Adds a new elif render_videos_enabled branch to eval that collects map binaries, ensures the visualize binary is built, and runs parallel rendering jobs via ThreadPool. |
pufferlib/ocean/drive/drive.h |
Adds a forward declaration for check_lane_aligned to match its implementation and usage elsewhere in the file. |
pufferlib/config/ocean/drive.ini |
Wires new eval config flags (render_videos_eval, render_view_mode, render_policy_path) used by the eval rendering mode. |
docs/src/visualizer.md |
Documents the new eval-based batch rendering flow, including how it uses eval.num_maps, render_videos_eval, and vec.num_workers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
1128
to
1188
| elif render_videos_enabled: | ||
| # Renders first num_maps from map_dir using visualize binary | ||
| map_dir = args["eval"]["map_dir"] | ||
| num_maps = args["eval"]["num_maps"] | ||
| view_mode = args["eval"].get("render_view_mode", "both") | ||
| render_policy_path = args["eval"].get("render_policy_path", None) | ||
| num_workers = args["vec"]["num_workers"] | ||
|
|
||
| if num_maps > len(os.listdir(map_dir)): | ||
| num_maps = len(os.listdir(map_dir)) | ||
|
|
||
| render_maps = [os.path.join(map_dir, f) for f in sorted(os.listdir(map_dir)) if f.endswith(".bin")][:num_maps] | ||
| output_dir = "resources/drive/render_videos" | ||
| os.makedirs(output_dir, exist_ok=True) | ||
|
|
||
| # Rebuild visualize binary | ||
| ensure_drive_binary() | ||
|
|
||
| def render_task(map_path): | ||
| base_cmd = ( | ||
| ["./visualize"] | ||
| if sys.platform == "darwin" | ||
| else ["xvfb-run", "-a", "-s", "-screen 0 1280x720x24", "./visualize"] | ||
| ) | ||
| cmd = base_cmd.copy() | ||
| cmd.extend(["--map-name", map_path]) | ||
| train_configs = args["train"] | ||
| if train_configs.get("show_grid", False): | ||
| cmd.append("--show-grid") | ||
| if train_configs.get("obs_only", False): | ||
| cmd.append("--obs-only") | ||
| if train_configs.get("show_lasers", False): | ||
| cmd.append("--lasers") | ||
| if train_configs.get("show_human_logs", False): | ||
| cmd.append("--show-human-logs") | ||
| if train_configs.get("zoom_in", False): | ||
| cmd.append("--zoom-in") | ||
| if train_configs.get("frame_skip", 1) > 1: | ||
| cmd.extend(["--frame-skip", str(train_configs["frame_skip"])]) | ||
| cmd.extend(["--view", view_mode]) | ||
| if render_policy_path is not None: | ||
| cmd.extend(["--policy-name", render_policy_path]) | ||
|
|
||
| map_name = os.path.basename(map_path).replace(".bin", "") | ||
|
|
||
| if view_mode == "topdown" or view_mode == "both": | ||
| cmd.extend(["--output-topdown", output_dir + f"/topdown_{map_name}.mp4"]) | ||
| if view_mode == "agent" or view_mode == "both": | ||
| cmd.extend(["--output-agent", output_dir + f"/agent_{map_name}.mp4"]) | ||
|
|
||
| env_vars = os.environ.copy() | ||
| env_vars["ASAN_OPTIONS"] = "exitcode=0" | ||
| result = subprocess.run(cmd, cwd=os.getcwd(), capture_output=True, text=True, timeout=600, env=env_vars) | ||
| if result.returncode != 0: | ||
| print(f"Error rendering {map_name}: {result.stderr}") | ||
|
|
||
| if render_maps: | ||
| print(f"Rendering {len(render_maps)} from {map_dir} with {num_workers} workers...") | ||
| with ThreadPool(num_workers) as pool: | ||
| pool.map(render_task, render_maps) | ||
| print(f"Finished rendering videos to {output_dir}") |
There was a problem hiding this comment.
This new render_videos_eval branch introduces non-trivial behavior (building the visualize binary, constructing per-map subprocess commands, and dispatching them via a ThreadPool), but there are no tests covering this path; if it regresses, failures may only surface at runtime. Consider adding an automated test (potentially modeled after tests/test_drive_render.py) that exercises this eval mode end-to-end for a small number of maps to validate command construction and successful completion.
…stions by greptile
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Implemented parallel video rendering
evalmode using Thread Pool.Renders videos for first num_maps(eval) videos in map_dir(eval) and saves in resources/drive/videos.