[Bug Fix] change the snap point to be attached the largest island id #1371
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
the
CLA Signed
Skylion007
reviewed
May 23, 2023
habitat-lab/habitat/datasets/rearrange/samplers/object_sampler.py
Outdated
Show resolved
Hide resolved
Skylion007
approved these changes
May 23, 2023
aclegg3
reviewed
May 25, 2023
habitat-lab/habitat/datasets/rearrange/samplers/object_sampler.py
Outdated
Show resolved
Hide resolved
jimmytyyang
force-pushed
the
fix_episode_generation
branch
from
4963e61
to
63d192d
Compare
aclegg3
reviewed
May 26, 2023
habitat-lab/habitat/datasets/rearrange/samplers/object_sampler.py
Outdated
Show resolved
Hide resolved
aclegg3
reviewed
May 30, 2023
There was a problem hiding this comment.
Looks good to me overall.
My only remaining consideration is that the largest island may not always be the correct one. For example if the scene has a large pool/yard then that may be a larger navmesh than the interior.
Possible improvements:
- Make this configurable. Allow users to turn off this check (maybe already done?)
- Check if the navmesh is outdoors (raycast upwards does not hit a ceiling). This won't work for all scenes, but should work for some.
We should have a way to validate this logic for any scenes we want to use. Maybe a script to highlight the largest navmesh island in a topdown view or map.
aclegg3
reviewed
May 30, 2023
|
Hi @aclegg3! I have added the flag for checking the largest island ID. Please let me know if we want to do |
aclegg3
added a commit
that referenced
this pull request
Aug 2, 2023
…tached the largest island id (#1371) * allow the users to set if the snapped point in the largest island id * clean-up, document, pass variable through from config --------- Co-authored-by: Jimmy Yang <jimmytyyang@meta.com> Co-authored-by: aclegg3 <alexanderwclegg@gmail.com>
5 tasks
dannymcy
pushed a commit
to dannymcy/habitat-lab
that referenced
this pull request
Jul 8, 2024
…acebookresearch#1371) * change the snap point to be arrached the largest island id * based on Alex and Aaron suggestion * change it to list format * add back list map trick * cahce * allow the users to set if the snapped point in the largest island id * clean-up, document, pass variable through from config --------- Co-authored-by: Jimmy Yang <jimmytyyang@meta.com> Co-authored-by: aclegg3 <alexanderwclegg@gmail.com>
HHYHRHY
pushed a commit
to SgtVincent/habitat-lab
that referenced
this pull request
Aug 31, 2024
…acebookresearch#1371) * change the snap point to be arrached the largest island id * based on Alex and Aaron suggestion * change it to list format * add back list map trick * cahce * allow the users to set if the snapped point in the largest island id * clean-up, document, pass variable through from config --------- Co-authored-by: Jimmy Yang <jimmytyyang@meta.com> Co-authored-by: aclegg3 <alexanderwclegg@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation and Context
This PR ensures that the snap point of the object is always in the largest island. Previously it was checked by computing the largest island radius, which is not stable.This is the joint work with Alex!
How Has This Been Tested
Run the script, and it generates the episode.Checklist