[Proposal] remove/change assert agent in last()
gnoLLex opened this issue · 3 comments
Proposal
In the AECEnv the method last
assert the agent. I would propose to check if the agent is not None
or remove the assertion entirely, if the agent_selector
cant return invalid agents anyway.
Motivation
I'm currently trying to implement some homogeneous multi agent system, where the a specific AgentID does not matter. I just used int
(the index of the agent) as id's. Now if last
is called and the agent with id 0
is selected the assertion fails even though the id is valid.
Pitch
Check if this is a desired behavior. If not change assert agent
to assert agent != None
in last
of AECEnv
or remove the check entirely.
Alternatives
No response
Additional context
Maybe I'm confused about this and I'm just starting to use this PettingZoo, but from having a quick glance at agent_selector
I would assume it to always return a valid id.
Also shouldn't the agent_selector
also use AgentID
as a generic?
Checklist
- I have checked that there is no similar issue in the repo
Ahhh good catch, yeah for integers assert zero fails. We can change it to assert agent is not None. I believe when a new contributor added support for agent ids to be integers there was a test case added, but if not that also should be done. Is your use case nonstandard in any other way that we could test for to be more comprehensive?
Our code coverage turns out to be over 90% so we are testing most parts of the code but there’s definitely some edge cases that aren’t covered
For now I don't think there is a need for any other tests.
Cool let me know if you encounter any other issues then