-
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
Report resource usage #83
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
victorreijgwart
added
documentation
Improvements or additions to documentation
enhancement
New feature or request
labels
Nov 19, 2024
LionelOtt
reviewed
Nov 19, 2024
LionelOtt
reviewed
Nov 19, 2024
LionelOtt
reviewed
Nov 19, 2024
LionelOtt
reviewed
Nov 19, 2024
Looks good overall to me, think once the code to prevent it causing compile errors on non-linux systems is in we're good. Maybe adding the "summary generator" could be nice for this PR as well. |
victorreijgwart
commented
Nov 20, 2024
victorreijgwart
commented
Nov 20, 2024
LionelOtt
approved these changes
Nov 20, 2024
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 with the latest modifications.
13 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR introduces a
ResourceMonitor
class to measure resource usage (RAM, CPU time, wall time). Additionally, it updates the repository's pull request template to standardize the inclusion of performance and accuracy benchmarking results and make it more descriptive.Type of change
Detailed Summary
The primary motivation for this PR is to begin tracking performance to avoid regressions and monitor the evolution of the framework over time. While the repository's tests and CI pipelines test correctness, they do not account for efficiency changes. Additionally, while the tests ensure that wavemap's different map types and measurement integrators are lossless with respect to each other, they do not assess the overall end-to-end accuracy of our reconstruction pipelines.
To address this gap, we have set up a dedicated benchmarking server and extended the evaluation code from our RSS paper. This will allow us to benchmark different wavemap releases against each other and attach updated performance and accuracy results to future PRs.
On a practical level, the code changes in this PR:
ResourceMonitor
class for measuring CPU time, wall time, and RAM usage during macro-benchmarking.wavemap_ros
, to make it easier for users to run their own benchmarks.API Changes
List any changes to wavemap's APIs to help users update their code. Write "None" if there are no changes.
C++ API:
ResourceMonitor
class inlibrary/cpp/include/wavemap/core/utils/profile/resource_usage_monitor.h
wavemap/core/utils/profiler_interface.h
moved towavemap/core/utils/profile/profiler_interface.h
Python API:
ROS1 Interface:
Review Notes
We are particularly interested in feedback on the new pull request template.
Testing
Automated Tests
Tests have been added for the
ResourceMonitor
, and extended for theStopwatch
andThreadPool
classes.Manual Tests
No manual tests were run for this PR, other than the benchmarks below.
Benchmarks
We conduct benchmarks on a dedicated server with the following specifications:
The server is liquid-cooled, and its CPU power profile is set to
performance
. To ensure accurate results, no other jobs are active during benchmarks, and we allow the server to cool down between runs.For a complete description of the evaluation procedure and metrics, please refer to the RSS paper. We will briefly summarize them in the following.
The following datasets are used:
We compare the performance of
In each test, all frameworks are configured with identical voxel sizes, perception ranges, and calibrations. To ensure fairness, we also use the same data loader for all frameworks.
Performance
The following metrics are compared:
Note that dividing the CPU time by the wall time yields the CPU utilization, which can be higher than 100% on multi-core systems.
Newer College 20cm
Newer College 5cm
Panoptic Flat 5cm
Panoptic Flat 2cm
Accuracy over distance
The graph below shows how accuracy varies with the distance to the surface. A high accuracy just behind the surface (small negative distances) corresponds to a high recall on thin objects. A smaller dip at the surface (distance 0.0) indicates better classification performance in challenging areas, improving safety in near-surface operations.
Checklist
General
Documentation (where applicable)