-
Notifications
You must be signed in to change notification settings - Fork 12
Add remove and stop collision behaviors #78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Thanks @Aditya-Gupta26. On a first look it looks good to me. Can you share with me wandb links to:
You can use 10 maps, (num_maps=10), probably easiest to use the WOMD dataset for this Will do an in-depth review tomorrow! |
pufferlib/ocean/drive/drive.h
Outdated
| agent->stopped = 1; | ||
| agent->vx=agent->vy = 0.0f; | ||
| } | ||
| else if(env->post_collision_action==REMOVE_AGENT && !agent->removed){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens to agents that are still alive that collide with agents where stopped=1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In the rendered GIFs, I could see that if a moving agent collides with an already collided agent, both of the agents stop, and freeze
|
How do you configure the collision_behavior in the environment on the Python side? This is missing. Can you add an argument inside |
| #define STOP_AGENT 1 | ||
| #define REMOVE_AGENT 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about the mode that is currently active where collisions are ignored and agents can go through each other?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
During the execution, the use of STOP_AGENT and REMOVE_AGENT are in an if-else if-else block.
If neither of the two conditions (STOP,REMOVE) are satisfied, the code continues in the "else" block which is currently the same logic as before (default case - ignore and pass through)
| int stopped; // 0/1 -> freeze if set | ||
| int removed; //0/1 -> remove from sim if set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like removed does not need to be an additional variable (though maybe it does), if a vehicle is removed it will only have this status for a single step
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
|
||
| if(collided == VEHICLE_COLLISION && is_active_agent == 1 && respawned){ | ||
| agent->collision_state = 0; | ||
| if(collided == VEHICLE_COLLISION){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we want this for non-vehicle collisions too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By non-vehicle collision, do you mean offroad, or is there any other scenario as well ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think @eugenevinitsky is referring to collisions with other road users, such as cyclists and pedestrians. Since these are currently not created, it's probably easiest to take care of this in a separate PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, yeah makes sense !
| e->stopped = 0; | ||
| e->removed = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
think we need to send a done signal when either of these events happen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will update this !
pufferlib/ocean/drive/drive.h
Outdated
| env->post_collision_action = 1; | ||
| env->post_offroad_action = 1; | ||
| env->dynamics_model = CLASSIC; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's this for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added two parameters to allow for independent behavior for collision and offroad. Ex - We could also simulate the behavior when a collision "removes" agent from the scene and "freezes" if it goes offroad.
Setting both to 1 (as above) will set it to "remove" for both collision and offroad
|
On the remove behavior, don't we want on a collision to "respawn" the agent like if it have reach the goal (but with the reward of collision) ? |
|
In general I don't like the respawn behavior at all, it's challenging to re-insert an agent into a dynamic scene. I'd prefer to remove the respawn behavior entirely |
We decided to keep the respawning for now and first merge #76. Respawning is a good fallback option. Making a note here to bring this up again during our next sync meeting @vcharraut @eugenevinitsky @Victorbares |
| goal_radius = 2.0 # Meters around goal to be considered "reached" | ||
| resample_frequency = 910 | ||
| num_maps = 1 | ||
| collision_behaviour = 1 # 0 - Ignore, 1 - Stop, 2 - Remove |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the current default is ignore? Ensure the default option is what is currently in main. Running this with the default params should give exactly the same results as what we have in main
pufferlib/config/ocean/drive.ini
Outdated
| scale = auto | ||
|
|
||
|
|
||
| [sweep.env.offroad_behaviour] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be removed
pufferlib/config/ocean/drive.ini
Outdated
| resample_frequency = 910 | ||
| num_maps = 1 | ||
| collision_behaviour = 1 # 0 - Ignore, 1 - Stop, 2 - Remove | ||
| offroad_behaviour = 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing comment
| if (respawned_collided_with_car) { | ||
| agent->collision_state = 0; | ||
| agent->metrics_array[COLLISION_IDX] = 0.0f; | ||
| agent->collision_state = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apply linting

This branch modifies post collision/offroad behaviors of agents where they now have the option of stoping/disappearing from the scene depending on the parameter set.
All changes are in "drive.h" file.

