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

Use controls to report if a collision occurred #70

Merged
merged 4 commits into from
Jun 8, 2019

Conversation

erikwijmans
Copy link
Contributor

@erikwijmans erikwijmans commented Jun 7, 2019

Motivation and Context

Our current collision logic is a little buggy :( It also requires knowing if an action can results in a collision (i.e. move_forward can result in a collision but turn_left cannot); this problem will only get worse know that we allow people to define their own actions.

This PR seeks to resolve this by having the controls check if a collision occurred by checking to see if the filtering step caused us to move less than expected.

@msavva, thoughts in the API?

How Has This Been Tested

Visually/with the collision test in the api side.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jun 7, 2019
Copy link
Collaborator

@msavva msavva left a comment

Choose a reason for hiding this comment

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

LGTM! 👍

@@ -150,7 +150,7 @@ def detach(self):
for _, v in self.sensors.items():
v.detach()

def act(self, action_id: Any):
def act(self, action_id: Any) -> bool:
r"""Take the action specified by action_id
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add semantics of return value to docstring (i.e. collision during action)

import habitat_sim.bindings as hsim
from habitat_sim import utils

__all__ = ["ActuationSpec", "SceneNodeControl", "ObjectControls"]


EPS = 1e-5
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add brief comment about semantics of EPS for readability

observations = self.get_sensor_observations()
observations["collided"] = collided
Copy link
Collaborator

Choose a reason for hiding this comment

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

It would be good to document this added field somewhere

@erikwijmans erikwijmans merged commit d383c20 into master Jun 8, 2019
@erikwijmans erikwijmans deleted the better-collision-detection branch June 8, 2019 01:19
Ram81 pushed a commit to Ram81/habitat-web-sim that referenced this pull request Dec 10, 2020
* Have controls report if the control function results in a collision

* Some docs

* Comments/docs
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.

None yet

3 participants