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.

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

Always return full state dict (even when only one agent) #328

Open
sethmnielsen opened this issue Aug 31, 2019 · 5 comments
Open

Always return full state dict (even when only one agent) #328

sethmnielsen opened this issue Aug 31, 2019 · 5 comments
Labels

Comments

@sethmnielsen
Copy link

Describe the bug
In some cases, generic code that is meant to be compatible with any of the Holodeck worlds is required (more specifically for my application: any of the worlds that contain a UavAgent). Currently, it is not possible to access the state after ticking with the same syntax for all the worlds containing UavAgent. This is because some worlds have multiple agents while others have only one, and the dictionary returned for multiple agents is agent name to sensor, but for a single agent world it is a sensor to data dict.

I understand that the setup code for the two different world types needs to be different, but in my case, I have separated the setup code from the main script, and the setup code is different depending on which world is specified. My main script, however, contains a lot of logic and operations that are meant to work for a UavAgent in any world, and I need to access the state multiple times throughout.

To Reproduce
The following code works fine for a multi-agent world:

uav_state = env.tick()['uav0']

But fails if the world has only one agent.

Expected behavior
It seems to me that the syntax should be consistent regardless of the number of agents. I don't see a strong need for a different interface between one or multiple agents (other than for slightly simpler code in the single-agent case), and I am of the opinion that the state should simply always return as an agent name to sensor dict.

I tested changing the following lines of environments.py (in the reset function) from

if self.num_agents == 1:
    self._default_state_fn = self._get_single_state
else:
    self._default_state_fn = self._get_full_state

to

self._default_state_fn = self._get_full_state

and the code for accessing the state in a multi-agent world (the Ocean world) had no problems accessing the state in a single-agent world (UrbanCity) as well.

I realize I may be missing something and am open to other solutions.

Version Information:

  • OS: Ubuntu
  • Version: 18.04
  • Holodeck Version: 0.2.2dev (boat-freeze branch)
  • World/Scenario version: 0.2.2dev

Additional context
Also, I noticed that self._default_state_fn is set in both the __init__ and reset functions of HolodeckEnvironment. Isn't one or the other redundant?

@sethmnielsen
Copy link
Author

sethmnielsen commented Aug 31, 2019

After further testing, I found that the following code works in both situations, without modifying the python source code:

uav = env.agents['uav0']
env.tick()
state = uav.agent_state_dict

Should I just stick with this workaround?

FYI, it took a good while of searching through the code to find this solution. I don't see the agents dict in the documentation, and the agent_state_dict was kind of hidden. So if this is a good solution, then maybe it should be made clearer in the examples/docs?

@nickwalton
Copy link
Collaborator

Hi Seth,

Good point it would probably make sense to make one agent and multi agent syntax the same. We'll look into what the best fix is.

@jaydenmilne
Copy link
Contributor

I believe that it was done this way so that with one agent the API would be the same as OpenAI Gym, since that is one of the goals of Holodeck. Since there is no standard multiagent API in Gym, the other method was used.

@nickwalton
Copy link
Collaborator

We discussed and decided that while env.step() should follow openai GYM api. But env.tick() should behave the same whether its a single or multi agent world @allisoncl8

@daniekpo
Copy link
Contributor

@nickwalton, can we close this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants