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

45% performance improvement in MPPI #4174

Merged
merged 20 commits into from
Mar 27, 2024
Merged

45% performance improvement in MPPI #4174

merged 20 commits into from
Mar 27, 2024

Conversation

SteveMacenski
Copy link
Member

@SteveMacenski SteveMacenski commented Mar 8, 2024

From 12.7ms on average to 6.80ms on average for 10000 cycles - with full footprint checking enabled


Validate final run-time and system performance of changes to make sure still high functioning with changes

  • Cost - OK avoiding obstacles and dynamic obstacles (@BriceRenaudeau ?)
  • Path Align - OK aligning effectively with the path and alongside other critics
  • Path Angle - OK checking only end in a broad set of situations?
  • High level Behavior - OK (retune?)

@ros-navigation ros-navigation deleted a comment from mergify bot Mar 9, 2024
@SteveMacenski SteveMacenski changed the title 18.5% performance improvement in MPPI 19.1% performance improvement in MPPI Mar 12, 2024
@ros-navigation ros-navigation deleted a comment from mergify bot Mar 12, 2024
@ros-navigation ros-navigation deleted a comment from mergify bot Mar 12, 2024
@SteveMacenski SteveMacenski changed the title 19.1% performance improvement in MPPI 22% performance improvement in MPPI Mar 12, 2024
@ros-navigation ros-navigation deleted a comment from mergify bot Mar 12, 2024
@SteveMacenski SteveMacenski changed the title 22% performance improvement in MPPI 30% performance improvement in MPPI Mar 13, 2024
@SteveMacenski SteveMacenski changed the title 30% performance improvement in MPPI 30%+ performance improvement in MPPI Mar 14, 2024
@SteveMacenski SteveMacenski changed the title 30%+ performance improvement in MPPI 45% performance improvement in MPPI Mar 15, 2024
Copy link

@jjd9 jjd9 left a comment

Choose a reason for hiding this comment

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

Hello, I did not find any glaring issues, but I found some small things I thought were worth pointing out. Thanks!

Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
@ros-navigation ros-navigation deleted a comment from mergify bot Mar 23, 2024
@ros-navigation ros-navigation deleted a comment from mergify bot Mar 23, 2024
@ros-navigation ros-navigation deleted a comment from mergify bot Mar 23, 2024
@ros-navigation ros-navigation deleted a comment from mergify bot Mar 23, 2024
@ros-navigation ros-navigation deleted a comment from mergify bot Mar 23, 2024
@ros-navigation ros-navigation deleted a comment from mergify bot Mar 23, 2024
@ros-navigation ros-navigation deleted a comment from mergify bot Mar 23, 2024
@ros-navigation ros-navigation deleted a comment from mergify bot Mar 23, 2024
@ros-navigation ros-navigation deleted a comment from mergify bot Mar 23, 2024
@ros-navigation ros-navigation deleted a comment from mergify bot Mar 23, 2024
@ros-navigation ros-navigation deleted a comment from mergify bot Mar 23, 2024
@aosmw
Copy link
Contributor

aosmw commented Mar 25, 2024

Hi Steve,

Do you know about this "pitfall"? https://xtensor.readthedocs.io/en/latest/pitfall.html#random-numbers-not-consistent
Using a random number function from xtensor actually returns a lazy generator. That means, accessing the same element of a random number generator does not give the same random number if called twice.

Is there a speed advantage if the xt::random::randn operations in NoiseGenerator::generateNoisedControls are wrapped in xt::eval?

regards
Mikew

@SteveMacenski
Copy link
Member Author

SteveMacenski commented Mar 25, 2024

That's a good question. I can look into that.

Edit: That is not relevent since we set to noises_vx_ (and analog) which is an xtensor object. That evaluates on construction, but just to verify I added eval and it made no difference. It actually made me wonder if this would be faster if we did use lazy evaluation, so I moved some of the sampling into the main function so that we could take advantage of lazy evaluation and it made statistically no meaningful improvement (but seems slightly worse, but within error bounds. It mostly added more jitter, which I think is why we originally did it this way if my foggy memory serves).

However that attribute actually calls something interesting into question. Do we need all 3 noise distributions if we lazy evaluate them? if calling multiple times isn't the same value, we could use a lazy expression to create 1 noise sample and then use it for all 3 axes. If we hard evaluate it, then we can't do that since the same noises would be used, but if we used the lazy evaulation, that could make an impact.

That doesn't actually speed things up for us unfortunately since we're still re-evaluating 3x56x2000 samples no matter how we cut it in memory (and with the added problem of no longer having control over the std per axis).

Overall, I still don't see a way to optimize this further unforunately - which is of annoyance to me as well. I feel like this shouldn't be as heavy as it is.

Copy link
Contributor

mergify bot commented Mar 25, 2024

@SteveMacenski, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

Copy link
Contributor

mergify bot commented Mar 25, 2024

@SteveMacenski, your PR has failed to build. Please check CI outputs and resolve issues.
You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

@SteveMacenski SteveMacenski merged commit 5a8cbd3 into main Mar 27, 2024
6 of 9 checks passed
@SteveMacenski SteveMacenski deleted the mppi_opt branch March 27, 2024 18:24
ashwinvkNV pushed a commit to ashwinvkNV/navigation2 that referenced this pull request Apr 3, 2024
* 18.5% performance improvement in MPPI

* adding in path angle update

* path align improvements

* adding views

* more updates with some TODOs

* massive improvements to cost critic

* remove TODOs

* removing some jitter

* updates

* use fabs

* 1ms saved baby!

* completed optimizations

* remove TODO

* linting

* fixing test failure

* indexing fix

* fix bug

* make MPPI default for Jazzy

* Adding higher velocity limits

---------

Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
(cherry picked from commit 5a8cbd3)
ajtudela pushed a commit to grupo-avispa/navigation2 that referenced this pull request Apr 19, 2024
* 18.5% performance improvement in MPPI

* adding in path angle update

* path align improvements

* adding views

* more updates with some TODOs

* massive improvements to cost critic

* remove TODOs

* removing some jitter

* updates

* use fabs

* 1ms saved baby!

* completed optimizations

* remove TODO

* linting

* fixing test failure

* indexing fix

* fix bug

* make MPPI default for Jazzy

* Adding higher velocity limits

---------

Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
enricosutera pushed a commit to enricosutera/navigation2 that referenced this pull request May 19, 2024
* 18.5% performance improvement in MPPI

* adding in path angle update

* path align improvements

* adding views

* more updates with some TODOs

* massive improvements to cost critic

* remove TODOs

* removing some jitter

* updates

* use fabs

* 1ms saved baby!

* completed optimizations

* remove TODO

* linting

* fixing test failure

* indexing fix

* fix bug

* make MPPI default for Jazzy

* Adding higher velocity limits

---------

Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
Signed-off-by: enricosutera <enricosutera@outlook.com>
Marc-Morcos pushed a commit to Marc-Morcos/navigation2 that referenced this pull request Jul 4, 2024
* 18.5% performance improvement in MPPI

* adding in path angle update

* path align improvements

* adding views

* more updates with some TODOs

* massive improvements to cost critic

* remove TODOs

* removing some jitter

* updates

* use fabs

* 1ms saved baby!

* completed optimizations

* remove TODO

* linting

* fixing test failure

* indexing fix

* fix bug

* make MPPI default for Jazzy

* Adding higher velocity limits

---------

Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
Marc-Morcos pushed a commit to Marc-Morcos/navigation2 that referenced this pull request Jul 4, 2024
* 18.5% performance improvement in MPPI

* adding in path angle update

* path align improvements

* adding views

* more updates with some TODOs

* massive improvements to cost critic

* remove TODOs

* removing some jitter

* updates

* use fabs

* 1ms saved baby!

* completed optimizations

* remove TODO

* linting

* fixing test failure

* indexing fix

* fix bug

* make MPPI default for Jazzy

* Adding higher velocity limits

---------

Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
savalena pushed a commit to savalena/navigation2 that referenced this pull request Jul 5, 2024
* 18.5% performance improvement in MPPI

* adding in path angle update

* path align improvements

* adding views

* more updates with some TODOs

* massive improvements to cost critic

* remove TODOs

* removing some jitter

* updates

* use fabs

* 1ms saved baby!

* completed optimizations

* remove TODO

* linting

* fixing test failure

* indexing fix

* fix bug

* make MPPI default for Jazzy

* Adding higher velocity limits

---------

Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
Manos-G pushed a commit to Manos-G/navigation2 that referenced this pull request Aug 1, 2024
* 18.5% performance improvement in MPPI

* adding in path angle update

* path align improvements

* adding views

* more updates with some TODOs

* massive improvements to cost critic

* remove TODOs

* removing some jitter

* updates

* use fabs

* 1ms saved baby!

* completed optimizations

* remove TODO

* linting

* fixing test failure

* indexing fix

* fix bug

* make MPPI default for Jazzy

* Adding higher velocity limits

---------

Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
masf7g pushed a commit to quasi-robotics/navigation2 that referenced this pull request Oct 22, 2024
* 18.5% performance improvement in MPPI

* adding in path angle update

* path align improvements

* adding views

* more updates with some TODOs

* massive improvements to cost critic

* remove TODOs

* removing some jitter

* updates

* use fabs

* 1ms saved baby!

* completed optimizations

* remove TODO

* linting

* fixing test failure

* indexing fix

* fix bug

* make MPPI default for Jazzy

* Adding higher velocity limits

---------

Signed-off-by: Steve Macenski <stevenmacenski@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants