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

Optimize integrators, Rviz plugin and add usage examples #37

Merged
merged 73 commits into from
Oct 23, 2023

Conversation

victorreijgwart
Copy link
Member

@victorreijgwart victorreijgwart commented Sep 29, 2023

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

  • New feature (non-breaking change which adds functionality)

Detailed summary

New features

  • Wavemap library
    • Add map query acceleration utils
    • Add trilinear interpolation utils
  • ROS server
    • Make ROS logging level configurable through ROS params
    • Make the number of threads to use configurable through ROS params
    • Add option to TF handler to directly query the most up-to-date transform
    • Add service that resets the map (enabled through ROS params, disabled by default)
  • Rviz plugin
    • Add option to load and display maps directly from files
    • Add option to only draw surface voxels
    • Add button to call the wavemap_server's reset map service

Improvements

  • Wavemap library
    • Refactor wavemap utils into an extendable toolbox
    • Optimize measurement integration
      • Replace stack with recursion (faster and easier to read)
      • Vectorize batched leaf updater
      • Reduce memory move and copy overheads
      • Simplify measurement model math
      • Postpone image offset error norm root computation
      • Share a single thread pool among all integrators
  • ROS server (and ROS interface libraries)
    • Update incremental map transmission to communicate block deletions
    • Add option for multi-threaded block to ROS msg serialization
    • Simplify incremental map transmission logic
    • Consistently use ROS logging in all ROS packages
    • Do not latch map topic
    • Improve example configs
  • Rviz plugin
    • General UI improvements
    • Improve block drawing scheduling for faster and smoother rendering
    • Update visualizers to handle deleted blocks
    • Rename "grid" to "voxels" in UI and code for clarity
    • Clean up and optimize visibility query handling
    • Clean up and optimize alpha handling
    • Add Tracy annotations for profiling
  • Tools
    • Show documentation preview without using Python HTTP server

Documentation

  • Add initial usage examples

Bug fixes

  • Wavemap server
    • Fix bug causing delays when transmitting blocks with identical timestamps
  • Rviz plugin
    • Fix bug causing delays when drawing blocks with identical timestamps
    • Fix bug causing segfaults when destroying instances of the Rviz plugin

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

  • Newly added test suites
    • Map to ROS msg conversions
    • Map query accelerator
    • Trilinear interpolator
  • Extended tests
    • Projection model nearest index and offset computation

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:

  • Measurement integration by ~32% on my laptop and ~10% on my desktop.
  • Rviz's frame rate when rendering the map by ~5x on my laptop and 1-2x on my desktop, depending on the map size. On the desktop, the GPU also runs significantly less hot as its compute utilization is reduced from >95% to ~60%.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • Any required changes in dependencies have been committed and pushed

@victorreijgwart victorreijgwart marked this pull request as draft September 29, 2023 09:45
@victorreijgwart victorreijgwart self-assigned this Sep 29, 2023
@victorreijgwart victorreijgwart added the enhancement New feature or request label Sep 29, 2023
@victorreijgwart victorreijgwart force-pushed the feature/optimize_chunked_integrator branch from 9219056 to f4d3d80 Compare September 29, 2023 13:54
@victorreijgwart victorreijgwart force-pushed the feature/optimize_chunked_integrator branch from f4d3d80 to 464fd9c Compare September 29, 2023 14:01
@victorreijgwart victorreijgwart linked an issue Sep 29, 2023 that may be closed by this pull request
@victorreijgwart victorreijgwart marked this pull request as ready for review October 17, 2023 18:44
Copy link
Contributor

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.

Copy link
Member Author

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.

@@ -1,5 +1,5 @@
#ifndef WAVEMAP_UTILS_BIT_MANIPULATION_H_
#define WAVEMAP_UTILS_BIT_MANIPULATION_H_
#ifndef WAVEMAP_UTILS_BITS_BIT_OPERATIONS_H_
Copy link
Contributor

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.

Copy link
Contributor

@LionelOtt LionelOtt left a 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.

@victorreijgwart victorreijgwart merged commit 172836c into main Oct 23, 2023
17 checks passed
@victorreijgwart
Copy link
Member Author

Thanks for reviewing @LionelOtt!

@victorreijgwart victorreijgwart deleted the feature/optimize_chunked_integrator branch October 23, 2023 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Config option to suppress debug output
2 participants