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

Fix SMARTS ignores social agent start time. #1899

Merged
merged 9 commits into from
Mar 10, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ Copy and pasting the git commit messages is __NOT__ enough.
### Changed
### Deprecated
### Fixed
- Fixed an issue with SMARTS where the social vehicles started instantly regardless of what mission start time they were given.
### Removed
### Security

Expand Down
16 changes: 9 additions & 7 deletions scenarios/sumo/zoo_intersection/scenario.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,17 +86,19 @@
social_agent_missions={
f"s-agent-{social_agent2.name}": (
[social_agent2],
[Mission(RandomRoute())],
),
f"s-agent-{social_agent1.name}": (
[social_agent1],
[
EndlessMission(begin=("edge-south-SN", 0, 30)),
Mission(
Route(
begin=("edge-west-WE", 0, 10), end=("edge-east-WE", 0, 10)
)
begin=("edge-south-SN", 0, 30), end=("edge-east-WE", 0, 10)
),
),
],
),
f"s-agent-{social_agent1.name}": (
[social_agent1],
[
EndlessMission(begin=("edge-south-SN", 0, 10), start_time=0.7),

],
),
},
Expand Down
64 changes: 42 additions & 22 deletions smarts/core/agent_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ def __init__(self, sim, interfaces, zoo_addrs=None):
# would not be included
self._initial_interfaces = interfaces
self._pending_agent_ids = set()
self._pending_social_agent_ids = set()

# Agent interfaces are interfaces for _all_ active agents
self._agent_interfaces = {}
Expand Down Expand Up @@ -118,6 +119,11 @@ def pending_agent_ids(self) -> Set[str]:
"""The IDs of agents that are waiting to enter the simulation"""
return self._pending_agent_ids

@property
def pending_social_agent_ids(self) -> Set[str]:
"""The IDs of social agents that are waiting to enter the simulation"""
return self._pending_social_agent_ids
Comment on lines +122 to +125
Copy link
Collaborator Author

@Gamenot Gamenot Mar 9, 2023

Choose a reason for hiding this comment

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

I am unsure if I should distinguish between social agents and ego agents in this way but it is easy to lose the context of what the agent is if I do not separate them.


@property
def active_agents(self) -> Set[str]:
"""A list of all active agents in the simulation (agents that have a vehicle.)"""
Expand Down Expand Up @@ -460,28 +466,7 @@ def _setup_social_agents(self):
sim = self._sim()
assert sim
social_agents = sim.scenario.social_agents
if social_agents:
self._setup_agent_buffer()
else:
return

self._remote_social_agents = {
agent_id: self._agent_buffer.acquire_agent() for agent_id in social_agents
}

for agent_id, (social_agent, social_agent_model) in social_agents.items():
self._add_agent(
agent_id,
social_agent.interface,
social_agent_model,
trainable=False,
# XXX: Currently boids can only be run from bubbles
boid=False,
)
self._social_agent_ids.add(agent_id)

for social_agent_id, remote_social_agent in self._remote_social_agents.items():
remote_social_agent.start(social_agents[social_agent_id][0])
Comment on lines -463 to -484
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Essentially, the agent manager was in control of determining when social agents should enter the simulation... which was completely wrong. We have the bubble manager and trap manager for that.

self._pending_social_agent_ids.update(social_agents.keys())

def _start_keep_alive_boid_agents(self):
"""Configures and adds boid agents to the sim."""
Expand Down Expand Up @@ -510,6 +495,41 @@ def _start_keep_alive_boid_agents(self):
)
self.start_social_agent(agent_id, social_agent, social_agent_data_model)

def add_and_emit_social_agent(
self, agent_id: str, agent_spec, agent_model: SocialAgent
):
"""Generates an entirely new social agent and emits a vehicle for it immediately.

Args:
agent_id (str): The agent id for the new agent.
agent_spec (AgentSpec): The agent spec of the new agent
agent_model (SocialAgent): The agent configuration of the new vehicle.
Returns:
bool:
If the agent is added. False if the agent id is already reserved
by a pending ego agent or current social/ego agent.
"""
if agent_id in self.agent_ids or agent_id in self.pending_agent_ids:
return False

self._setup_agent_buffer()
remote_agent = self._agent_buffer.acquire_agent()
self._add_agent(
agent_id=agent_id,
agent_interface=agent_spec.interface,
agent_model=agent_model,
trainable=False,
boid=False,
)
if agent_id in self._pending_social_agent_ids:
self._pending_social_agent_ids.remove(agent_id)
remote_agent.start(agent_spec=agent_spec)
self._remote_social_agents[agent_id] = remote_agent
self._agent_interfaces[agent_id] = agent_spec.interface
self._social_agent_ids.add(agent_id)
self._social_agent_data_models[agent_id] = agent_model
return True

def _add_agent(
self, agent_id, agent_interface, agent_model, boid=False, trainable=True
):
Expand Down
97 changes: 74 additions & 23 deletions smarts/core/trap_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,10 @@ def step(self, sim):
captures_by_agent_id = defaultdict(list)

# Do an optimization to only check if there are pending agents.
if not sim.agent_manager.pending_agent_ids:
if (
not sim.agent_manager.pending_agent_ids
| sim.agent_manager.pending_social_agent_ids
):
return

social_vehicle_ids: List[str] = [
Expand All @@ -184,7 +187,10 @@ def largest_vehicle_plane_dimension(vehicle: Vehicle):
for v in vehicles.values()
]

for agent_id in sim.agent_manager.pending_agent_ids:
for agent_id in (
sim.agent_manager.pending_agent_ids
| sim.agent_manager.pending_social_agent_ids
):
trap = self._traps.get(agent_id)

if trap is None:
Expand Down Expand Up @@ -224,29 +230,32 @@ def largest_vehicle_plane_dimension(vehicle: Vehicle):
),
)
)
# TODO: Resolve overlap using a tree instead of just removing.
social_vehicle_ids.remove(v_id)
break

# Use fed in trapped vehicles.
agents_given_vehicle = set()
used_traps = []
for agent_id in sim._agent_manager.pending_agent_ids:
if agent_id not in self._traps:
continue

trap = self._traps[agent_id]
for agent_id in (
sim.agent_manager.pending_agent_ids
| sim.agent_manager.pending_social_agent_ids
):
trap = self._traps.get(agent_id)

captures = captures_by_agent_id[agent_id]
if trap is None:
continue

if not trap.ready(sim.elapsed_sim_time):
continue

captures = captures_by_agent_id[agent_id]

vehicle = None
if len(captures) > 0:
vehicle_id, trap, mission = rand.choice(captures)
vehicle = sim.switch_control_to_agent(
vehicle_id, agent_id, mission, recreate=True, is_hijacked=False
vehicle = self._take_existing_vehicle(
sim,
vehicle_id,
agent_id,
mission,
social=agent_id in sim.agent_manager.pending_social_agent_ids,
)
elif trap.patience_expired(sim.elapsed_sim_time):
# Make sure there is not a vehicle in the same location
Expand All @@ -265,28 +274,50 @@ def largest_vehicle_plane_dimension(vehicle: Vehicle):
if overlapping:
continue

vehicle = TrapManager._make_vehicle(
sim, agent_id, mission, trap.default_entry_speed
vehicle = TrapManager._make_new_vehicle(
sim,
agent_id,
mission,
trap.default_entry_speed,
social=agent_id in sim.agent_manager.pending_social_agent_ids,
)
else:
continue
if vehicle == None:
if vehicle is None:
continue
sim.create_vehicle_in_providers(vehicle, agent_id, True)
agents_given_vehicle.add(agent_id)
used_traps.append((agent_id, trap))

if len(agents_given_vehicle) > 0:
if len(used_traps) > 0:
self.remove_traps(used_traps)
sim.agent_manager.remove_pending_agent_ids(agents_given_vehicle)

@property
def traps(self) -> Dict[str, Trap]:
"""The traps in this manager."""
return self._traps

@staticmethod
def _make_vehicle(sim, agent_id, mission, initial_speed):
def _take_existing_vehicle(sim, vehicle_id, agent_id, mission, social=False):
from smarts.core.smarts import SMARTS

assert isinstance(sim, SMARTS)
if social:
# Not supported
return None
Comment on lines +309 to +310
Copy link
Collaborator Author

@Gamenot Gamenot Mar 9, 2023

Choose a reason for hiding this comment

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

Unsure if I should support the social agent taking over social vehicles, it may be reasonable but only if I add another entry tactic.

We classically have not taken over vehicles for social agents.

vehicle = sim.switch_control_to_agent(
vehicle_id, agent_id, mission, recreate=True, is_hijacked=False
)
if vehicle is not None:
sim.agent_manager.remove_pending_agent_ids({agent_id})
sim.create_vehicle_in_providers(vehicle, agent_id, True)
return vehicle

@staticmethod
def _make_new_vehicle(sim, agent_id, mission, initial_speed, social=False):
from smarts.core.smarts import SMARTS

assert isinstance(sim, SMARTS)
if social:
return TrapManager._make_new_social_vehicle(sim, agent_id, initial_speed)
agent_interface = sim.agent_manager.agent_interface_for_agent_id(agent_id)
plan = Plan(sim.road_map, mission)
# 3. Apply agent vehicle association.
Expand All @@ -302,11 +333,31 @@ def _make_vehicle(sim, agent_id, mission, initial_speed):
initial_speed=initial_speed,
boid=False,
)
if vehicle is not None:
sim.agent_manager.remove_pending_agent_ids({agent_id})
sim.create_vehicle_in_providers(vehicle, agent_id, True)
return vehicle

@staticmethod
def _make_new_social_vehicle(sim, agent_id, initial_speed):
from smarts.core.smarts import SMARTS

sim: SMARTS = sim
social_agent_spec, social_agent_model = sim.scenario.social_agents[agent_id]

social_agent_model = replace(social_agent_model, initial_speed=initial_speed)
sim.agent_manager.add_and_emit_social_agent(
agent_id,
social_agent_spec,
social_agent_model,
)
vehicles = sim.vehicle_index.vehicles_by_actor_id(agent_id)

return vehicles[0] if len(vehicles) else None

def reset(self):
"""Resets to a pre-initialized state."""
self.captures_by_agent_id = defaultdict(list)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This attribute was not used.

pass

def teardown(self):
"""Clear internal state"""
Expand Down
61 changes: 36 additions & 25 deletions smarts/env/tests/test_social_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,28 +25,27 @@
from smarts.core.agent import Agent
from smarts.core.agent_interface import AgentInterface, AgentType
from smarts.core.utils.episodes import episodes
from smarts.zoo.agent_spec import AgentSpec
from smarts.env.hiway_env import HiWayEnv

AGENT_ID = "Agent-007"
SOCIAL_AGENT_ID = "Alec Trevelyan"

MAX_EPISODES = 3
MAX_EPISODES = 1
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There did not seem to be a point to having multiple episodes for this test.



@pytest.fixture
def agent_spec():
return AgentSpec(
interface=AgentInterface.from_type(AgentType.Laner, max_episode_steps=100),
agent_builder=lambda: Agent.from_function(lambda _: "keep_lane"),
def agent_interface():
return AgentInterface.from_type(
AgentType.Laner, max_episode_steps=100, neighborhood_vehicle_states=True
)


@pytest.fixture
def env(agent_spec):
def env(agent_interface: AgentInterface):
env = gym.make(
"smarts.env:hiway-v0",
scenarios=["scenarios/sumo/zoo_intersection"],
agent_specs={AGENT_ID: agent_spec},
agent_interfaces={AGENT_ID: agent_interface},
headless=True,
visdom=False,
fixed_timestep_sec=0.01,
Expand All @@ -56,29 +55,41 @@ def env(agent_spec):
env.close()


def test_social_agents(env, agent_spec):
episode = None
for episode in episodes(n=MAX_EPISODES):
agent = agent_spec.build_agent()
def test_social_agents_not_in_env_obs_keys(env: HiWayEnv):
for _ in range(MAX_EPISODES):
observations = env.reset()
episode.record_scenario(env.scenario_log)

dones = {"__all__": False}
while not dones["__all__"]:
obs = observations[AGENT_ID]
observations, rewards, dones, infos = env.step({AGENT_ID: agent.act(obs)})
episode.record_step(observations, rewards, dones, infos)
observations, rewards, dones, infos = env.step({AGENT_ID: "keep_lane"})

assert SOCIAL_AGENT_ID not in observations
assert SOCIAL_AGENT_ID not in dones

# Reward is currently the delta in distance travelled by this agent.
# We want to make sure that this is infact a delta and not total distance
# travelled since this bug has appeared a few times.
#
# The way to verify this is by making sure the reward does not grow without bounds
assert -3 < rewards[AGENT_ID] < 3

assert episode is not None and episode.index == (
MAX_EPISODES - 1
), "Simulation must cycle through to the final episode"
def test_social_agents_in_env_neighborhood_vehicle_obs(
env: HiWayEnv, agent_interface: AgentInterface
):
first_seen_vehicles = {}
for _ in range(MAX_EPISODES):
observations = env.reset()

dones = {"__all__": False}
while not dones["__all__"]:
observations, rewards, dones, infos = env.step({AGENT_ID: "keep_lane"})

new_nvs_ids = [
nvs.id
for nvs in observations[AGENT_ID].neighborhood_vehicle_states
if nvs.id not in first_seen_vehicles
]
for v_id in new_nvs_ids:
first_seen_vehicles[v_id] = observations[AGENT_ID].step_count + 1

seen_zoo_social_vehicles = [v_id for v_id in first_seen_vehicles if "zoo" in v_id]
assert len(seen_zoo_social_vehicles) == 2
late_entry = next(
(v_id for v_id in seen_zoo_social_vehicles if "zoo-car1" in v_id), None
)
assert late_entry is not None, seen_zoo_social_vehicles
Comment on lines +91 to +94
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Suggested change
late_entry = next(
(v_id for v_id in seen_zoo_social_vehicles if "zoo-car1" in v_id), None
)
assert late_entry is not None, seen_zoo_social_vehicles
assert (late_entry := next(
(v_id for v_id in seen_zoo_social_vehicles if "zoo-car1" in v_id), None
))

I would use the walrus assignment operator if the 3.8 upgrade goes in.

assert first_seen_vehicles[late_entry] == 70