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

Fix agent indicator inf bounded spaces #240

Conversation

KaleabTessera
Copy link
Contributor

Fixes the agent indicator so that it works with envs that have unbounded spaces (space.high=np.inf) e.g. simple spread.

Closes #239.

return new_obs
elif ndims == 3 or ndims == 2:
obs = obs if ndims == 3 else np.expand_dims(obs, 2)
old_shaped3 = obs.shape[2]
new_obs = np.pad(obs, [(0, 0), (0, 0), (0, num_indicators)])
new_obs[:, :, old_shaped3 + indicator_num] = np.min(space.high)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only question @KaleabTessera is why was the code previously np.min(space.high) whereas now it is np.max(space.high), would this not mean different behavior?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm going to trust your tests ensure this behavior is correct and make a release because I need to today for something else, if this ends up being a bug we can do a hotfix release fixing it though

Copy link
Member

@jjshoots jjshoots Jan 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have this same question too, would like a clarification. I'm guessing it makes it easier for RL agents to see the indicator if they internally normalize observation, but this would change behaviour of all agents downstream.

@elliottower elliottower merged commit 07f4d3a into Farama-Foundation:master Jan 18, 2024
5 checks passed
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

Successfully merging this pull request may close these issues.

Bug when using the agent_indicator_v0 wrapper with unbounded envs (where space.high=np.inf)
3 participants