Fixing Gym for single and multi-agent following reset changes on C# #3417
Conversation
Co-Authored-By: Chris Elion <chris.elion@unity3d.com>
gym-unity/gym_unity/envs/__init__.py
Outdated
| if info.n_agents() == self._n_agents: | ||
| return info | ||
| n_extra_agents = info.n_agents() - self._n_agents | ||
| if self._n_agents is None: |
There was a problem hiding this comment.
It seems like either this is never true, or the math on the line above is bad (can't do int - None)
There was a problem hiding this comment.
I think this should never be true as self.n_agents will be assigned by the check_agents call, which is done before this method - prob should remove this if
There was a problem hiding this comment.
I got mypy freeking out
ervteng
left a comment
ervteng
left a comment
There was a problem hiding this comment.
Probably more comments and better exceptions but the logic seems OK
gym-unity/gym_unity/envs/__init__.py
Outdated
| n_extra_agents = info.n_agents() - self._n_agents | ||
| if self._n_agents is None: | ||
| self._n_agents = 1 | ||
| if n_extra_agents < 0 or n_extra_agents > 2 * self._n_agents: |
There was a problem hiding this comment.
What about Agents that don't make a decision every step? Wouldn't that lead to info.n_agents() < self._n_agents?
What about an Agent that calls Done() multiple times in the same step? Wouldn't that lead to info.n_agents() > 2 * self._n_agents ?
There was a problem hiding this comment.
Yes, but you should not call Done multiple times per step : )
There was a problem hiding this comment.
Then we should assert that on the C# side.
There was a problem hiding this comment.
And/or provide a more descriptive error message here
There was a problem hiding this comment.
If Done is sent multiple times, the gym will ignore it if it is sent between decision steps (this is what the while loop is for)
| while info.n_agents() < self._n_agents: | ||
| if not info.done.all(): | ||
| raise UnityGymException( | ||
| "The environment does not have the expected amount of agents." |
There was a problem hiding this comment.
Need a more detailed exception here - something like "Agents didn't make decisions at the same time"
There was a problem hiding this comment.
Added to the error
gym-unity/gym_unity/envs/__init__.py
Outdated
| else: | ||
| self._env.step() | ||
| info = self._env.get_step_result(self.brain_name) | ||
| while info.n_agents() < self._n_agents: |
There was a problem hiding this comment.
Comment on the scary-looking while loop maybe
There was a problem hiding this comment.
Let me know what you think
There was a problem hiding this comment.
Should we add to the documentation that the gym can only be used with environments in which the agents request decisions at the same time and not to use the offset feature?
Co-Authored-By: Chris Elion <chris.elion@unity3d.com>
Co-Authored-By: Chris Elion <chris.elion@unity3d.com>
Co-Authored-By: Chris Elion <chris.elion@unity3d.com>
Co-Authored-By: Chris Elion <chris.elion@unity3d.com>
vincentpierre
changed the title
* rename and comments * enumerate
No description provided.