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

Fixes for the HRL Trainer #1209

Merged
merged 39 commits into from
Mar 28, 2023
Merged

Fixes for the HRL Trainer #1209

merged 39 commits into from
Mar 28, 2023

Conversation

ASzot
Copy link
Contributor

@ASzot ASzot commented Mar 20, 2023

Motivation and Context

Bug fixes for the HRL trainer to properly work

How Has This Been Tested

Training the HL policy on the rearrange easy task.

Types of changes

  • [Bug Fix] (non-breaking change which fixes an issue)

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Mar 20, 2023
@ASzot ASzot marked this pull request as ready for review March 21, 2023 21:56
Copy link
Contributor

@vincentpierre vincentpierre left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments. Please address especially on the habitat-lab side.

Comment on lines 445 to 447
island_radius = sim.pathfinder.island_radius(start_position)
if island_radius != targ_island_radius:
continue
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why we are checking this is not obvious.

Suggested change
island_radius = sim.pathfinder.island_radius(start_position)
if island_radius != targ_island_radius:
continue
island_radius = sim.pathfinder.island_radius(start_position)
position_on_largest_island = island_radius == targ_island_radius
if not position_on_largest_island:
continue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, let me delay this change for a later PR to get the right solution.

habitat-lab/habitat/tasks/rearrange/utils.py Outdated Show resolved Hide resolved
habitat-lab/habitat/tasks/rearrange/rearrange_sim.py Outdated Show resolved Hide resolved
habitat-lab/habitat/tasks/rearrange/utils.py Show resolved Hide resolved
@ASzot ASzot mentioned this pull request Mar 28, 2023
@ASzot ASzot merged commit 5272e6d into facebookresearch:main Mar 28, 2023
dannymcy pushed a commit to dannymcy/habitat-lab that referenced this pull request Jul 8, 2024
* Speed fix

* Added config overwrite

* Updated speed fixes

* Merged

* Fixed PDDL

* Bug Fix For Interactive Play Script (facebookresearch#1155)

* Fixed interactive play

* removing f string

---------

Co-authored-by: vincentpierre <vincentpierre@users.noreply.github.com>

* More PDDL fixes

* Precommit

* Updated docs

* Cleaned up PDDL

* Precommit

* Fixed tests

* Fixed PDDL base problem

* Renamed PDDL types

* CI test fix

* PDDL Fixes

* Handle repeat IDs

* Fixed grasp criteria

* Removed unnecessary code

* Added rel idx fix

* Proper rewards

* Cleaned up sim

* Removed unnecessary

* Only PDDL for interactive play

* Added some type check fixes

* Addressing PR

* Pddl fixes

* precommit

* Abstraction methods

* Fixed problem with open/close tasks

* Fixed tests

---------

Co-authored-by: vincentpierre <vincentpierre@users.noreply.github.com>
HHYHRHY pushed a commit to SgtVincent/EMOS that referenced this pull request Aug 31, 2024
* Speed fix

* Added config overwrite

* Updated speed fixes

* Merged

* Fixed PDDL

* Bug Fix For Interactive Play Script (facebookresearch#1155)

* Fixed interactive play

* removing f string

---------

Co-authored-by: vincentpierre <vincentpierre@users.noreply.github.com>

* More PDDL fixes

* Precommit

* Updated docs

* Cleaned up PDDL

* Precommit

* Fixed tests

* Fixed PDDL base problem

* Renamed PDDL types

* CI test fix

* PDDL Fixes

* Handle repeat IDs

* Fixed grasp criteria

* Removed unnecessary code

* Added rel idx fix

* Proper rewards

* Cleaned up sim

* Removed unnecessary

* Only PDDL for interactive play

* Added some type check fixes

* Addressing PR

* Pddl fixes

* precommit

* Abstraction methods

* Fixed problem with open/close tasks

* Fixed tests

---------

Co-authored-by: vincentpierre <vincentpierre@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants