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

Faster RGB & Depth noise models #548

Merged
merged 5 commits into from
Mar 29, 2020
Merged

Faster RGB & Depth noise models #548

merged 5 commits into from
Mar 29, 2020

Conversation

mathfac
Copy link
Contributor

@mathfac mathfac commented Mar 28, 2020

Motivation and Context

We noticed that RGB noise models are 7x times slower than whole Habitat step call.
@erikwijmans with @Skylion007 made our RGB noise models 50x faster with following PR.

  • One minor change is to make GreedyFollower compatible with PyRobot actions. The change makes it possible to build shortest path on the fly with noise actuations.

How Has This Been Tested

Existing HSIM noise tests, HAPI noise tests.

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)

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Mar 28, 2020
@@ -65,7 +65,7 @@ def __attrs_post_init__(self):
def _find_action(self, name):
candidates = list(
filter(
lambda v: v[1].name == name,
lambda v: name in v[1].name,
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this change necessary?

@@ -65,7 +65,7 @@ def __attrs_post_init__(self):
def _find_action(self, name):
candidates = list(
filter(
lambda v: v[1].name == name,
lambda v: name in v[1].name,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a little dangerous as things are assumed to be deterministic in the follower, but #546 will subsume this, so fine.

@codecov
Copy link

codecov bot commented Mar 28, 2020

Codecov Report

Merging #548 into master will decrease coverage by 0.03%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #548      +/-   ##
==========================================
- Coverage   63.79%   63.75%   -0.04%     
==========================================
  Files         162      162              
  Lines        7880     7878       -2     
  Branches       84       84              
==========================================
- Hits         5027     5023       -4     
- Misses       2853     2855       +2     
Flag Coverage Δ
#CPP 60.39% <ø> (ø)
#JavaScript 10.00% <ø> (ø)
#Python 80.39% <50.00%> (-0.09%) ⬇️
Impacted Files Coverage Δ
...t_sim/sensors/noise_models/gaussian_noise_model.py 93.33% <50.00%> (-6.67%) ⬇️
.../sensors/noise_models/redwood_depth_noise_model.py 61.64% <50.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 bfbe9fc...2ca7635. Read the comment docs.

@mathfac
Copy link
Contributor Author

mathfac commented Mar 28, 2020

Removed greedy_geodesic_follower change.

@mathfac mathfac merged commit e836c89 into master Mar 29, 2020
@mathfac mathfac deleted the faster-noise-models branch March 29, 2020 00:25
Ram81 pushed a commit to Ram81/habitat-web-sim that referenced this pull request Dec 10, 2020
We noticed that RGB noise models are 7x times slower than whole Habitat step call.
@erikwijmans with @Skylion007 made our RGB noise models 50x faster with following PR.

One minor change is to make GreedyFollower compatible with PyRobot actions. The change makes it possible to build shortest path on the fly with noise actuations.
luoj1 pushed a commit to luoj1/habitat-sim that referenced this pull request Aug 16, 2022
* Support properties/members with call_at

* Use named consts
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

4 participants