feat(extrinsic_reflector_based_calibrator): add metric plotter for cross validation and add deletion button

[vividf]

Description

This PR adds three new features to the radar-lidar calibrator.

1. Add an additional cross-validation metric for evaluation.
2. Add a delete button on the UI for the user to delete the previous radar-lidar pair.
3. Add a new plotter to visualize the metrics in real time.

   Related links

Tests performed

Notes for reviewers

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

• [x] I've confirmed the contribution guidelines.
• [x] The PR follows the pull request guidelines.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

• [x] The PR follows the pull request guidelines.
• [x] The PR has been properly tested.
• [x] The PR has been reviewed by the code owners.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

• [x] There are no open discussions or they are tracked via tickets.
• [ ] The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

[knzo25]

Thanks for the PR :tada: I will be checking this PR around next week, but in the meantime can you check that the CI/CD passes? (galactic is not needed)

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 183 lines in your changes missing coverage. Please review.

Project coverage is 0.00%. Comparing base (21a1158) to head (61ef8f7). Report is 44 commits behind head on tier4/universe.

Files with missing lines	Patch %	Lines
...rator/src/extrinsic_reflector_based_calibrator.cpp	0.00%	183 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## tier4/universe #145 +/- ## ================================================= - Coverage 0.95% 0.00% -0.96% ================================================= Files 269 6 -263 Lines 20905 952 -19953 Branches 383 0 -383 ================================================= - Hits 200 0 -200 + Misses 20548 952 -19596 + Partials 157 0 -157 ``` | [Flag](https://app.codecov.io/gh/tier4/CalibrationTools/pull/145/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tier4) | Coverage Δ | | |---|---|---| | [differential](https://app.codecov.io/gh/tier4/CalibrationTools/pull/145/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tier4) | `0.00% <0.00%> (?)` | | | [total](https://app.codecov.io/gh/tier4/CalibrationTools/pull/145/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tier4) | `?` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tier4#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[knzo25]

Just in case no one told you before. The spellchecker currently fails in the following words:

Warning: Unknown word (forground)
sensor/extrinsic_reflector_based_calibrator/rviz/x2_front_center.rviz:129:19 Unknown word (forground)
sensor/extrinsic_reflector_based_calibrator/scripts/metrics_plotter_node.py:27:44 Unknown word (nrows)
sensor/extrinsic_reflector_based_calibrator/scripts/metrics_plotter_node.py:27:[53](https://github.com/tier4/CalibrationTools/actions/runs/7044433176/job/19172099781?pr=145#step:3:59) Unknown word (ncols)
sensor/extrinsic_reflector_based_calibrator/scripts/metrics_plotter_node.py:86:16 Unknown word (annos)
sensor/extrinsic_reflector_based_calibrator/scripts/metrics_plotter_node.py:227:21 Unknown word (annos)
sensor/extrinsic_reflector_based_calibrator/src/extrinsic_reflector_based_calibrator.cpp:1661:7 Unknown word (ncrossval)
sensor/extrinsic_reflector_based_calibrator/src/extrinsic_reflector_based_calibrator.cpp:1662:7 Unknown word (ncrossval)

Putting aside the one that are misspells, abbreviations are marked as unknown by our current dictionary. For example nrows -> number_of_rows or rows In the case we are using external libraries, we can use tags in the code to disable the spell checker in that line In the case we are actually using conventions that are better to keep as is, we can add the words in the dictionary: https://github.com/tier4/CalibrationTools/blob/tier4/universe/.cspell.json

[knzo25]

The CalibrationTools s full of misspells, which are going to be fixed in our big next task. They were not caught at the proper time since the dictionary was added after the code was written

[knzo25]

@vividf General comments first: The plot works as expected but there are several small details I would like to be addressed. 1) At the beginning, the matplotlib window covers all the screen, I would like it to have a small initial resolution so that it does not bother the user. 2) At the beginning, the number of reflectors are not enforced to be integers. Please add that condition 3) Although the tool is called reflector-based, it was to make it clear that it is different that our other tag based tools. Please call it marker in the UI 4) Please add a suitable name to the figure instead of "Figure 1" 5) The distance units in ROS is meters. However, for evaluation purposes, it is more convenient to have it in centimeters in the UI. We report the error in centimeters. In the same way, the value ad the end of the plot is a good idea, but when converted to cm, please fix the decimals to the milimeter. Our sensors don't go beyond that anyway. 5) Avoid abbreviations when possible to make it as clear as possible:

• "# reflector" -> "number of reflectors"
• "Cross val error: D..." -> "Cross-validation error: d" 6) As you can see in the attached image, the axis of the upper image appears in the same position that the title of the lower plot. Please make sure that that does not happen (this may be resolution dependent though)

[knzo25]

[knzo25]

About the lifetime of markers. The text marker should have infinite lifetime. The other day I told you that the error of the converged tracks markers not disappearing was due to its infinite lifetime. However, though giving the marker a finite lifetime is not the solution. In this case what you need to do is send a deletion command via the marker topic. Example marker.action = visualization_msgs::Marker::DELETE;

[vividf]

@knzo25 Thanks for the detailed review. I fixed those issues with the newest commit. Please review again when you have free time :) Thanks!

[knzo25]

@vividf pre-commit is not passing. Are you using it locally?

[knzo25]

@vividf Are using pre-commit locally? ci/cd keeps complaining it seems

[vividf]

Yes, I run it. But seems that I forget to commit it 😅

[knzo25]

@vividf What do you mean you forget to commit? If you installed it correctly, it should not allow you to commit things that do not pass its tests

[vividf]

I commit my code but didn't push. And I remember to run precommit. So I run the precommit run -a. There are some error in other code but not mine and it autofix my code (just like what GitHub does) but after that I forget to commit the changes and I push.😅

[knzo25]

@vividf I don't know how everyone else is doing it by if you execute pre-commit install, then every git commit -s you execute afterwards will automatically run pre-commit so there are no chances your commit will violate the linters and stuff

[vividf]

I see. I was taught by another way. Thank you! I will run the -s in the future.

[knzo25]

@vividf The -s is for another reason completely. Even if you run git commit the pre-commit will still run. -s is for signing your commits (I think David mentioned something to you along the lines the other day)

[knzo25]

(signing the commits is an absoluute rule in other Tier IV repositories)

[vividf]

Understand! I am sorry😓 and thanks for reminding me again.

[knzo25]

No biggie :+1: Firsts PRs in a new organization always come with lots of things to learn