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

[BUG] TransformedEnv does not copy env properties #1806

Closed
matteobettini opened this issue Jan 16, 2024 · 2 comments · Fixed by #1810
Closed

[BUG] TransformedEnv does not copy env properties #1806

matteobettini opened this issue Jan 16, 2024 · 2 comments · Fixed by #1810
Assignees
Labels
bug Something isn't working

Comments

@matteobettini
Copy link
Contributor

When wrapping an env in TransofrmedEnv, its properties are changed

from torchrl.envs import VmasEnv, RewardSum, TransformedEnv

if __name__ == "__main__":
    env = VmasEnv("balance", num_envs=3)
    print(env._allow_done_after_reset)
    t_env = TransformedEnv(env, RewardSum())
    print(t_env._allow_done_after_reset)
True
False
@matteobettini matteobettini added the bug Something isn't working label Jan 16, 2024
@matteobettini matteobettini changed the title [BUG] TranformedEnv does not copy env properties [BUG] TransformedEnv does not copy env properties Jan 16, 2024
@vmoens
Copy link
Contributor

vmoens commented Jan 16, 2024

Is this a generic bug or just specific to _allow_done_after_reset?

Not sure it's really a bug in general, there are instances where one would like to have different attributes in the transformed env and in the env.
For some (eg _allow_done_after_reset?) we can have fallbacks if we're confident that there is no reason for TransformedEnv to have a different behaviour. It's already the case for batch_size, device, run_type_checks and others.

For instance, I can imagine that if one is using the no-op transform she expects the behaviour of _allow_done_after_reset to change.

@matteobettini
Copy link
Contributor Author

matteobettini commented Jan 16, 2024

We can default to the attribute value in the original env and the specific transforms can change it.

I think it is counterintuitive that, if the transform does not need to change the property, the wrapper changes it.

Not sure if it is just specific to this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants