-
Notifications
You must be signed in to change notification settings - Fork 190
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
Conversation
def reset(self): | ||
"""Resets to a pre-initialized state.""" | ||
self.captures_by_agent_id = defaultdict(list) |
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.
This attribute was not used.
|
||
AGENT_ID = "Agent-007" | ||
SOCIAL_AGENT_ID = "Alec Trevelyan" | ||
|
||
MAX_EPISODES = 3 | ||
MAX_EPISODES = 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.
There did not seem to be a point to having multiple episodes for this test.
# Not supported | ||
return None |
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.
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.
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]) |
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.
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.
@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 |
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 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.
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 |
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.
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.
This fixes an issue with SMARTS where the social vehicles started instantly regardless of what mission start time they were given.