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

Writing tests for transforms #79

Merged
merged 18 commits into from
Apr 20, 2022
Merged

Writing tests for transforms #79

merged 18 commits into from
Apr 20, 2022

Conversation

vmoens
Copy link
Contributor

@vmoens vmoens commented Apr 19, 2022

Introduces tests for torchrl.envs.transforms classes.
Together with this PR, we also change the logic by which envs return observations.

Before

Each env.step returned a tensordict with various keys. All the new observations were labelled with the prefix'next_observation'.
The envs observation_spec attribute could have been any type of TensorSpec.

Now

We expect all observation_spec to be of the Composite type. This ensures that all observations are signed with their keyword.
It is not expected anymore that all observations have the 'next_observation' prefix, but each key of the Composite TensorSpec must start with 'next_'. Therefore, the signature of the env step is consistent: the observation spec points to 'next_' keys that can be found when calling step().
For reset, while it is still the default behaviour that the 'next_' is removed, it can be recovered by setting the execute_step to False. This is used by _BatchedEnv subclasses and by the TransformedEnv class to make sure that all observations are marked with a next_ (which is necessary for the transforms to be applied, for instance).

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Apr 19, 2022
@vmoens vmoens added enhancement New feature or request bc breaking backward compatibility breaking change major major refactoring in the code base naming Naming convention in the code labels Apr 20, 2022
@vmoens vmoens marked this pull request as ready for review April 20, 2022 09:47
@vmoens vmoens merged commit fa7bce2 into main Apr 20, 2022
@vmoens vmoens deleted the test_transforms branch April 20, 2022 09:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bc breaking backward compatibility breaking change CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. enhancement New feature or request major major refactoring in the code base naming Naming convention in the code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants