Conversation
Greptile OverviewGreptile SummaryThis PR adds comprehensive z-coordinate awareness to the autonomous driving simulation, enabling agents to understand elevation and implement 2.5D collision detection. The implementation spans from data loading through neural network observations to collision checking. Key Changes:
Critical Issues Found:
Confidence Score: 2/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant Env as Drive Environment
participant Agent as Agent Entity
participant Road as Road Segments
participant Collision as Collision System
participant NN as Neural Network
Note over Env: Initialize with z-coordinates
Env->>Agent: Load trajectory data (x, y, z)
Env->>Road: Load road geometry (x, y, z)
Env->>Env: Compute world_mean_z
Env->>Agent: Normalize z -= world_mean_z
Env->>Road: Normalize z -= world_mean_z
Note over Env: Step Loop
Agent->>NN: Compute observations (8 features: goal_x, goal_y, goal_z, ...)
NN->>Agent: Return actions
Agent->>Agent: Update position (x, y)
Note over Agent: Z-Coordinate Update
Agent->>Road: Find nearby road segments (checkNeighbors)
Road->>Agent: Return list of road entities
Agent->>Agent: Compute z-distance to each road (compute_z_distance_to_road_segment)
Agent->>Agent: Sort by distance (qsort)
Agent->>Agent: Average z from nearest segments
Agent->>Agent: Update agent->z
Note over Collision: 2.5D Collision Check
Agent->>Collision: Check collision with other agents
Collision->>Collision: check_aabb_collision (x, y planes)
alt AABB collision detected
Collision->>Agent: Set aabb_collision_state = 1
Collision->>Collision: check_z_collision (z axis overlap)
alt Z overlap detected
Collision->>Agent: Set collision_state = 1
else No Z overlap
Collision->>Agent: No collision (agents on different elevations)
end
end
Agent->>Env: Return observations, rewards, done
|
pufferlib/ocean/drive/drive.h
Outdated
| a_long_new = clip(a_long_new, -5.0f, 2.5f); | ||
| } | ||
|
|
||
| typedef struct { | ||
| float dis; |
There was a problem hiding this comment.
syntax: Duplicate DepthPoint typedef - already defined at line ~424. This will cause compilation errors.
| a_long_new = clip(a_long_new, -5.0f, 2.5f); | |
| } | |
| typedef struct { | |
| float dis; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: pufferlib/ocean/drive/drive.h
Line: 1828:1832
Comment:
**syntax:** Duplicate `DepthPoint` typedef - already defined at line ~424. This will cause compilation errors.
```suggestion
```
How can I resolve this? If you propose a fix, please make it concise.
pufferlib/ocean/torch.py
Outdated
| road_onehot = F.one_hot(road_categorical.long(), num_classes=8) # Shape: [batch, 200, 8] | ||
| print(road_onehot.shape) | ||
| print(road_continuous.shape) |
There was a problem hiding this comment.
style: Debug print statements left in production code
| road_onehot = F.one_hot(road_categorical.long(), num_classes=8) # Shape: [batch, 200, 8] | |
| print(road_onehot.shape) | |
| print(road_continuous.shape) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: pufferlib/ocean/torch.py
Line: 78:80
Comment:
**style:** Debug print statements left in production code
```suggestion
```
How can I resolve this? If you propose a fix, please make it concise.
pufferlib/ocean/drive/drive.py
Outdated
| print(len(map_data.get("objects", []))) | ||
| print(len(map_data.get("roads", []))) |
There was a problem hiding this comment.
style: Debug print statements left in production code
| print(len(map_data.get("objects", []))) | |
| print(len(map_data.get("roads", []))) |
Prompt To Fix With AI
This is a comment left during a code review.
Path: pufferlib/ocean/drive/drive.py
Line: 425:426
Comment:
**style:** Debug print statements left in production code
```suggestion
```
How can I resolve this? If you propose a fix, please make it concise.
pufferlib/ocean/drive/visualize.c
Outdated
|
|
||
| if (render_topdown) { | ||
| printf("Recording topdown view...\n"); | ||
| fflush(stdout); |
There was a problem hiding this comment.
style: Debug printf left in code - env.observations is a pointer, this will print the address not the size
Prompt To Fix With AI
This is a comment left during a code review.
Path: pufferlib/ocean/drive/visualize.c
Line: 361:361
Comment:
**style:** Debug printf left in code - `env.observations` is a pointer, this will print the address not the size
How can I resolve this? If you propose a fix, please make it concise.
pufferlib/ocean/drive/drive.h
Outdated
| //To update agent's z-coordinate based on road elevation of 5 nearest elements | ||
| GridMapEntity entity_list[MAX_ENTITIES_PER_CELL*100]; // Array big enough for all neighboring cells | ||
| int list_size = checkNeighbors(env, agent->x, agent->y, entity_list, MAX_ENTITIES_PER_CELL*100, collision_offsets, 100); | ||
| DepthPoint road_neighbours[list_size]; | ||
| int max_check = (list_size < 10) ? list_size : 10; | ||
| int diffarray[max_check-1]; | ||
| if(list_size>0){ | ||
| //store an array masuring the distance of the agent with each road segment nearby | ||
| for(int i = 0;i<list_size;i++){ | ||
| if(entity_list[i].entity_idx == -1 ) continue; | ||
| Entity* entity = &env->entities[entity_list[i].entity_idx]; | ||
| if(entity->type == ROAD_EDGE || entity->type == ROAD_LANE || entity->type == ROAD_LINE){ |
There was a problem hiding this comment.
logic: road_neighbours array not fully initialized. Loop only sets elements where entity->type is ROAD_EDGE/ROAD_LANE/ROAD_LINE, leaving other indices uninitialized. When qsort reads these, it accesses undefined values.
// Initialize array first
DepthPoint road_neighbours[list_size];
memset(road_neighbours, 0, list_size * sizeof(DepthPoint));
int valid_count = 0;
for(int i = 0; i < list_size; i++){
if(entity_list[i].entity_idx == -1) continue;
Entity* entity = &env->entities[entity_list[i].entity_idx];
if(entity->type == ROAD_EDGE || entity->type == ROAD_LANE || entity->type == ROAD_LINE){
road_neighbours[valid_count++] = compute_z_distance_to_road_segment(agent, entity);
}
}
qsort(road_neighbours, valid_count, sizeof(DepthPoint), compare_depthpoint);
Prompt To Fix With AI
This is a comment left during a code review.
Path: pufferlib/ocean/drive/drive.h
Line: 1685:1696
Comment:
**logic:** `road_neighbours` array not fully initialized. Loop only sets elements where `entity->type` is ROAD_EDGE/ROAD_LANE/ROAD_LINE, leaving other indices uninitialized. When `qsort` reads these, it accesses undefined values.
```
// Initialize array first
DepthPoint road_neighbours[list_size];
memset(road_neighbours, 0, list_size * sizeof(DepthPoint));
int valid_count = 0;
for(int i = 0; i < list_size; i++){
if(entity_list[i].entity_idx == -1) continue;
Entity* entity = &env->entities[entity_list[i].entity_idx];
if(entity->type == ROAD_EDGE || entity->type == ROAD_LANE || entity->type == ROAD_LINE){
road_neighbours[valid_count++] = compute_z_distance_to_road_segment(agent, entity);
}
}
qsort(road_neighbours, valid_count, sizeof(DepthPoint), compare_depthpoint);
```
How can I resolve this? If you propose a fix, please make it concise.| float* observations = calloc(num_agents*num_obs, sizeof(float)); | ||
| for (int i = 0; i < num_obs*num_agents; i++) { | ||
| observations[i] = i % 7; | ||
| observations[i] = i % 8; |
There was a problem hiding this comment.
We discussed this on Slack, but after rebasing, you can simply increment these
// Ego features depend on dynamics model
#define EGO_FEATURES_CLASSIC 7
#define EGO_FEATURES_JERK 10
pufferlib/ocean/drive/drive.h
Outdated
| {-2, 0}, {-1, 0}, {0, 0}, {1, 0}, {2, 0}, // Middle row (including center) | ||
| {-2, 1}, {-1, 1}, {0, 1}, {1, 1}, {2, 1}, // Fourth row | ||
| {-2, 2}, {-1, 2}, {0, 2}, {1, 2}, {2, 2} // Bottom row | ||
| static const int collision_offsets[225][2] = { |
There was a problem hiding this comment.
Wow, why is this necessary? Can this be done more simply?
There was a problem hiding this comment.
I wanted to increase the size of the grid so at all points, there are always enough road lanes/lines/edges which the agent can find. With the older (smaller) grid, I noticed that it was unable to find enough agents, and would update the z value based on 1-2 surrounding points, which might be inaccurate.
| float init_goal_z; | ||
| int mark_as_expert; | ||
| int collision_state; | ||
| int aabb_collision_state; |
There was a problem hiding this comment.
Why does aabb_collision_state need to be added to the timespec struct?
There was a problem hiding this comment.
aabb_collision_state is meant to be set only when the x and y coordinates of agents overlap (2D collision). I added this specifically so that the agent knows when to turn green.
pufferlib/ocean/drive/drive.h
Outdated
| fread(&entities[i].length, sizeof(float), 1, file); | ||
| fread(&entities[i].height, sizeof(float), 1, file); | ||
| fread(&entities[i].goal_position_x, sizeof(float), 1, file); | ||
| //fread(&entities[i].goal_position_z, sizeof(float), 1, file); |
There was a problem hiding this comment.
Yes, support for goal_position_z exists, I also added support to update it in case of reset, or while computing new goals
pufferlib/ocean/drive/drive.h
Outdated
| car_collided_with_index = index; | ||
| break; | ||
| agent->aabb_collision_state = 1; | ||
| if(check_z_collision(agent,entity)){ |
There was a problem hiding this comment.
Why not include the z-axis into the check_aabb_collision? What is the rationale behind creating a separate function for the z-axis?
There was a problem hiding this comment.
So the way collision check works now is that we first check for a 2D collision (x and y values overlap). Only if there is a 2D collision, we go ahead and check if the z-values of the agents are close enough for a collision (the check_z_collision function).
We can include the z-axis check inside check_aabb_collision, I just thought separating this would be cleaner.
98bf017 to
262bfa7
Compare
This PR is aimed at adding z-coordinate support for agents from training to inference.
The agents now are additionally aware of the z-coordinate positions of their surroundings and themselves, which should modify the training behavior. Also, a 2.5D z-collision check support has been added which also looks at the z-coordinate of the agents before marking a collision.
The agents also briefly turn green in situations where they overlap each other (x and y coordinate match).