-
Notifications
You must be signed in to change notification settings - Fork 42
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
Optimize integrators, Rviz plugin and add usage examples #37
Conversation
9219056
to
f4d3d80
Compare
f4d3d80
to
464fd9c
Compare
docs/pages/usage_examples.rst
Outdated
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.
For a future revision it might be good to have some explanations of what the code does in addition to the code itself.
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.
Yes you're right. I ran out of steam when writing this 🙃 Do you think the explanations would help a lot? If so I can add them now. Otherwise in the next release.
...ries/wavemap/include/wavemap/data_structure/volumetric/hashed_chunked_wavelet_octree_block.h
Outdated
Show resolved
Hide resolved
libraries/wavemap/include/wavemap/integrator/measurement_model/impl/continuous_beam_inl.h
Outdated
Show resolved
Hide resolved
libraries/wavemap/include/wavemap/integrator/measurement_model/impl/continuous_ray_inl.h
Outdated
Show resolved
Hide resolved
libraries/wavemap/include/wavemap/integrator/projection_model/impl/ouster_projector_inl.h
Show resolved
Hide resolved
@@ -1,5 +1,5 @@ | |||
#ifndef WAVEMAP_UTILS_BIT_MANIPULATION_H_ | |||
#define WAVEMAP_UTILS_BIT_MANIPULATION_H_ | |||
#ifndef WAVEMAP_UTILS_BITS_BIT_OPERATIONS_H_ |
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.
Technically #pragma once is fine to use despite being non-standard as all sane compilers support it nowadays.
libraries/wavemap/test/src/integrator/test_range_image_intersector.cc
Outdated
Show resolved
Hide resolved
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.
Looks good to me.
Thanks for reviewing @LionelOtt! |
Description
This PR significantly improves the performance of wavemap's measurement integrators and the Rviz plugin. It also introduces several new features, such as utility functions for accelerated map queries and trilinear interpolation in the wavemap library, the option to set the logging verbosity through ROS params in wavemap's ROS server, and options to load and display maps directly from files in the Rviz plugin. Finally, the documentation has been extended with code usage examples.
Type of change
Detailed summary
New features
Improvements
Documentation
Bug fixes
The code optimizations were mostly performed by analyzing the assembly code of the most runtime critical functions, as identified through frame and sampling-based profiling.
Testing
Correctness
Performance
All the changes were validated on an AMD laptop (Ryzen 7 PRO 6850U CPU with integrated graphics) and an Intel desktop (i9-9900K CPU and Nvidia RTX 2080 Ti GPU). The changes speed up:
Checklist: