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

handle multiple dones in a single step #3700

Merged
merged 5 commits into from
Mar 27, 2020
Merged

handle multiple dones in a single step #3700

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
812ccfc
handle multiple dones in a single step
Mar 26, 2020
eb7a460
test new agents done in same step
Mar 26, 2020
6415d11
handle in reference impl too
Mar 26, 2020
6e1e82f
update changelog
Mar 26, 2020
b61fa76
Merge remote-tracking branch 'origin/master' into MLA-805-handle-mult…
Mar 26, 2020
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
15 changes: 11 additions & 4 deletions gym-unity/gym_unity/envs/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,9 +364,8 @@ def _check_agents(self, n_agents: int) -> None:

def _sanitize_info(self, step_result: BatchedStepResult) -> BatchedStepResult:
n_extra_agents = step_result.n_agents() - self._n_agents
if n_extra_agents < 0 or n_extra_agents > self._n_agents:
if n_extra_agents < 0:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not 100% it's safe to remove this, but it's what was triggering in the single agent case (with 2 "done" agents)

# In this case, some Agents did not request a decision when expected
# or too many requested a decision
raise UnityGymException(
"The number of agents in the scene does not match the expected number."
)
Expand All @@ -386,6 +385,10 @@ def _sanitize_info(self, step_result: BatchedStepResult) -> BatchedStepResult:
# only cares about the ordering.
for index, agent_id in enumerate(step_result.agent_id):
if not self._previous_step_result.contains_agent(agent_id):
if step_result.done[index]:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just making the mark_agent_done() change without this means that we'll register the agent and run out of "slots" later.

# If the Agent is already done (e.g. it ended its epsiode twice in one step)
# Don't try to register it here.
continue
# Register this agent, and get the reward of the previous agent that
# was in its index, so that we can return it to the gym.
last_reward = self.agent_mapper.register_new_agent_id(agent_id)
Expand Down Expand Up @@ -528,8 +531,12 @@ def mark_agent_done(self, agent_id: int, reward: float) -> None:
"""
Declare the agent done with the corresponding final reward.
"""
gym_index = self._agent_id_to_gym_index.pop(agent_id)
self._done_agents_index_to_last_reward[gym_index] = reward
if agent_id in self._agent_id_to_gym_index:
gym_index = self._agent_id_to_gym_index.pop(agent_id)
Copy link
Contributor Author

@chriselion chriselion Mar 26, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

This is where the reported exceptions are. We could also make a check before the call to mark_agent_done, in _sanitize_info() but it seemed a bit cleaner to do it here.

self._done_agents_index_to_last_reward[gym_index] = reward
else:
# Agent was never registered in the first place (e.g. EndEpisode called multiple times)
pass

def register_new_agent_id(self, agent_id: int) -> float:
"""
Expand Down