Implement IDM to allow nuplan reactive eval#441
Open
WaelDLZ wants to merge 3 commits into
Open
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces per-agent controller selection in the Ocean Drive environment to support mixed-control evaluation (policy-controlled, log-replay, static, and a new IDM-based controller), with Python/C plumbing and configuration updates to expose and configure these modes.
Changes:
- Adds controller mode constants and wires new controller config fields from Python (
drive.py) into the C simulator (Drivestruct + bindings). - Implements an IDM controller (
pufferlib/ocean/drive/idm.h) and routes agent motion through a unifiedmove_agent_with_controllerdispatcher. - Extends agent introspection/export to Python by adding a per-agent
controllerfield, and updatesdrive.iniwith new controller options.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| pufferlib/ocean/env_binding.h | Exposes controller-mode constants to Python bindings. |
| pufferlib/ocean/drive/idm.h | Adds IDM controller implementation (route-following + leader/traffic-light handling). |
| pufferlib/ocean/drive/drive.py | Adds new controller kwargs, validation, and passes controller settings into the native env. |
| pufferlib/ocean/drive/drive.h | Adds controller fields to Drive, controller resolution, and dispatches movement by controller. |
| pufferlib/ocean/drive/datatypes.h | Adds controller field to Agent. |
| pufferlib/ocean/drive/binding.c | Plumbs controller kwargs into env init and exports per-agent controller to Python. |
| pufferlib/config/ocean/drive.ini | Adds controller configuration keys and documentation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+71
to
+77
| sdc_controller = "policy" | ||
| ; Controller used by non-SDC vehicles. | ||
| ; options: "static", "policy", "replay", "idm" | ||
| non_sdc_controller = "policy" | ||
| ; Controller used by non-vehicle agents. "auto" follows non_sdc_controller unless it is "idm", then it uses "replay". | ||
| ; options: "auto", "static", "policy", "replay", "idm" | ||
| non_vehicle_controller = "auto" |
Comment on lines
+268
to
+293
| controller_values = { | ||
| "static": binding.CONTROLLER_STATIC, | ||
| "policy": binding.CONTROLLER_POLICY, | ||
| "replay": binding.CONTROLLER_REPLAY, | ||
| "idm": binding.CONTROLLER_IDM, | ||
| } | ||
| controller_options = "'static', 'policy', 'replay', or 'idm'" | ||
| if self.sdc_controller_str not in controller_values: | ||
| raise ValueError(f"sdc_controller must be one of {controller_options}. Got: {self.sdc_controller_str}") | ||
| if self.non_sdc_controller_str not in controller_values: | ||
| raise ValueError( | ||
| f"non_sdc_controller must be one of {controller_options}. Got: {self.non_sdc_controller_str}" | ||
| ) | ||
| if self.non_vehicle_controller_str == "auto": | ||
| if self.non_sdc_controller_str == "idm": | ||
| self.non_vehicle_controller_str = "replay" | ||
| else: | ||
| self.non_vehicle_controller_str = self.non_sdc_controller_str | ||
| elif self.non_vehicle_controller_str not in controller_values: | ||
| raise ValueError( | ||
| f"non_vehicle_controller must be 'auto' or one of {controller_options}. " | ||
| f"Got: {self.non_vehicle_controller_str}" | ||
| ) | ||
| self.sdc_controller = controller_values[self.sdc_controller_str] | ||
| self.non_sdc_controller = controller_values[self.non_sdc_controller_str] | ||
| self.non_vehicle_controller = controller_values[self.non_vehicle_controller_str] |
Comment on lines
+4949
to
+4951
| if (env->simulation_mode == SIMULATION_REPLAY) { | ||
| move_expert(env, env->actions, agent_idx); | ||
| } |
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
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.
This PR introduces one major change: "Controllers"
Idea: now some policy can be controlled by IDM, some other by policy, some other by log-replay.
It is now split in sdc_controller, non_sdc_controller because it's the most useful split for evals, but we can do other things later if we want.
For WOMD, since IDM shouldn't be used to move pedestrians or cyclists, I added also a non_vehicles_controllers.
IDM uses the route info to know which lane to move on, then detects a leader on that lane (first object it intersects on that lane), and uses the IDM formula to update it's speed. I made it unaware of dynamics and simply teleporting on the centerlanes, which I think is good for the "evals" use case. By the way on nuPlan maps it will sometimes offroad it the road is too narrow for a long vehicle to take a turn.
I've made it use all the same values as the nuPlan official implementation, and also added the "extended footprint mechanism" that makes it better at intersections.