Add the noise model from PyRobot #89

erikwijmans · 2019-06-27T02:04:40Z

Motivation and Context

Our friends at https://github.com/facebookresearch/pyrobot have "benchmarked" the noise of two real robots, LoCoBot and LoCoBot-Lite. So now we can add that noise model to habitat!

How Has This Been Tested

Via unit tests and with a little cv2 based viewer so I can step around :)

Types of changes

New feature (non-breaking change which adds functionality)

dhruvbatra · 2019-06-27T02:07:45Z

CC: @kalyanvasudev @s-gupta @devendrachaplot

mathfac

That's amazing how implementation of Python specified actions for the agents makes easier to add functionality as noise models. Code looks clean to me, in your tests do you want to check that the actual actions have noise specified with performing same action.

mathfac · 2019-06-28T23:32:59Z

habitat_sim/agent/pyrobot_noisy_controls.py

+        return getattr(self, key)
+
+
+noise_models = {


Let's add link to PyRobot paper to reference source of noise parameters.

dhruvbatra · 2019-06-29T03:17:42Z

Can we add a gif to demonstrate the noisy actuation?

erikwijmans · 2019-06-29T20:17:15Z

@dhruvbatra

erikwijmans · 2019-06-29T20:22:37Z

@dhruvbatra @mathfac @s-gupta @kalyanvasudev Thoughts on the current way we are pointing people to PyRobot so they can include the proper citation if they use the noise model?

dhruvbatra · 2019-07-01T04:07:09Z

README.md

+* PyRobot Noise Model -- [PyRobot](https://github.com/facebookresearch/pyrobot#citation)
+
+    Contributes the noise model used for the noisy control functions named `pyrobot_*` and defined in `habitat_sim/agent/pyrobot_noisy_controls.py`
+


Can we restructure this bit?

"If you use the PyRobot noise models, please cite:
blah

Noisy control function named pyrobot_* defined in blah.

kalyanvasudev · 2019-07-01T16:38:35Z

Hi Everyone, this all looks great! I have a quick suggestion, would it be possible to incorporate a noise model for state-estimation as well? Because there is some error in it as well on the real robot.

erikwijmans · 2019-07-01T18:09:37Z

Hi Everyone, this all looks great! I have a quick suggestion, would it be possible to incorporate a noise model for state-estimation as well? Because there is some error in it as well on the real robot.

I'm not entirely certain what you are referring to, but I assume this is the robot estimating it's pose relative to some initial pose? We currently have a noise-free GPS/Compass sensor, and we'd add noise to that? Can you point me to the noise models/parameters I should use for that?

mathfac · 2019-07-01T19:05:21Z

@erikwijmans, thank you for the gif of noisy navigation, looks great. Maybe, worth to add top down map with sampling same action from the same position and show distribution of final positions with opacity.

dhruvbatra · 2019-07-01T20:57:05Z

Maybe, worth to add top down map with sampling same action from the same position and show distribution of final positions with opacity.

Yes! Like Fig 4 here:
http://saurabhg.web.illinois.edu/pdfs/kumar2018visual.pdf

erikwijmans · 2019-07-01T23:17:01Z

@mathfac @dhruvbatra

dhruvbatra · 2019-07-01T23:22:05Z

Woah, so pretty!

How many trajectories is this showing? 100? 500?

erikwijmans · 2019-07-01T23:24:09Z

100

msavva

This is awesome! The flexibility of the actuation specification API is clearly demonstrated here!

Did a quick pass and all LGTM! 👍

… pyrobot-noisy-actions

erikwijmans · 2019-07-10T18:29:40Z

Merged this with master. I also put the control definitions in their own folder, habitat_sim/agent/controls as the agent folder was starting to feel too big. I also fixed one issue with the imports and now habitat_sim.register_move_fn works.

erikwijmans · 2019-07-10T18:30:45Z

habitat_sim/agent/controls/pyrobot_noisy_controls.py

+        np.sign(rotate_amount + 1e-8) * model.rotation.mean, model.rotation.cov
+    )
+
+    scene_node.rotate_local(mn.Deg(rotate_amount) + mn.Rad(rot_noise[0]), hsim.geo.UP)


@mosra -- Strongly typed angles are great! I can now add degrees and radians without fear :)

Hah, didn't even know this was possible (it isn't in C++, there it can't decide on what the result type should be). Nice to see it "just works" in Python.

abhiskk · 2019-07-15T15:03:02Z

habitat_sim/agent/controls/controls.py

@@ -15,7 +15,7 @@
 import habitat_sim.bindings as hsim
 from habitat_sim import utils

-__all__ = ["ActuationSpec", "SceneNodeControl", "ObjectControls"]
+__all__ = ["ActuationSpec", "SceneNodeControl", "ObjectControls", "register_move_fn"]


Ideally we should move the definition of __all__ to __init__.py

abhiskk · 2019-07-15T15:03:23Z

habitat_sim/agent/controls/pyrobot_noisy_controls.py

+
+from .controls import ActuationSpec, SceneNodeControl, register_move_fn
+
+__all__ = ["PyRobotNoisyActuationSpec"]


Same comment as above.

abhiskk · 2019-07-15T15:03:42Z

habitat_sim/agent/controls/pyrobot_noisy_controls.py

+import habitat_sim.bindings as hsim
+from habitat_sim import utils
+
+from .controls import ActuationSpec, SceneNodeControl, register_move_fn


NIT: if possible we should move to absolute imports instead of relative imports.

abhiskk · 2019-07-15T15:04:20Z

habitat_sim/agent/controls/pyrobot_noisy_controls.py

+
+
+@attr.s(auto_attribs=True)
+class MotionNoiseModel:


A base NoiseModel class could be useful here.

abhiskk · 2019-07-15T15:05:07Z

habitat_sim/agent/controls/pyrobot_noisy_controls.py

+    "LoCoBot": RobotNoiseModel(
+        ILQR=ControllerNoiseModel(
+            linear_motion=MotionNoiseModel(
+                _MultivariateGaussian([0.014, 0.009], [0.006, 0.005]),


Abstracting out the constants into a different class would make this code more readable.

Maybe stick this in a different file? Any class that has all these constants will be as unreadable as this class by definition (all these classes are doing is holding these constants).

abhiskk · 2019-07-15T15:05:45Z

habitat_sim/agent/controls/pyrobot_noisy_controls.py

+    noise_multiplier: float = 1.0
+
+
+_x_axis = 0


Make these constants X_AXIS etc.

… pyrobot-noisy-actions

facebookresearch#89) * Modify partitioning logic to handle left-over scenes due to integer division

* Adds noisy actuations based on the noise model from PyRobot

erikwijmans added 3 commits June 24, 2019 17:31

Pyrobot

0a1f271

Should be good!

d1bcf0a

Add sentence asking people to cite pyrobot

7638a31

erikwijmans requested review from msavva, mathfac, bigbike and kalyanvasudev June 27, 2019 02:04

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

mathfac approved these changes Jun 28, 2019

View reviewed changes

mathfac reviewed Jun 28, 2019

View reviewed changes

Add info about cites

ed94dc5

erikwijmans added 2 commits June 29, 2019 14:12

Better tests

4f25e14

Need to multiply the mean by the sign!

0313022

dhruvbatra reviewed Jul 1, 2019

View reviewed changes

PyRobot

5d44536

msavva approved these changes Jul 2, 2019

View reviewed changes

erikwijmans added 2 commits July 10, 2019 11:10

Merge branch 'master' of github.com:facebookresearch/habitat-sim into…

826aa54

… pyrobot-noisy-actions

Update to work on master

0d99bde

erikwijmans commented Jul 10, 2019

View reviewed changes

erikwijmans added 3 commits July 10, 2019 11:32

Remove unneeded code

3b702ac

[ci skip] fix readme

08b69c5

Faster implementation

da75909

abhiskk suggested changes Jul 15, 2019

View reviewed changes

erikwijmans added 3 commits July 15, 2019 10:39

Nits

0fbd246

Add __all__

3484798

Merge branch 'master' of github.com:facebookresearch/habitat-sim into…

4a9e959

… pyrobot-noisy-actions

erikwijmans requested a review from abhiskk July 17, 2019 00:12

erikwijmans added 6 commits July 17, 2019 12:25

Fix translate coordinates

772f363

Merge branch 'master' of github.com:facebookresearch/habitat-sim into…

e7db6ba

… pyrobot-noisy-actions

Force grid wrap doesn't work with black

df13ea8

Fix

452e122

Add option back

542b163

Merge branch 'master' of github.com:facebookresearch/habitat-sim into…

637d767

… pyrobot-noisy-actions

erikwijmans merged commit 15994e4 into master Jul 22, 2019

erikwijmans deleted the pyrobot-noisy-actions branch July 22, 2019 20:59

mathfac added this to the Realistic noise models milestone Sep 5, 2019

eundersander pushed a commit to eundersander/habitat-sim that referenced this pull request Aug 6, 2020

Modify partitioning logic to handle left-over scenes due to integer d… (

b68d38c

facebookresearch#89) * Modify partitioning logic to handle left-over scenes due to integer division

Ram81 pushed a commit to Ram81/habitat-web-sim that referenced this pull request Dec 10, 2020

Add the noise model from PyRobot (facebookresearch#89)

85f5457

* Adds noisy actuations based on the noise model from PyRobot

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the noise model from PyRobot #89

Add the noise model from PyRobot #89

erikwijmans commented Jun 27, 2019 •

edited

Loading

dhruvbatra commented Jun 27, 2019 •

edited

Loading

mathfac left a comment

mathfac Jun 28, 2019

dhruvbatra commented Jun 29, 2019

erikwijmans commented Jun 29, 2019 •

edited

Loading

erikwijmans commented Jun 29, 2019

dhruvbatra Jul 1, 2019

erikwijmans Jul 1, 2019

kalyanvasudev commented Jul 1, 2019

erikwijmans commented Jul 1, 2019

mathfac commented Jul 1, 2019

dhruvbatra commented Jul 1, 2019

erikwijmans commented Jul 1, 2019

dhruvbatra commented Jul 1, 2019 •

edited

Loading

erikwijmans commented Jul 1, 2019

msavva left a comment

erikwijmans commented Jul 10, 2019

erikwijmans Jul 10, 2019

mosra Jul 18, 2019

abhiskk Jul 15, 2019

abhiskk Jul 15, 2019

abhiskk Jul 15, 2019

abhiskk Jul 15, 2019

abhiskk Jul 15, 2019

erikwijmans Jul 15, 2019

abhiskk Jul 15, 2019

		* PyRobot Noise Model -- [PyRobot](https://github.com/facebookresearch/pyrobot#citation)

		Contributes the noise model used for the noisy control functions named `pyrobot_*` and defined in `habitat_sim/agent/pyrobot_noisy_controls.py`


		from .controls import ActuationSpec, SceneNodeControl, register_move_fn

		__all__ = ["PyRobotNoisyActuationSpec"]

Add the noise model from PyRobot #89

Add the noise model from PyRobot #89

Conversation

erikwijmans commented Jun 27, 2019 • edited Loading

Motivation and Context

How Has This Been Tested

Types of changes

dhruvbatra commented Jun 27, 2019 • edited Loading

mathfac left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhruvbatra commented Jun 29, 2019

erikwijmans commented Jun 29, 2019 • edited Loading

erikwijmans commented Jun 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kalyanvasudev commented Jul 1, 2019

erikwijmans commented Jul 1, 2019

mathfac commented Jul 1, 2019

dhruvbatra commented Jul 1, 2019

erikwijmans commented Jul 1, 2019

dhruvbatra commented Jul 1, 2019 • edited Loading

erikwijmans commented Jul 1, 2019

msavva left a comment

Choose a reason for hiding this comment

erikwijmans commented Jul 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erikwijmans commented Jun 27, 2019 •

edited

Loading

dhruvbatra commented Jun 27, 2019 •

edited

Loading

erikwijmans commented Jun 29, 2019 •

edited

Loading

dhruvbatra commented Jul 1, 2019 •

edited

Loading