-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add a method add_agents() to all multiagents domain + fix paper-rock-scissors multiagent domain #283
Conversation
This is required for instance by mahd solver. It mirrors ray.rllib API (which has get_agent_ids() in their multiagent environments)
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 just added very minor remarks, the rest is OK to merge for me.
tests/autocast/test_agent.py
Outdated
# single agent domain | ||
domain = MyDomain() | ||
|
||
# up-casted as multi-agent domain |
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 believe that the past/passive form of cast is "cast" (not "casted)
tests/autocast/test_agent.py
Outdated
domain = MyDomain() | ||
|
||
# up-casted as multi-agent domain | ||
domainupcasted = MyDomain() |
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.
... so maybe upcast_domain
instead (cf. previous comment)?
Must return a dict for termination attribute of TransitionOutcome
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.
All good for me, thanks!
add_agents()
is supposed to exist by mahd solver and mirrors rllib api (add_agent_ids()
)_state_step()
was not returning a dictionary inTransitionOutcome.termination