Skip to content

Add localization quality estimation metric - #563

Open
papalmac wants to merge 7 commits into
Ekumen-OS:mainfrom
papalmac:patricio/quality_degradation_metric
Open

Add localization quality estimation metric#563
papalmac wants to merge 7 commits into
Ekumen-OS:mainfrom
papalmac:patricio/quality_degradation_metric

Conversation

@papalmac

@papalmac papalmac commented May 20, 2026

Copy link
Copy Markdown

Proposed changes

Implements a quality/degradation metric based in a comparison between a expected covariance and the real covariance. It is based in the answer in #562.
A new publisher "localization_quality" provides a float between [0, 1] , the closer to 1 better quality.

Type of change

  • 🐛 Bugfix (change which fixes an issue)
  • 🚀 Feature (change which adds functionality)
  • 📚 Documentation (change which fixes or extends documentation)

Checklist

Put an x in the boxes that apply. This is simply a reminder of what we will require before merging your code.

  • Lint and unit tests (if any) pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • All commits have been signed for DCO

Additional comments

It is my first PR. Need some help to test this and to add a test to check the feature.

Patricio Palma added 2 commits May 20, 2026 11:28
Parameters for expected standard deviation x, y and yaw were added.
Added compute_quality function.

Signed-off-by: Patricio Palma <patricio.palma@creativa77.com.ar>
"localization_quality" publisher added.
Parameter descriptors for expected stddev in x, y, and yaw have been added.

Signed-off-by: Patricio Palma <patricio.palma@creativa77.com.ar>
@papalmac
papalmac force-pushed the patricio/quality_degradation_metric branch from c6cec0a to 6c839dc Compare May 20, 2026 15:29
Patricio Palma added 2 commits May 20, 2026 15:59
Signed-off-by: Patricio Palma <patricio.palma@ekumenlabs.com>
Signed-off-by: Patricio Palma <patricio.palma@ekumenlabs.com>
@hidmic
hidmic requested review from glpuga and hidmic May 26, 2026 15:06

@hidmic hidmic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@papalmac mind to update the PR title to a more descriptive name?

Comment thread beluga_ros/src/amcl.cpp Outdated
return estimate;
}

double Amcl::compute_quality(const Sophus::Matrix3d& actual_covariance) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@papalmac meta, maybe followup: I think it may be worth to move this into the core, taking the nominal distribution, the motion model, and the last available estimate.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This will also help unit testing this. That's step 1. Then we'll need to do some benchmarking and/or testing in the field. @agalbachicar would you be open to collaborate on that?

Comment thread beluga_ros/src/amcl.cpp Outdated
Comment thread beluga_ros/src/amcl.cpp Outdated
for (int i = 0; i < 3; ++i) {
const double actual = actual_covariance.coeff(i, i);
if (actual > std::numeric_limits<double>::epsilon()) {
quality = std::min(quality, ref_covariance.coeff(i, i) / actual);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@papalmac meta: this ratio bugs me a bit. A very small actual covariance would be indicative of distribution collapse, not a high quality estimate. I'd expect the covariance to stay in the vicinity of expectations. CC'ing @glpuga as the mastermind behind this heuristic.

@papalmac papalmac changed the title Patricio/quality degradation metric Add localization quality estimation metric May 26, 2026
Generate reference states on Amcl class construction.
Fix parameter value range.

Signed-off-by: Patricio Palma <patricio.palma@ekumenlabs.com>
Comment thread beluga_ros/src/amcl.cpp Outdated
Comment on lines +56 to +60
std::mt19937 gen{42};
ref_states_.reserve(params_.min_particles);
for (std::size_t i = 0; i < params_.min_particles; ++i) {
ref_states_.emplace_back(dist(gen));
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not sure I would use min_particles here, especially since we are reusing systematically the same set, which means we will amplify any initial bias.

Comment thread beluga_ros/src/amcl.cpp Outdated
for (int i = 0; i < 3; ++i) {
const double actual = actual_covariance.coeff(i, i);
if (actual > std::numeric_limits<double>::epsilon()) {
quality = std::min(quality, ref_covariance.coeff(i, i) / actual);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It's a bit distorsive than the quotient is made in terms of the squared values, since that will amplify any difference in the standard deviations.

Comment thread beluga_ros/src/amcl.cpp Outdated
for (int i = 0; i < 3; ++i) {
const double actual = actual_covariance.coeff(i, i);
if (actual > std::numeric_limits<double>::epsilon()) {
quality = std::min(quality, ref_covariance.coeff(i, i) / actual);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

having a single index for all three dimensions clouds poor choices for a particular dimension. I think all three should be outputs.

@glpuga glpuga Jun 1, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Also, calculating the min over the three with no averaging is very biasing, since you only need one of the values to take a tail end value of the distribution to make a huge difference in the results.

@papalmac papalmac Jun 1, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Comment addressed in 9c8d158

Patricio Palma added 2 commits June 1, 2026 16:07
… yaw.

Signed-off-by: Patricio Palma <patricio.palma@ekumenlabs.com>
Documentation for input parameters now states "typical stddev" for x, y and yaw.

Signed-off-by: Patricio Palma <patricio.palma@ekumenlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants