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

Interactive RigidObject python tutorial page #611

Merged
merged 12 commits into from
May 12, 2020
Merged

Conversation

aclegg3
Copy link
Contributor

@aclegg3 aclegg3 commented May 1, 2020

Motivation and Context

Added a much needed tutorial for rigid object interactivity through the python Simulator API.

How Has This Been Tested

New tutorial code is run in tests/test_examples.py.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have completed my CLA (see CONTRIBUTING)
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label May 1, 2020
@codecov
Copy link

codecov bot commented May 1, 2020

Codecov Report

Merging #611 into master will increase coverage by 0.93%.
The diff coverage is 88.54%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #611      +/-   ##
==========================================
+ Coverage   57.76%   58.69%   +0.93%     
==========================================
  Files         135      136       +1     
  Lines        5974     6162     +188     
  Branches       84       84              
==========================================
+ Hits         3451     3617     +166     
- Misses       2523     2545      +22     
Flag Coverage Δ
#CPP 53.40% <ø> (ø)
#JavaScript 10.00% <ø> (ø)
#Python 77.24% <88.54%> (+0.90%) ⬆️
Impacted Files Coverage Δ
habitat_sim/simulator.py 95.93% <75.00%> (-0.32%) ⬇️
examples/tutorials/rigid_object_tutorial.py 88.70% <88.70%> (ø)
habitat_sim/physics.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1bc0350...fff7e33. Read the comment docs.

Copy link
Contributor

@jturner65 jturner65 left a comment

Choose a reason for hiding this comment

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

Looks really good. Should be really helpful for folks.

@dhruvbatra
Copy link
Contributor

dhruvbatra commented May 2, 2020

I am unable to build documentation:

Edit: Nvm. For some reason, bindings weren't being built. Clean build fixed it.

dbatra at dbatra-mbp in ~/projects/habitat-sim/docs on rigid-object-tutorial
$ ./build-public.sh
...
...
Traceback (most recent call last):
  File "./m.css/documentation/python.py", line 2636, in <module>
    module = SourceFileLoader(name, args.conf).load_module()
  File "<frozen importlib._bootstrap_external>", line 399, in _check_name_wrapper
  File "<frozen importlib._bootstrap_external>", line 823, in load_module
  File "<frozen importlib._bootstrap_external>", line 682, in load_module
  File "<frozen importlib._bootstrap>", line 265, in _load_module_shim
  File "<frozen importlib._bootstrap>", line 684, in _load
  File "<frozen importlib._bootstrap>", line 665, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 678, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "conf-public.py", line 9, in <module>
    from conf import *
  File "/Users/dbatra/projects/habitat-sim/docs/conf.py", line 17, in <module>
    import habitat_sim  # NOQA
  File "/Users/dbatra/projects/habitat-sim/docs/../habitat_sim/__init__.py", line 16, in <module>
    from habitat_sim.simulator import *
  File "/Users/dbatra/projects/habitat-sim/docs/../habitat_sim/simulator.py", line 21, in <module>
    from habitat_sim.physics import MotionType
  File "/Users/dbatra/projects/habitat-sim/docs/../habitat_sim/physics.py", line 5, in <module>
    from habitat_sim._ext.habitat_sim_bindings import MotionType, VelocityControl
ImportError: cannot import name 'VelocityControl'

Copy link
Contributor

@dhruvbatra dhruvbatra left a comment

Choose a reason for hiding this comment

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

Looks great overall! Kudos! Will be quite valuable to our users.

docs/pages/rigid-object-tutorial.rst Outdated Show resolved Hide resolved
docs/pages/rigid-object-tutorial.rst Show resolved Hide resolved
docs/pages/rigid-object-tutorial.rst Show resolved Hide resolved
docs/pages/rigid-object-tutorial.rst Show resolved Hide resolved
docs/pages/rigid-object-tutorial.rst Show resolved Hide resolved
@aclegg3 aclegg3 requested a review from dhruvbatra May 12, 2020 00:29
@@ -298,6 +298,9 @@ jobs:
wget http://dl.fbaipublicfiles.com/habitat/objects_v0.1.zip
unzip objects_v0.1.zip -d data/objects/
rm objects_v0.1.zip
wget http://dl.fbaipublicfiles.com/habitat/locobot_merged.zip
unzip locobot_merged.zip -d data/objects/
rm locobot_merged.zip
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand the workflow now after reading this. I had initially unzipped locobot_merged into the data/locobot_merged folder and was surprised when the example script crashed. (I do now see that in your instructions, you do ask them to be unzipped into data/objects/)

Instead of dumping everything into a single objects folder, would it be better to unzip locobot_merged.zip into data/locobot_merged so that way each asset-dataset is contained in it's own folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, in this case it may be best to add another directory like we have for scenes (data/scene_datasets/) and do data/object_datasets/objects/ and data/object_datasets/locobot_merged/.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changing this would break some existing user workflows I think. Let's talk about data organization and make a separate PR to update things to clarify the breaking nature of this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good.

docs/pages/rigid-object-tutorial.rst Show resolved Hide resolved
@dhruvbatra
Copy link
Contributor

Reviewing this PR was like unwrapping a basket of gifts. So many goodies.

@aclegg3 aclegg3 merged commit 1c8497c into master May 12, 2020
@aclegg3 aclegg3 deleted the rigid-object-tutorial branch May 12, 2020 18:58
dhruvbatra pushed a commit that referenced this pull request May 13, 2020
* master:
  Interactive RigidObject python tutorial page (#611)
  Update README.md
  Separate Scene And Object functionality into different classes (#628)
  Updated m.css submodule. (#627)
  Update agent state set state to be less confusing (#614)
  Direct Navmesh/Meshdata handling; PluginManager instance variable (#623)
  Documentation generator updates (#624)
  Update corrade submodule. (#618)
  Physics primitives (non-colliding) (#622)
  save_nav_mesh python bindings (#619)
  --Renamed GltfMeshData -> GenericMeshData; (#617)
  Update README.md
  Update README.md
Ram81 pushed a commit to Ram81/habitat-web-sim that referenced this pull request Dec 10, 2020
* Added rigid object tutorial to Habitat-sim python pages.

* update .circleci/config.yml to download locobot_merged asset for CI tutorial test
Ram81 pushed a commit to Ram81/habitat-web-sim that referenced this pull request Dec 10, 2020
* master:
  Interactive RigidObject python tutorial page (facebookresearch#611)
  Update README.md
  Separate Scene And Object functionality into different classes (facebookresearch#628)
  Updated m.css submodule. (facebookresearch#627)
  Update agent state set state to be less confusing (facebookresearch#614)
  Direct Navmesh/Meshdata handling; PluginManager instance variable (facebookresearch#623)
  Documentation generator updates (facebookresearch#624)
  Update corrade submodule. (facebookresearch#618)
  Physics primitives (non-colliding) (facebookresearch#622)
  save_nav_mesh python bindings (facebookresearch#619)
  --Renamed GltfMeshData -> GenericMeshData; (facebookresearch#617)
  Update README.md
  Update README.md
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