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

Add script to run benchmarks #117

Merged
merged 35 commits into from
Feb 28, 2023
Merged

Add script to run benchmarks #117

merged 35 commits into from
Feb 28, 2023

Conversation

ivanpauno
Copy link
Collaborator

@ivanpauno ivanpauno commented Feb 13, 2023

Related to #35.

Summary

Adds a script that allows running a rosbag repeatedly, configuring amcl with different number of particles.
It records another bagfile with the result, the results of timem, and optionally perf events data.

The missing thing is the postprocessing steps.

qq: does the current rosbag provide a groundtruth topic?
We will need that to use something a tool like evo.

Checklist

  • Read the contributing guidelines.
  • Configured pre-commit and ran colcon test locally.
  • Signed all commits for DCO.
  • Added tests (regression tests for bugs, coverage of new code for features).
  • Updated documentation (as needed).
  • Checked that CI is passing.

@ivanpauno ivanpauno added the enhancement New feature or request label Feb 13, 2023
@ivanpauno ivanpauno self-assigned this Feb 13, 2023
Copy link
Member

@nahueespinosa nahueespinosa left a comment

Choose a reason for hiding this comment

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

@ivanpauno Left some comments!

does the current rosbag provide a groundtruth topic?

I didn't record an extra topic for ground truth since odom-to-base_link is the ground truth in the perfect_odometry bagfile, but +1 to having ground truth in bagfiles. Note that for this case, in evo you can compare odom-to-base_link and map-to-base_link.

scripts/benchmarking/parameterized_run.sh Outdated Show resolved Hide resolved
scripts/benchmarking/parameterized_run.sh Outdated Show resolved Hide resolved
scripts/benchmarking/parameterized_run.sh Outdated Show resolved Hide resolved
@ivanpauno
Copy link
Collaborator Author

in the perfect_odometry bagfile

Lol, I didn't pay attention to the name of the bag haha.
Ok, that's enough to get evo to work.

@ivanpauno
Copy link
Collaborator Author

Note that for this case, in evo you can compare odom-to-base_link and map-to-base_link.

I tried to make some progress on this.
The problem is that evo doesn't support tf topic for ROS 2, it only does for ROS 1.
I tried to convert the bag using rosbags which doesn't require ROS 1 installed.
The problem is that evo does when using the rosbag reader.

It seems that the easy solution is to use a bag with a ground truth topic (or check how to add tf support to evo).

@olmerg
Copy link
Collaborator

olmerg commented Feb 17, 2023

maybe we can use the Odometry message ground_thruth_topic which generate the flatland plugin to obtain the ground truth, which is suuported by evo

@ivanpauno
Copy link
Collaborator Author

maybe we can use the Odometry message ground_thruth_topic which generate the flatland plugin to obtain the ground truth, which is suuported by evo

Yes, I think that's the easier thing to do, I will record a new bag that includes the odometry topic.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno ivanpauno force-pushed the ivanpauno/script-to-benchmark branch from 1183461 to 7343ed3 Compare February 23, 2023 19:49
@ivanpauno
Copy link
Collaborator Author

Results:
benchmark.zip

The folder benchmark was compiled in Release mode.
The folder benchmark with profiling was compiled using RelWithDebInfo and -fno-omit-frame-pointer.

I'm failing to generate results for nav2_amcl, for some reason it's segfaulting now ......

To see the timem results, use:

/ws/src/scripts/benchmarking/timem_metrics_results /path/to/folder/with/timem/json/output

To get ape metrics:

evo_ape bag2 benchmark_1000_particles_output/rosbag/ /odometry/ground_truth /pose

@ivanpauno
Copy link
Collaborator Author

The issue I'm running into is ros-navigation/navigation2#3311.
I think it was fixed for rolling (see here), but it seems it wasn't backported to humble.

@ivanpauno
Copy link
Collaborator Author

ivanpauno commented Feb 23, 2023

I was finally able to record the same cases for nav2 (it doesn't seem to fail if commenting out the recovery_alpha_fast/recovery_alpha_slow, so I did that).

benchmark_nav2.zip

(edit: uploaded new zip, as I forgot to record the /amcl_pose topic)

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@gmail.com>
@ivanpauno
Copy link
Collaborator Author

I run both again with some changes:

  • beluga
    • Used covariance 0 for the initial pose, i.e. all initial particles in the same state.
      That's what nav2 does if you use the parameters.
  • amcl
    • Changed recovery_alpha_fast/recovery_alpha_slow to the default, i.e. disabled.
      It avoids the crash mentioned above. I cannot disable this in beluga, but I think it shouldn't affect much the comparison.
    • Run tests again without using google meet at the same time 😄

benchmark.zip

I'm working in an script to easily compare both results

* Add script to plot results comparision.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@gmail.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@gmail.com>
@hidmic
Copy link
Collaborator

hidmic commented Feb 27, 2023

I don't think it's useful for the comparison

Not for a comparison, but I wonder why the APE maximum is consistently higher for beluga. Is it a transient that is quickly corrected? Is it a larger variance in the distribution (ie. beluga being less consistent in its performance than nav2_amcl)?

@ivanpauno
Copy link
Collaborator Author

Not for a comparison, but I wonder why the APE maximum is consistently higher for beluga. Is it a transient that is quickly corrected? Is it a larger variance in the distribution (ie. beluga being less consistent in its performance than nav2_amcl)?

Yes, I guessed it was that.
I will probably add that in another PR to limit the scope of this one, but I think it's a good idea to have that available as well.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
* Remove .sh extension from executable scripts.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@ivanpauno
Copy link
Collaborator Author

I will probably add that in another PR to limit the scope of this one, but I think it's a good idea to have that available as well.

I see that's already provided by evo, see 1a50027.


The maximum APE error seems to happen just before the bagfile ends (i.e. before shutdown).

image

It happens pretty consistently for beluga, and not for nav2_amcl.
It doesn't seem to be something to worry much about ...

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Copy link
Member

@nahueespinosa nahueespinosa left a comment

Choose a reason for hiding this comment

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

@ivanpauno I have only minor comments.

beluga_benchmark/CMakeLists.txt Outdated Show resolved Hide resolved
beluga_benchmark/docs/BENCHMARKING.md Outdated Show resolved Hide resolved
beluga_benchmark/docs/BENCHMARKING.md Outdated Show resolved Hide resolved
beluga_benchmark/docs/BENCHMARKING.md Outdated Show resolved Hide resolved
beluga_benchmark/docs/BENCHMARKING.md Outdated Show resolved Hide resolved
beluga_benchmark/docs/BENCHMARKING.md Outdated Show resolved Hide resolved
docker/images/humble/Dockerfile Outdated Show resolved Hide resolved
@nahueespinosa nahueespinosa added the python Related to Python code label Feb 28, 2023
ivanpauno and others added 7 commits February 28, 2023 15:13
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Co-authored-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Co-authored-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Co-authored-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Co-authored-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Co-authored-by: Nahuel Espinosa <nespinosa@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
Copy link
Member

@nahueespinosa nahueespinosa left a comment

Choose a reason for hiding this comment

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

@ivanpauno LGTM!

@ivanpauno ivanpauno merged commit f93602e into main Feb 28, 2023
@ivanpauno ivanpauno deleted the ivanpauno/script-to-benchmark branch February 28, 2023 21:26
olmerg pushed a commit that referenced this pull request Mar 2, 2023
Related to #35.

Adds a script that allows running a rosbag repeatedly, configuring amcl
with different number of particles.
It records another bagfile with the result, the results of timem, and
optionally perf events data.

It also adds postprocessing scripts, and instructions of how to run them.

Signed-off-by: Ivan Santiago Paunovic <ivanpauno@ekumenlabs.com>
@nahueespinosa nahueespinosa mentioned this pull request Jul 10, 2023
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request python Related to Python code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants