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.

Sign up for GitHub

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

Jump to bottom

Remove vehicle type from agent interface #1294

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Remove vehicle type from agent interface #1294

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c4922b0
remove vehicle type from agent interface
liu-allan Feb 23, 2022
c7b66b6
udpate changelog
liu-allan Feb 23, 2022
40b185d
udpate changelog
liu-allan Feb 23, 2022
604e159
allow custom vehicle specification from scenario
liu-allan Feb 28, 2022
f36e85b
cleanup
liu-allan Feb 28, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ Copy and pasting the git commit messages is __NOT__ enough.
### Fixed
- Fixed a secondary exception that the `SumoTrafficSimulation` will throw when attempting to close a TraCI connection that is closed by an error.
### Removed
- Removed `vehicle_type` from `AgentInterface`, instead get the vehicle type from scenario's agent missions.
### Security

## [0.5.0] - 2022-01-07
Expand Down
6 changes: 0 additions & 6 deletions smarts/core/agent_interface.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -266,11 +266,6 @@ class AgentInterface:
The choice of action space, this action space also decides the controller that will be enabled.
"""

vehicle_type: str = "sedan"
"""
The choice of vehicle type.
"""

accelerometer: Union[Accelerometer, bool] = True
"""
Enable acceleration and jerk observations.
Expand All @@ -293,7 +288,6 @@ def __post_init__(self):
self.accelerometer = AgentInterface._resolve_config(
self.accelerometer, Accelerometer
)
assert self.vehicle_type in {"sedan", "bus"}

@staticmethod
def from_type(requested_type: AgentType, **kwargs):
Expand Down
5 changes: 0 additions & 5 deletions smarts/core/controllers/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ def perform_action(
controller_state,
sensor_state,
action_space,
vehicle_type,
):
"""Calls control for the given vehicle based on a given action space and action.
Args:
Expand All @@ -84,13 +83,9 @@ def perform_action(
The state of a vehicle sensor as relates to vehicle sensors.
action_space:
The action space of the provided action.
vehicle_type:
Vehicle type information about the given vehicle.
"""
if action is None:
return
if vehicle_type == "bus":
liu-allan marked this conversation as resolved.
Show resolved Hide resolved
assert action_space == ActionSpaceType.Trajectory
if action_space == ActionSpaceType.Continuous:
vehicle.control(
throttle=np.clip(action[0], 0.0, 1.0),
Expand Down
12 changes: 1 addition & 11 deletions smarts/core/plan.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
from dataclasses import dataclass, field
from typing import Optional, Tuple, Union

from smarts.sstudio.types import EntryTactic, TrapEntryTactic
from smarts.sstudio.types import EntryTactic, TrapEntryTactic, VehicleSpec

from .coordinates import Dimensions, Heading, Point, Pose, RefLinePoint
from .road_map import RoadMap
Expand Down Expand Up @@ -185,16 +185,6 @@ class Via:
hit_distance: float
required_speed: float


@dataclass(frozen=True)
class VehicleSpec:
"""Vehicle specifications"""

veh_id: str
veh_config_type: str
dimensions: Dimensions


@dataclass(frozen=True)
class Mission:
"""A navigation mission."""
Expand Down
5 changes: 3 additions & 2 deletions smarts/core/scenario.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
PositionalGoal,
Start,
TraverseGoal,
VehicleSpec,
Via,
default_entry_tactic,
)
Expand All @@ -52,6 +51,7 @@
from smarts.sstudio import types as sstudio_types
from smarts.sstudio.types import MapSpec
from smarts.sstudio.types import Via as SSVia
from smarts.sstudio.types import VehicleSpec


class Scenario:
Expand Down Expand Up @@ -689,14 +689,14 @@ def to_scenario_via(
road_map,
)
goal = PositionalGoal(position, radius=2)

return Mission(
start=start,
route_vias=mission.route.via,
goal=goal,
start_time=mission.start_time,
entry_tactic=mission.entry_tactic,
via=to_scenario_via(mission.via, road_map),
vehicle_spec=mission.vehicle_spec,
)
elif isinstance(mission, sstudio_types.EndlessMission):
position, heading = to_position_and_heading(
Expand All @@ -711,6 +711,7 @@ def to_scenario_via(
start_time=mission.start_time,
entry_tactic=mission.entry_tactic,
via=to_scenario_via(mission.via, road_map),
vehicle_spec=mission.vehicle_spec,
)
elif isinstance(mission, sstudio_types.LapMission):
start_road_id, start_lane, start_road_offset = mission.route.begin
Expand Down
12 changes: 8 additions & 4 deletions smarts/core/smarts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -573,7 +573,7 @@ def create_vehicle_in_providers(
provider.create_vehicle(
VehicleState(
vehicle_id=vehicle.id,
vehicle_config_type="passenger",
vehicle_config_type=vehicle._vehicle_config_type,
pose=vehicle.pose,
dimensions=vehicle.chassis.dimensions,
speed=vehicle.speed,
Expand Down Expand Up @@ -928,7 +928,7 @@ def _get_provider_state(self, source: str, action_space_pred) -> ProviderState:
provider_state.vehicles.append(
VehicleState(
vehicle_id=vehicle.id,
vehicle_config_type="passenger",
vehicle_config_type=vehicle._vehicle_config_type,
pose=vehicle.pose,
dimensions=vehicle.chassis.dimensions,
speed=vehicle.speed,
Expand Down Expand Up @@ -1239,7 +1239,6 @@ def _perform_agent_actions(self, agent_actions):
controller_state,
sensor_state,
agent_interface.action_space,
agent_interface.vehicle_type,
)

def _sync_vehicles_to_renderer(self):
Expand Down Expand Up @@ -1354,10 +1353,15 @@ def _try_emit_envision_state(self, provider_state: ProviderState, obs, scores):
for paths in vehicle_obs.road_waypoints.lanes.values()
for path in paths
]

veh_type = (
v.vehicle_config_type if v.vehicle_config_type else v.vehicle_type
)

traffic[v.vehicle_id] = envision_types.TrafficActorState(
name=self._agent_manager.agent_name(agent_id),
actor_type=actor_type,
vehicle_type=envision_types.VehicleType.Car,
vehicle_type=veh_type,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This change seems to cause the ego vehicle to be the wrong color (grey instead of red) on SUMO maps

position=tuple(v.pose.position),
heading=float(v.pose.heading),
speed=v.speed,
Expand Down
14 changes: 11 additions & 3 deletions smarts/core/vehicle.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -394,9 +394,16 @@ def build_agent_vehicle(
SceneColors.Agent.value if trainable else SceneColors.SocialAgent.value
)

if agent_interface.vehicle_type == "sedan":
if mission.vehicle_spec == None:
vehicle_type = "passenger"
controller_type = "sedan"
else:
vehicle_type = mission.vehicle_spec.veh_config_type
Copy link
Contributor Author

Choose a reason for hiding this comment

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

is this the right place to get the vehicle type?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, although I am slightly confused as to why the vehicle_spec is inside the mission rather than in the plan.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

where should vehicle_spec be specified then?

controller_type = vehicle_type

if vehicle_type == "passenger":
urdf_name = "vehicle"
elif agent_interface.vehicle_type == "bus":
elif vehicle_type == "bus":
urdf_name = "bus"
else:
raise Exception("Vehicle type does not exist!!!")
Expand All @@ -406,7 +413,7 @@ def build_agent_vehicle(
vehicle_filepath = str(path.absolute())

controller_parameters = sim.vehicle_index.controller_params_for_vehicle_type(
agent_interface.vehicle_type
controller_type
)

chassis = None
Expand Down Expand Up @@ -438,6 +445,7 @@ def build_agent_vehicle(
id=vehicle_id,
chassis=chassis,
color=vehicle_color,
vehicle_config_type=vehicle_type
)

return vehicle
Expand Down
17 changes: 16 additions & 1 deletion smarts/sstudio/types.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@
from shapely.ops import split, unary_union

from smarts.core import gen_id
from smarts.core.coordinates import RefLinePoint
from smarts.core.coordinates import RefLinePoint, Dimensions
from smarts.core.default_map_builder import get_road_map
from smarts.core.road_map import RoadMap
from smarts.core.utils.id import SocialAgentId
Expand Down Expand Up @@ -464,6 +464,14 @@ class TrapEntryTactic(EntryTactic):
"""The speed that the vehicle starts at when the hijack limit expiry emits a new vehicle"""


@dataclass(frozen=True)
class VehicleSpec:
"""Vehicle specifications"""

veh_id: str
veh_config_type: str
Copy link
Contributor

Choose a reason for hiding this comment

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

This is problematic because we have been and are still distinguishing in our (core) code between veh_type and veh_config_type, where the latter should eventually be fully encapsulated to just the Sumo types (per Issue #1270). I do not think we should be exposing the "config" (Sumo) types in Scenario Studio, but to just change the variable name here to veh_type would also be a misnomer without further unification in the core. Thus, I suggest we deal with this as part of the more comprehensive solution that issue #1270 requires.

dimensions: Dimensions

@dataclass(frozen=True)
class Mission:
"""The descriptor for an actor's mission."""
Expand All @@ -482,6 +490,9 @@ class Mission:
entry_tactic: Optional[EntryTactic] = None
"""A specific tactic the mission should employ to start the mission."""

vehicle_spec: Optional[VehicleSpec] = None
"""Vehicle Specifications"""


@dataclass(frozen=True)
class EndlessMission:
Expand All @@ -503,6 +514,8 @@ class EndlessMission:
"""The earliest simulation time that this mission starts"""
entry_tactic: Optional[EntryTactic] = None
"""A specific tactic the mission should employ to start the mission"""
vehicle_spec: Optional[VehicleSpec] = None
"""Vehicle Specifications"""


@dataclass(frozen=True)
Expand All @@ -521,6 +534,8 @@ class LapMission:
"""The earliest simulation time that this mission starts"""
entry_tactic: Optional[EntryTactic] = None
"""A specific tactic the mission should employ to start the mission"""
vehicle_spec: Optional[VehicleSpec] = None
"""Vehicle Specifications"""


@dataclass(frozen=True)
Expand Down