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

Unoccluded navmesh snap #1591

Merged
merged 9 commits into from
Oct 9, 2023
Merged

Unoccluded navmesh snap #1591

merged 9 commits into from
Oct 9, 2023

Conversation

aclegg3
Copy link
Contributor

@aclegg3 aclegg3 commented Sep 29, 2023

Motivation and Context

TL;DR: This PR introduces a rejection sampling based approach to generate unoccluded snap points.

Context:

Snapping a target point to the navmesh is used in many ways throughout the rearrangement task including:

  • as an accessibility check when generating object placements
  • to generate nav location targets from which and agent should pick an object
  • to check whether an object can be picked from an agent's position

However, standard navmesh snapping can result in a snapped point on the wrong side of obstacles (e.g. on the other side of a wall in another room) especially when snapping clutter object locations which sit off the navmesh on furniture. For example, a bowl on the back of a counter may actually be closer to the navmesh in the room sharing a wall with the counter than the kitchen floor. These error manifest in infeasible object placements and pick/place artifacts which hinder the realism of the task.

New feature:

Adds the unoccluded_navmesh_snap and snap_point_is_occluded utility functions.
The new approach is slower than standard snapping, but still fast enough to be used within the generator loop.

  • snap_point_is_occluded - uses raycasting to check whether a provided point (assumed to be on the floor) has an unoccluded line to the target point. Uses a configurable height and search granularity to match agent constraints and compute limitations.
  • unoccluded_navmesh_snap - uses get_random_navigable_point_near sim API call and rejection sampling with snap_point_is_occluded to find an approximation to the closest unoccluded snap point for a given target position. Relies on sim PR 2221 for accuracy. See test section below for examples of the variance using this approximation.

NOTE:

TODO: This PR currently includes changes in the generator to produce better episodes, but does not refactor rearrange_task or rearrange_sim to use the new functions for snapping.

How Has This Been Tested

Local episode generation and interactive testing application.

Examples of different results returned by unoccluded_navmesh_snap to demonstrate approximation variance. This variance is decreased by increasing max samples and decreasing minimum distance between samples (at the cost of compute):
unoccluded_snap_1

unoccluded_snap_2

Types of changes

  • [Development] A pull request that add new features to the habitat-lab task and environment codebase. Development Pull Requests must be small (less that 500 lines of code change), have unit testing, very extensive documentation and examples. These are typically new tasks, environments, sensors, etc... The review process for these Pull Request is longer because these changes will be maintained by our core team of developers, so make sure your changes are easy to understand!

Checklist

  • My code follows the code style of this project.
  • I have updated the documentation if required.
  • I have read the CONTRIBUTING document.
  • I have completed my CLA (see CONTRIBUTING)
  • I have added tests to cover my changes if required.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Sep 29, 2023
Copy link
Contributor

@jimmytyyang jimmytyyang left a comment

Choose a reason for hiding this comment

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

This is such an awesome design!!! Thank you so much for this, and the code makes sense to me. I just have a few design question and how to use the function. Thank you so much, Alex!

@jimmytyyang jimmytyyang self-requested a review September 29, 2023 19:26
Copy link
Contributor

@jimmytyyang jimmytyyang left a comment

Choose a reason for hiding this comment

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

LGTM! thank you so much for working on this. This looks nice!

@aclegg3 aclegg3 requested a review from jimmytyyang October 3, 2023 23:16
Copy link
Contributor

@jimmytyyang jimmytyyang left a comment

Choose a reason for hiding this comment

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

Thanks for the discussion, Alex! Please let me know what you think. After this fix, I do not think we need to have a threshold thing anymore. Happy to chat :)

ray = habitat_sim.geo.Ray()
ray.origin = snap_point + mn.Vector3(0, cur_height, 0)
cur_height -= granularity
ray.direction = target - ray.origin
Copy link
Contributor

@jimmytyyang jimmytyyang Oct 4, 2023

Choose a reason for hiding this comment

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

Hi Alex,

After a test, I found that the ray pointing to target (i.e., target object) from a snap_point location usually has 0.98 distance value, which is close to one. I think this is because that the target object is usually on the ground/floor, and the first thing it hits could be something else. So, I propose to change this to ray.direction = ray.origin - target. Theoretically, these two are the same thing if there is nothing in between, but this offers much predicted results since we are casting a ray from the target object to the ceiling (open space)

Copy link
Contributor

Choose a reason for hiding this comment

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

Some visual about this proposed change. Red circle is the resulting snap point found
<img width="715" alt="Screenshot 2023-10-04 at 9 25 01 AM" src="https://github.com/facebookr
<img width="573" alt="Screenshot 2023-10-04 at 9 25 52 AM" src="https://github.com/faceb
Screenshot 2023-10-04 at 9 26 04 AM
ookresearch/habitat-lab/assets/55121504/44ddb51d-2597-4c3a-8bf3-c97e9d3a6396">
esearch/habitat-lab/assets/55121504/9e8b4f40-5d4a-48f9-a7cd-329970d539d7">

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 that the ray direction does not matter too much theoretically.

However, there is risk with starting the ray inside the target. We expect the snap_point is in open space (because snap was possible), but we don't know about target. Starting a raycast inside a convex collision shape will miss that shape. This seems like it could introduce errors and I'm not sure what the gain here would be.

Copy link
Contributor

@jimmytyyang jimmytyyang left a comment

Choose a reason for hiding this comment

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

LGTM! thanks alex!!!

@aclegg3 aclegg3 merged commit 8ce0eab into main Oct 9, 2023
@aclegg3 aclegg3 deleted the unoccluded-snap branch October 9, 2023 18:34
dannymcy pushed a commit to dannymcy/habitat-lab that referenced this pull request Jul 8, 2024
* fix typo bug with BalancedSceneSampler

* add unoccluded_navmesh_snap to generate snap points with visibility to a target location. Changes to use unoccluded snap in generator.

* reduce bench threshold
HHYHRHY pushed a commit to SgtVincent/EMOS that referenced this pull request Aug 31, 2024
* fix typo bug with BalancedSceneSampler

* add unoccluded_navmesh_snap to generate snap points with visibility to a target location. Changes to use unoccluded snap in generator.

* reduce bench threshold
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