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.

Sign up for GitHub

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

Jump to bottom

Overhaul Observation Transformation API + Better 360 and multisensor support. #478

Merged
merged 36 commits into from
Sep 16, 2020
Merged

Overhaul Observation Transformation API + Better 360 and multisensor support. #478

merged 36 commits into from
Sep 16, 2020

Conversation

Skylion007
Copy link
Contributor

@Skylion007 Skylion007 commented Sep 8, 2020 •
edited
Loading

Motivation and Context

Solves #367

Okay, this ObservationTransformer API does the following:

  • Adds a new observation transformer registry
  • Videos generated now show the final output of the network after all the observation transformers have been fed in.
  • Still not ideal since Observations may need to be normalized/ denormalized in each operation. Should fix this longterm.
  • Implemented a demo cubemap2equirect that stitches together 6 sensors into one using this API. Creating a 360 Sensor.
  • Remove unnecessary float conversion in PPO trainer.
  • Reads in video input from batched input now.
  • ObservationTransformers separated from Policies.
  • Introduces a new API to run.py that can feed in a config without writing it to disk.
  • Add more documentations
  • Add more typing to better document what is going on.
  • Adds tests to cover the change.
  • Changes the policy fromConfig API to not have access to the full environment.

How Has This Been Tested

With Pytest and by splitting the ResizeCenterCropper into two PyTorch modules

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have completed my CLA (see CONTRIBUTING)
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Sep 8, 2020
@Skylion007 Skylion007 mentioned this pull request Sep 8, 2020
Closed
@Skylion007 Skylion007 requested a review from mathfac September 11, 2020 15:35
@Skylion007 Skylion007 mentioned this pull request Sep 15, 2020
Merged
@Skylion007
Copy link
Contributor Author

All tests are now passing. Ready to get this merged. Only thing I am unsure of is the ObsTransformer name and if I should move it from the RL.POLICY to just RL.

@Skylion007
Copy link
Contributor Author

I am going to move the OBS_TRANSFORMER out of RL.POLICY to RL. and willl add the doc strings.

@Skylion007
Copy link
Contributor Author

I am still not sure what scope to put the config under, thoughts @erikwijmans I could RL.OBS_TRANSFORMER, but I could also see the arguement for RL.POLICY.OBS_TRANSFORMER. It's general enough to just be at the root node too though.

@Skylion007 Skylion007 mentioned this pull request Sep 16, 2020
Closed
@Skylion007 Skylion007 merged commit 803a677 into master Sep 16, 2020
@Skylion007 Skylion007 deleted the obs_trans_registry branch September 16, 2020 20:21
@Skylion007 Skylion007 mentioned this pull request Sep 19, 2020
Open
@jacobkrantz jacobkrantz mentioned this pull request Oct 21, 2020
Closed
srama2512 pushed a commit that referenced this pull request Mar 15, 2023
@msbaines
[esp/sim] collapse SimulatorWithAgents into Simulator (#478)
8336948 
* [esp/sim] collapse SimulatorWithAgents into Simulator

* Fix simulator reset behaviour to reset Agent to initial state.
dannymcy pushed a commit to dannymcy/habitat-lab that referenced this pull request Jul 8, 2024
@Skylion007
Overhaul Observation Transformation API + Better 360 and multisensor …
4b313be 
…support. (facebookresearch#478)

* Add UUID config to simulator visual sensors

* Add comments about UUID and add to BumpSensor

* Incorporate suggestions

* Remove unused code

* Make base sensor have overidable UUID

* Added observation transformer config values

* Remove duplicated class

* Bugfixes and missing fromConfig params

* Remove debug statements and add more type hints

* More typehints

* Improve docstring

* Apply suggestions from code review

Co-authored-by: Oleksandr <maksymets.o@gmail.com>

* Address most of the comments

* Refactor to use attr

* Update API to have access to entire dict

* bugfixes

* More bugfixes

* more bugfixes

* Major refactor + Obs_Transformer overhaul

* Fix license comment

Co-authored-by: Oleksandr <maksymets.o@gmail.com>

* Remove observation transformers

* Fix errors and merge conflicts

* Fix issues, refactor and add tests

* Fix minor bug

* address comments

* Fix test and doc bug

* More docstring improvements

* Fix bug in ResizeShortestEdge

* remove attrs from nn.module

* Fix remaining bugs and refactor Cube2Equirec for smart device selection

* Improve obs_space handling in PPO and change defaults of Cube2EQ

* more doc strings

* bump for ci
HHYHRHY pushed a commit to SgtVincent/EMOS that referenced this pull request Aug 31, 2024
@Skylion007
Overhaul Observation Transformation API + Better 360 and multisensor …
cb1901f 
…support. (facebookresearch#478)

* Add UUID config to simulator visual sensors

* Add comments about UUID and add to BumpSensor

* Incorporate suggestions

* Remove unused code

* Make base sensor have overidable UUID

* Added observation transformer config values

* Remove duplicated class

* Bugfixes and missing fromConfig params

* Remove debug statements and add more type hints

* More typehints

* Improve docstring

* Apply suggestions from code review

Co-authored-by: Oleksandr <maksymets.o@gmail.com>

* Address most of the comments

* Refactor to use attr

* Update API to have access to entire dict

* bugfixes

* More bugfixes

* more bugfixes

* Major refactor + Obs_Transformer overhaul

* Fix license comment

Co-authored-by: Oleksandr <maksymets.o@gmail.com>

* Remove observation transformers

* Fix errors and merge conflicts

* Fix issues, refactor and add tests

* Fix minor bug

* address comments

* Fix test and doc bug

* More docstring improvements

* Fix bug in ResizeShortestEdge

* remove attrs from nn.module

* Fix remaining bugs and refactor Cube2Equirec for smart device selection

* Improve obs_space handling in PPO and change defaults of Cube2EQ

* more doc strings

* bump for ci
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mathfac mathfac mathfac approved these changes

@eundersander eundersander Awaiting requested review from eundersander

@aclegg3 aclegg3 Awaiting requested review from aclegg3

@erikwijmans erikwijmans Awaiting requested review from erikwijmans

Assignees
No one assigned
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Skylion007 @mathfac @erikwijmans @facebook-github-bot