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

Step Info Ordering - Bug or Documentation Gap #131

Open
bgiddens opened this issue Nov 15, 2023 · 1 comment
Open

Step Info Ordering - Bug or Documentation Gap #131

bgiddens opened this issue Nov 15, 2023 · 1 comment

Comments

@bgiddens
Copy link

bgiddens commented Nov 15, 2023

When the environment is initialized or reset, the assignment of agents to player positions in the environment is randomized. However, this operation applies only to the ordering of actions and observations in the step interface - it does not affect the environment info returned by the step interface, including shaped and sparse rewards. As a result, intuitive use of the interface will lead to misattribution of rewards 50% of the time.

The agent_idx flag is returned with the environment info, which seems like it's intended to give the user a workaround to this problem by allowing them to reinterpret the order of the environment info outside of the environment interface. However, this does not appear to be documented, so users have no way of knowing they need to do this. Users therefore have to reverse engineer source code to understand why their rewards are misattributed, if they catch the issue at all.

Ideas for improving this:

  • reorder the environment info, so that the shuffle operation applies to the entire step interface.
  • introduce an environment parameter to enable/disable shuffling of agents, and issue a warning if using the shuffling feature while it does not apply completely to the step interface.
  • write thorough docs/demos so users have a reasonable way to understand and avoid this pitfall.
@micahcarroll
Copy link
Member

micahcarroll commented Dec 4, 2023

Hi @bgiddens, thanks for pointing this out – you're totally right that it's tough to realize that this is happening under the hood right now.

reorder the environment info, so that the shuffle operation applies to the entire step interface.

This would not be backwards compatible, so maybe this is the change that I'd be most worried about.


Regarding the other two solutions, both seem like good ideas. I currently don't have time to implement them, but given that there seems to be some interest in this issue, if anyone wants to take a stab at doing either of those (or both), I'll happily review a PR!

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

No branches or pull requests

2 participants