Skip to content
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

Clean up object types before one-hot encoding #205

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

daphne-cornelisse
Copy link
Contributor

Description

This PR improves the preprocessing of road and object-type features before one-hot encoding. It fixes a bug that was occurring when running the environment on the CPU (device="cpu"), where large garbage values such as -30,000 caused issues during the one-hot encoding step.

To resolve this and improve readability, I added the entity type enums to the base environment and removed all garbage values from the tensors before the one-hot encoding step.

@daphne-cornelisse daphne-cornelisse added the bug Something isn't working label Jul 28, 2024
Comment on lines +115 to +131
self.ENTITY_TYPE_TO_INT = {
gpudrive.EntityType._None: 0,
gpudrive.EntityType.RoadEdge: 1,
gpudrive.EntityType.RoadLine: 2,
gpudrive.EntityType.RoadLane: 3,
gpudrive.EntityType.CrossWalk: 4,
gpudrive.EntityType.SpeedBump: 5,
gpudrive.EntityType.StopSign: 6,
gpudrive.EntityType.Vehicle: 7,
gpudrive.EntityType.Pedestrian: 8,
gpudrive.EntityType.Cyclist: 9,
gpudrive.EntityType.Padding: 10,
}
self.MIN_OBJ_ENTITY_ENUM = min(list(self.ENTITY_TYPE_TO_INT.values()))
self.MAX_OBJ_ENTITY_ENUM = max(list(self.ENTITY_TYPE_TO_INT.values()))
self.ROAD_MAP_OBJECT_TYPES = 7 # (enums 0-6)
self.ROAD_OBJECT_TYPES = 4 # (enums 7-10)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think there is some redundancy here:

  • An enum is an alias for an integer, so there isn't a need to map the names to their underlying representation.
  • If you split ENTITY_TYPE_TO_INT into two lists- one for agent entities and one for road map entities- you can eliminate ROAD_MAP_OBJECT_TYPES and ROAD_OBJECT_TYPES because they would equal the length of the corresponding lists.
Suggested change
self.ENTITY_TYPE_TO_INT = {
gpudrive.EntityType._None: 0,
gpudrive.EntityType.RoadEdge: 1,
gpudrive.EntityType.RoadLine: 2,
gpudrive.EntityType.RoadLane: 3,
gpudrive.EntityType.CrossWalk: 4,
gpudrive.EntityType.SpeedBump: 5,
gpudrive.EntityType.StopSign: 6,
gpudrive.EntityType.Vehicle: 7,
gpudrive.EntityType.Pedestrian: 8,
gpudrive.EntityType.Cyclist: 9,
gpudrive.EntityType.Padding: 10,
}
self.MIN_OBJ_ENTITY_ENUM = min(list(self.ENTITY_TYPE_TO_INT.values()))
self.MAX_OBJ_ENTITY_ENUM = max(list(self.ENTITY_TYPE_TO_INT.values()))
self.ROAD_MAP_OBJECT_TYPES = 7 # (enums 0-6)
self.ROAD_OBJECT_TYPES = 4 # (enums 7-10)
self.ROAD_MAP_ENTITY_TYPES = [
gpudrive.EntityType._None,
gpudrive.EntityType.RoadEdge,
gpudrive.EntityType.RoadLine,
gpudrive.EntityType.RoadLane,
gpudrive.EntityType.CrossWalk,
gpudrive.EntityType.SpeedBump,
gpudrive.EntityType.StopSign,
self.AGENT_ENTITY_TYPES = [
gpudrive.EntityType.Vehicle,
gpudrive.EntityType.Pedestrian,
gpudrive.EntityType.Cyclist,
gpudrive.EntityType.Padding,
]
self.MIN_OBJ_ENTITY_ENUM = min(self.ROAD_MAP_ENTITY_TYPES + self.AGENT_ENTITY_TYPES)
self.MAX_OBJ_ENTITY_ENUM = max(self.ROAD_MAP_ENTITY_TYPES + self.AGENT_ENTITY_TYPES)

Moreover, I'm not sure these should be instance variables.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for pointing that out, I made the changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Question: I still need to map the enums to integers. How would you suggest to do this?

Instead of the dictionary above, I could do something like:

{
     gpudrive.EntityType._None: int( gpudrive.EntityType._None),
...
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can use a comprehension instead (ie. {(k, int(k)) for k in self.ENTITY_TYPES}) but looking through the functions one_hot_encode_*, I don't immediately see where this mapping is necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, and it is used in the torch environment, e.g. see line 269 in env_torch.py in this PR

pygpudrive/env/env_torch.py Show resolved Hide resolved
).int()

return torch.nn.functional.one_hot(
road_types.long(), num_classes=self.ROAD_MAP_OBJECT_TYPES,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
road_types.long(), num_classes=self.ROAD_MAP_OBJECT_TYPES,
road_types.long(), len(self.ROAD_MAP_ENTITY_TYPES),

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants