Conversation
…nd removed everything else to prevent redundencies
…sampling logic for initial goals, configurable params for spawn settings
Greptile OverviewGreptile SummaryThis PR implements random agent spawning with configurable dimensions and collision-free placement. Agents are spawned with randomized vehicle dimensions (width, length, height) on drivable lanes, with rejection sampling to prevent collisions and offroad placements. Initial goals are sampled using the 2.0 goal sampling logic. Key Changes:
Critical Issues Found:
Confidence Score: 1/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Python as Drive.py
participant Binding as binding.c
participant Core as drive.h
Python->>Binding: shared() with random_agents mode
Binding->>Binding: Distribute agents across envs
Binding-->>Python: agent_offsets, map_ids, env_count
Python->>Binding: env_init() for each env
Binding->>Core: Initialize Drive struct
Core->>Core: load_map_binary()
Core->>Core: set_active_agents()
Core->>Core: spawn_agents_with_counts()
loop For each agent
Core->>Core: spawn_agent()
Core->>Core: Random vehicle dimensions
loop Rejection sampling (MAX_SPAWN_ATTEMPTS)
Core->>Core: get_random_point_on_lane()
Core->>Core: check_spawn_collision()
Core->>Core: check_spawn_offroad()
alt Valid spawn
Core->>Core: break
else Invalid
Core->>Core: continue
end
end
Core->>Core: Update agent position & state
end
Core->>Core: init_goal_positions()
Core->>Core: sample_new_goal() for each agent
Core-->>Python: Environment ready
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
pufferlib/ocean/drive/drive.py
Outdated
| min_spawn_agents=self.min_spawn_agents, | ||
| max_spawn_agents=self.max_spawn_agents, |
There was a problem hiding this comment.
undefined attribute references - self.min_spawn_agents and self.max_spawn_agents don't exist
| min_spawn_agents=self.min_spawn_agents, | |
| max_spawn_agents=self.max_spawn_agents, | |
| min_agents_per_env=self.min_agents_per_env, | |
| max_agents_per_env=self.max_agents_per_env, |
pufferlib/ocean/drive/drive.h
Outdated
| float mod_heading = atan2f(agent->heading_y, agent->heading_x); | ||
| float cos_theta = dot/(mod_to_pt * mod_heading); |
There was a problem hiding this comment.
incorrect cosine calculation - mod_heading uses atan2f which returns an angle, not a magnitude
the cosine is already computed via the dot product formula: cos(theta) = dot / (|v1| * |v2|), where |heading| should be the magnitude (which is 1.0 for normalized heading vectors), not atan2f
| float mod_heading = atan2f(agent->heading_y, agent->heading_x); | |
| float cos_theta = dot/(mod_to_pt * mod_heading); | |
| float mod_heading = sqrtf(agent->heading_x * agent->heading_x + agent->heading_y * agent->heading_y); | |
| float cos_theta = dot/(mod_to_pt * mod_heading); |
There was a problem hiding this comment.
Pull request overview
This PR adds a new random_agents initialization mode to the Ocean Drive environment, enabling randomized per-environment agent counts and randomized vehicle dimensions/poses during spawning, with additional spawn-time collision/off-road rejection and integration with goal sampling.
Changes:
- Extend INI/env config and Python/C bindings to support
random_agentsmode and spawn dimension parameters. - Implement random agent spawning logic in the C core, including collision/off-road checks and goal initialization behavior.
- Update demo/visualization entrypoints and default
drive.inisettings to exercise the new mode.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
pufferlib/ocean/env_config.h |
Adds new spawn/agent-count config fields and parses init_mode strings. |
pufferlib/ocean/drive/visualize.c |
Wires spawn settings into visualization env init; randomizes agent counts in random mode. |
pufferlib/ocean/drive/error.h |
Extends error types and adjusts error string formatting. |
pufferlib/ocean/drive/drive.py |
Adds Python-side config for random agent counts/dimensions and passes through to bindings. |
pufferlib/ocean/drive/drive.h |
Implements spawn rejection sampling, off-road checks, random init mode wiring, and goal sampling adjustments. |
pufferlib/ocean/drive/drive.c |
Updates demo to use spawn settings and random agent counts; changes default weights/map. |
pufferlib/ocean/drive/datatypes.h |
Adds free_agents helper to free an array of agents safely. |
pufferlib/ocean/drive/binding.c |
Adds random agent-count partitioning in shared() and passes spawn settings into env init. |
pufferlib/config/ocean/drive.ini |
Updates defaults to enable random_agents and sets spawn/agent-count parameters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| env_config->init_mode = 0; // Default to create_all_valid | ||
| } | ||
| } else if (MATCH("env", "control_mode")) { | ||
| env_config->control_mode = atoi(value); |
There was a problem hiding this comment.
control_mode is parsed with atoi(value), but the INI uses string values like "control_vehicles"/"control_wosac". With atoi, any non-numeric value becomes 0, silently selecting CONTROL_VEHICLES even when another mode is configured. Parse control_mode similarly to action_type/init_mode (accept quoted/unquoted strings) or require numeric values consistently in the INI.
| env_config->control_mode = atoi(value); | |
| /* Support both string and numeric values for control_mode. | |
| * Known string values (quoted or unquoted): | |
| * - control_vehicles -> 0 | |
| * - control_wosac -> 1 | |
| */ | |
| if (strcmp(value, "\"control_vehicles\"") == 0 || strcmp(value, "control_vehicles") == 0) { | |
| env_config->control_mode = 0; | |
| } else if (strcmp(value, "\"control_wosac\"") == 0 || strcmp(value, "control_wosac") == 0) { | |
| env_config->control_mode = 1; | |
| } else { | |
| /* Fallback: if the value looks numeric, parse it, otherwise warn and default. */ | |
| const char *p = value; | |
| while (*p == ' ' || *p == '\t') { | |
| ++p; | |
| } | |
| if ((*p >= '0' && *p <= '9') || *p == '-' || *p == '+') { | |
| env_config->control_mode = atoi(p); | |
| } else { | |
| printf("Warning: Unknown control_mode value '%s', defaulting to control_vehicles\n", value); | |
| env_config->control_mode = 0; // Default to control_vehicles | |
| } | |
| } |
pufferlib/ocean/drive/binding.c
Outdated
| @@ -98,6 +100,44 @@ static PyObject *my_shared(PyObject *self, PyObject *args, PyObject *kwargs) { | |||
| int maps_checked = 0; | |||
| PyObject *agent_offsets = PyList_New(max_envs + 1); | |||
| PyObject *map_ids = PyList_New(max_envs); | |||
|
|
|||
| if(init_mode == RANDOM_AGENTS) { | |||
| // Training mode: random agent counts per env | |||
There was a problem hiding this comment.
In the RANDOM_AGENTS branch, agent_offsets and map_ids are allocated before the if (lines 101-102) but the function returns early without Py_DECREF-ing them. This leaks Python references each time binding.shared() is called in random-agents mode; either delay these allocations until after the RANDOM_AGENTS early return or Py_DECREF them before returning.
There was a problem hiding this comment.
The function returns the tuple with the items, so their GC will be taken care of by the python part, it needs to be gced only if there is an error and they are not returned, i.e. persisted in with the C API call
pufferlib/ocean/drive/drive.h
Outdated
| float mod_heading = atan2f(agent->heading_y, agent->heading_x); | ||
| float cos_theta = dot/(mod_to_pt * mod_heading); |
There was a problem hiding this comment.
sample_new_goal’s new “ahead of agent” check computes mod_heading = atan2f(agent->heading_y, agent->heading_x) (an angle in radians), then uses it as a magnitude in cos_theta = dot/(mod_to_pt * mod_heading). This is mathematically incorrect and can divide by 0/near-0; it will misclassify points and destabilize goal sampling. Use the heading vector norm (typically 1.0 if normalized) or compute cos_theta = dot / (|to_point| * |heading|) with proper guards.
| float mod_heading = atan2f(agent->heading_y, agent->heading_x); | |
| float cos_theta = dot/(mod_to_pt * mod_heading); | |
| float heading_norm = sqrtf(agent->heading_x * agent->heading_x + agent->heading_y * agent->heading_y); | |
| if (mod_to_pt <= 1e-6f || heading_norm <= 1e-6f) | |
| continue; | |
| float cos_theta = dot / (mod_to_pt * heading_norm); |
pufferlib/ocean/drive/drive.h
Outdated
| // Design Choice(we don't have wide vehicles on roads) | ||
| if (spawn_width > spawn_length) | ||
| spawn_width = spawn_length; | ||
| float spawn_height = 1.5f; // Design Choice: Doesn't matter as we don't have flying cars in 2026 |
There was a problem hiding this comment.
spawn_agent hardcodes spawn_height = 1.5f, ignoring the configured spawn_settings.h value that gets parsed from INI/Python. This makes the new spawn_height config ineffective; use spawn_settings.h (or rename to avoid confusion) so the config actually controls spawn height.
| float spawn_height = 1.5f; // Design Choice: Doesn't matter as we don't have flying cars in 2026 | |
| float spawn_height = spawn_settings.h; // Use configured spawn height |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* Changed counts to max supported by sim, max_controlled, num_created and removed everything else to prevent redundencies * Fixed memory leaks
…ed defaults to align with requirements
riccardosavorgnan
force-pushed
the
3.0_beta
branch
from
e646569 to
09e725d
Compare
1 participant
Spawning logic added with a random number of agents and randomized dimensions
Integrated with the 2.0 goal sampling for initial goals
Collision and off-road checks during spawns