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

[Question] Why does MaskablePPO does not mask with some logic with last observation? #234

Open
4 tasks done
EloyAnguiano opened this issue Mar 21, 2024 · 4 comments
Open
4 tasks done
Labels
question Further information is requested

Comments

@EloyAnguiano
Copy link

❓ Question

At MaskablePPO class, the change for getting the masks is to ask the environment to provide it by he function get_action_mask. I can see that the get_action_mask only gets the environment object as input, but at that point we also have the self._last_obs variable. To provide the action mask more information about the observation it is facing, It would be interesting to provide that method with the last observation object, isn't it? I am thinking about a game that has some logic that we want to keep and code to prevent the agent making those actions.

I assume that I am not the first thinking this so, is it a performance killer to do like so? Has it something to do with the environment vectorizations?

Checklist

@EloyAnguiano EloyAnguiano added the question Further information is requested label Mar 21, 2024
@araffin
Copy link
Member

araffin commented Mar 31, 2024

It would be interesting to provide that method with the last observation object, isn't it? I am thinking about a game that has some logic that we want to keep and code to prevent the agent making those actions.

You can add that logic in the environment code, no? (that action mask may depend on previous observation or any other variable that represent the current env)

action_masks = get_action_masks(env)

@EloyAnguiano
Copy link
Author

Yes, and I am doing that indeed, but the problem with this order of things is that you have to calculate the action mask for time t usint the observation at t-1, and changing this order could be useful to code come logic at mask t with the observation at t (even at the t=0 case)

@araffin
Copy link
Member

araffin commented Apr 3, 2024

this order could be useful to code come logic at mask t with the observation at t (even at the t=0 case)

This is what is currently done, no? (the action mask depends on obs at t)

@EloyAnguiano
Copy link
Author

Yes, you are right. It was a mistake at my environment code.

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

No branches or pull requests

2 participants