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

"on_floor" query util #1841

Merged
merged 5 commits into from
Mar 13, 2024
Merged

"on_floor" query util #1841

merged 5 commits into from
Mar 13, 2024

Conversation

aclegg3
Copy link
Contributor

@aclegg3 aclegg3 commented Mar 9, 2024

Motivation and Context

This PR adds the on_floor utility function for heuristically checking whether an object is "on the floor".

Context: we want to know if objects are on the navigable area of the floor for rearrangement. This function checks that the heuristic point-to-surface distance between the object and its nearest navmesh snap point is within an error threshold.

Supporting utilities:

  • get_bb_for_object_id - a generic utility for getting the local bounding box and transform of any object or link from its object_id integer.
  • get_obj_size_along - a function to approximate the object's size in a particular global direction from its bounding box size. Uses aabb scale to transform the vector, essentially finding a point on the best-fit ellipsoid within the aabb.
  • size_regularized_distance - a function to get estimated surface-to-surface distance between two objects or links using the get_obj_size_along function to truncate the center-to-center distance between the objects.

Also includes a few minor refactors to other utilities.

How Has This Been Tested

Added a set of pytests.

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 Mar 9, 2024
Copy link
Contributor

@jacobkrantz jacobkrantz left a comment

Choose a reason for hiding this comment

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

Thanks, Alex! Left some comments/questions.

object_id_b: int,
ao_link_map: Dict[int, int] = None,
ao_aabbs: Dict[int, mn.Range3D] = None,
) -> float:
Copy link
Contributor

Choose a reason for hiding this comment

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

This function isn't used in on_floor. Do we have a use case for it? Why introduce it with on_floor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we could wait to merge this with "nearby". That is were it really belongs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, this is already cleaned and tested. I'm going to merge it now given that nearby will be added soon. Less work in the long-run. Would be different if not tested.


assert isinstance(
object_a, habitat_sim.physics.ManagedRigidObject
), "Object must be ManagedRigidObject, not implemented for ArticulatedObjects or links."
Copy link
Contributor

Choose a reason for hiding this comment

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

Just checking -- do we have use cases for AO on_floor?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think so because we don't support AO "pick-able" objects presently. I currently assume all AOs are furniture or robot/human.

:param sim: The Simulator instance.
:param obj_id: The integer id of the object or link.
:param ao_link_map: A pre-computed map from link object ids to their parent ArticulatedObject's object id.
:param ao_aabbs: A pre-computed map from ArticulatedObject object_ids to their local bounding boxes. If not provided, recomputed as necessary.
Copy link
Contributor

Choose a reason for hiding this comment

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

for my understanding, this is bounding box for a closed articulated object?

Copy link
Contributor Author

@aclegg3 aclegg3 Mar 13, 2024

Choose a reason for hiding this comment

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

This is a map of all AO bounding boxes for the scene. It includes the state of the links when computed.
Typically, if pre-computed at initialization it will be for closed AOs (unless non-default AO states are specified in the scene instance config).
If it is computed in this function or re-computed during simulation, it "could" include the open state of the receptacles if they are currently open.

Copy link
Contributor

@akshararai akshararai left a comment

Choose a reason for hiding this comment

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

looks good overall.
Just a clarification: the floor and the navmesh are the same in this abstraction? If yes, does this affect how we initialize objects on floor? Would they be initialized anywhere on the navmesh, which might include things like unsegmented furniture?

@aclegg3
Copy link
Contributor Author

aclegg3 commented Mar 13, 2024

Just a clarification: the floor and the navmesh are the same in this abstraction? If yes, does this affect how we initialize objects on floor? Would they be initialized anywhere on the navmesh, which might include things like unsegmented furniture?

Correct, the navmesh is our model of the valid floor space here.

It is true that an inaccurate navmesh could result in false positives. Like you say, using the entire navmesh could, for example, allow bed or sofa tops to be considered floor in some scenes.

There are controls available to prevent this. Primarily the island_index variable. We already assume in many other parts of the RearrangeTask/RearrangeSim/RearrangeGenerator code that we restrict the navmesh to a single island (the largest indoor island). This should also be used here to provide the best results.

@aclegg3 aclegg3 merged commit 30740ac into main Mar 13, 2024
1 of 3 checks passed
@aclegg3 aclegg3 deleted the alex-03_08-on_floor_util branch March 13, 2024 21:04
dannymcy pushed a commit to dannymcy/habitat-lab that referenced this pull request Jul 8, 2024
* add get_bb_for_object_id function

* add get_object_size_along and minor doc adjustments

* add size_regularized_distance function

* add on_floor function and tests
HHYHRHY pushed a commit to SgtVincent/habitat-lab that referenced this pull request Aug 31, 2024
* add get_bb_for_object_id function

* add get_object_size_along and minor doc adjustments

* add size_regularized_distance function

* add on_floor function and tests
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.

4 participants