rl icon indicating copy to clipboard operation
rl copied to clipboard

[Feature] Transform for Recording History Observations

Open btx0424 opened this issue 2 years ago • 6 comments

Description

This draft PR proposes a Transform for recording histories of observations, which is a common practice in robotic tasks. It should also address #1676: to record observation-action history, append the previous action to observation, and then transform the environment with History.

The naming, description (docstring), and tests are for the initial commit. Opinions and suggestions are wanted.

  • [x] I have raised an issue to propose this change (required for new features and bug fixes)

Types of changes

What types of changes does your code introduce? Remove all that do not apply:

  • [ ] Bug fix (non-breaking change which fixes an issue)
  • [x] New feature (non-breaking change which adds core functionality)
  • [ ] Breaking change (fix or feature that would cause existing functionality to change)
  • [ ] Documentation (update in the documentation)
  • [ ] Example (update in the folder of examples)

Checklist

Go over all the following points, and put an x in all the boxes that apply. If you are unsure about any of these, don't hesitate to ask. We are here to help!

  • [x] I have read the CONTRIBUTION guide (required)
  • [ ] My change requires a change to the documentation.
  • [x] I have updated the tests accordingly (required for a bug fix or a new feature).
  • [ ] I have updated the documentation accordingly.

btx0424 avatar Dec 28 '23 14:12 btx0424

:link: Helpful Links

:test_tube: See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/rl/1763

Note: Links to docs will display an error until the docs builds have been completed.

:white_check_mark: You can merge normally! (16 Unrelated Failures)

As of commit 6c1109e9451faa256dc69d59f7c564b4bdc32032 with merge base eec9f9e4bb0270e2d9c567836dd397f154c4f2fc (image):

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

pytorch-bot[bot] avatar Dec 28 '23 14:12 pytorch-bot[bot]

Thanks for replying!

Yes, I think renaming it to StackFrames is a good idea. However, do you prefer it to be stateful (like CatFrames) or stateless (the current implementation)? I will make it reuse CatFrames more if we go for stateful.

I will complete the tests and doc soon.

btx0424 avatar Jan 06 '24 16:01 btx0424

It's good to have it stateless I agree. Let's keep it as it is! Let's align the doc of this class and CatFrames and refer one to the other, it's likely that users will jump from one to the other. Thanks again!

vmoens avatar Jan 09 '24 13:01 vmoens

@btx0424 Anything I can help you with here? I can find some time to help if needed

vmoens avatar Feb 05 '24 08:02 vmoens

@btx0424 Anything I can help you with here? I can find some time to help if needed

Hi there. I had some trouble figuring out the no-env usage while trying to align it with CatFrames. I noticed that CatFrames, when used for RBs, do not split between trajectories, i.e., some observations may contain information from the last episode. Is that the desired behavior?

btx0424 avatar Feb 07 '24 09:02 btx0424

Just seeing this now sorry No definitely not! We should fix this if it's the case

vmoens avatar Feb 15 '24 20:02 vmoens