-
Notifications
You must be signed in to change notification settings - Fork 557
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
Using std types instead of boost for Gaussian sampling #2351
Conversation
a06fcf5
to
05f930f
Compare
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2351 +/- ##
==========================================
- Coverage 50.83% 50.34% -0.49%
==========================================
Files 391 390 -1
Lines 32125 31954 -171
==========================================
- Hits 16328 16084 -244
- Misses 15797 15870 +73 ☔ View full report in Codecov by Sentry. |
05f930f
to
ef37f68
Compare
@henningkayser I suppose using the <tr1/random> header does not read as standard library? |
963511f
to
01da506
Compare
I am directly calling the dist_(eng_) for updating output matrix, becasue that is what |
756df20
to
d9a89f3
Compare
68df427
to
85e4d96
Compare
85e4d96
to
e243823
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@henningkayser Not sure if the rng initialized with the default value has the desired effect.
moveit_planners/stomp/include/stomp_moveit/math/multivariate_gaussian.hpp
Outdated
Show resolved
Hide resolved
e243823
to
2658e4b
Compare
4f8c1b8
to
3c738b0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry @Shobuj-Paul I just discover an even simpler way to do this. We had a PR ~1 year ago which unified the use of random number generators across the repository(see the changes for the CHOMP planner as references for STOMP) by using a unified implementation to create a random number generator . It is more performant since creating a random device is computationally expensive. Can you test my suggestions? All that's left to do would be to add <depend>rsl</depend>
to the CMakeLists.txt. If that works this PR is finally good to go 😅
moveit_planners/stomp/include/stomp_moveit/math/multivariate_gaussian.hpp
Outdated
Show resolved
Hide resolved
moveit_planners/stomp/include/stomp_moveit/math/multivariate_gaussian.hpp
Outdated
Show resolved
Hide resolved
moveit_planners/stomp/include/stomp_moveit/math/multivariate_gaussian.hpp
Outdated
Show resolved
Hide resolved
moveit_planners/stomp/include/stomp_moveit/math/multivariate_gaussian.hpp
Outdated
Show resolved
Hide resolved
*Need to compare function implementations *Find an equivalent implementation for variate_generator
3c738b0
to
1cbb59e
Compare
Description
Getting rid of boost for sampling from a gaussian distribution and instead using standard library functions, wherever available.
Fixes #2166
Checklist