Skip to content

Conversation

@mpragnay
Copy link

@mpragnay mpragnay commented Jan 31, 2026

  • Added args parsing similar to visualize.c

  • Bug fixes: Resolved demo-related issues, init_only_controlled mode bug fix

Documentation updates:

  • Fixed table formatting issues in docs
  • Updated descriptions for control modes and init modes in the visualizer
  • Corrected naming inconsistencies in control mode documentation

Copilot AI review requested due to automatic review settings January 31, 2026 18:24
@greptile-apps
Copy link

greptile-apps bot commented Jan 31, 2026

Greptile Overview

Greptile Summary

This PR improves documentation for the PufferDrive simulator and visualizer with important clarifications and additions:

Key Changes:

  • Updated control mode naming from control_tracks_to_predict to control_wosac for WOSAC evaluation
  • Added detailed "Agent Dynamics" section explaining policy-controlled, expert, and static agents
  • Added "Init modes" section documenting create_all_valid vs create_only_controlled initialization
  • Expanded visualizer documentation with comprehensive CLI arguments table, visualization flags, and drive.ini settings reference

Issues Found:

  • One inconsistency in simulator.md line 29 still references old control_tracks_to_predict name
  • Minor spacing issue in visualizer.md line 39

Confidence Score: 4/5

  • This PR is safe to merge after fixing the control mode naming inconsistency
  • Score reflects high-quality documentation improvements with only one syntax error that needs correction. The changes are well-structured and add valuable context, but the inconsistent control mode name should be fixed before merging to avoid confusion.
  • docs/src/simulator.md requires attention to fix the control mode naming inconsistency on line 29

Important Files Changed

Filename Overview
docs/src/simulator.md Updated control mode from control_tracks_to_predict to control_wosac, added detailed agent dynamics documentation and init modes section. One inconsistency found in line 29.
docs/src/visualizer.md Added comprehensive documentation for CLI arguments, visualization flags, and drive.ini settings. Minor spacing issue in line 39.

Sequence Diagram

sequenceDiagram
    participant User
    participant Docs as Documentation
    participant Visualizer as visualize binary
    participant Config as drive.ini
    participant Simulator as PufferDrive Simulator
    
    User->>Docs: Read simulator.md & visualizer.md
    Note over Docs: Updated control mode naming<br/>control_wosac (not control_tracks_to_predict)
    Note over Docs: Added agent dynamics section<br/>& init modes documentation
    User->>Visualizer: Run ./visualize with CLI args
    Visualizer->>Config: Read pufferlib/config/ocean/drive.ini
    Config-->>Visualizer: Return control_mode, episode_length, etc.
    Visualizer->>Simulator: Initialize environment with config
    Note over Simulator: Apply control_mode:<br/>control_wosac for WOSAC eval<br/>control_vehicles for vehicles only
    Simulator-->>Visualizer: Environment ready
    Visualizer->>User: Render video output (agent/topdown views)
Loading

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.

2 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

- `control_sdc_only`: Self-driving car only

> [!NOTE]
> `control_vehicles` filters out agents marked as "expert" and those too close to their goal (<2m). For full WOMD evaluation, use `control_tracks_to_predict`.
Copy link

Choose a reason for hiding this comment

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

Inconsistent control mode name - control_tracks_to_predict should be control_wosac

Suggested change
> `control_vehicles` filters out agents marked as "expert" and those too close to their goal (<2m). For full WOMD evaluation, use `control_tracks_to_predict`.
> `control_vehicles` filters out agents marked as "expert" and those too close to their goal (<2m). For full WOMD evaluation, use `control_wosac`.
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/src/simulator.md
Line: 29:29

Comment:
Inconsistent control mode name - `control_tracks_to_predict` should be `control_wosac`

```suggestion
> `control_vehicles` filters out agents marked as "expert" and those too close to their goal (<2m). For full WOMD evaluation, use `control_wosac`.
```

How can I resolve this? If you propose a fix, please make it concise.


## Arguments & Configuration

The `visualize` tool supports several CLI arguments to control the rendering output. It also reads the `pufferlib/config/ocean/drive.ini` file for default environment settings(For more details on these settings, refer to [Configuration](simulator.md#configuration)).
Copy link

Choose a reason for hiding this comment

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

Missing space after "default" - add spacing for readability

Suggested change
The `visualize` tool supports several CLI arguments to control the rendering output. It also reads the `pufferlib/config/ocean/drive.ini` file for default environment settings(For more details on these settings, refer to [Configuration](simulator.md#configuration)).
The `visualize` tool supports several CLI arguments to control the rendering output. It also reads the `pufferlib/config/ocean/drive.ini` file for default environment settings (For more details on these settings, refer to [Configuration](simulator.md#configuration)).

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/src/visualizer.md
Line: 39:39

Comment:
Missing space after "default" - add spacing for readability

```suggestion
The `visualize` tool supports several CLI arguments to control the rendering output. It also reads the `pufferlib/config/ocean/drive.ini` file for default environment settings (For more details on these settings, refer to [Configuration](simulator.md#configuration)).
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates documentation for the PufferDrive visualizer and simulator, focusing on clarifying configuration options and control modes. The changes add comprehensive documentation for the visualizer's CLI arguments and update control mode terminology.

Changes:

  • Added detailed documentation for visualizer CLI arguments, visualization flags, and configuration settings
  • Updated control mode from control_tracks_to_predict to control_wosac for WOSAC evaluation
  • Added new documentation sections explaining init modes and agent dynamics in the simulator

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.

File Description
docs/src/visualizer.md Expanded documentation with comprehensive CLI argument tables, visualization flags, and drive.ini settings reference
docs/src/simulator.md Updated control mode terminology to control_wosac and added new sections on init modes and agent dynamics

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

> 2. **Experts:** Stepped using ground-truth log trajectories.
> 3. **Static:** Remain frozen in place.
>
> In the simulator, agents not selected for policy control will be treated as **Static** by default. To make them follow their **Expert trajectories**, you must set `mark_as_expert=true` for those agents in the jsons. This is critical for `control_sdc_only` to ensure the environment behaves realistically around the policy-controlled agents.
Copy link

Copilot AI Jan 31, 2026

Choose a reason for hiding this comment

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

The term "jsons" should be "JSON files" to maintain consistency with the rest of the documentation. Throughout the codebase, "JSON" is consistently capitalized (e.g., data.md:3, data.md:10, scene-editor.md:13). The phrase "in the jsons" should be replaced with "in the JSON files" for consistency.

Copilot uses AI. Check for mistakes.
@mpragnay mpragnay force-pushed the Demo/Visualize-fixes-and-docs-changes branch from 238ddf3 to 26795f5 Compare January 31, 2026 18:29
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.

2 participants