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

FIX: Robust loss error calculation #1161

Merged
merged 3 commits into from
Jul 1, 2022
Merged

FIX: Robust loss error calculation #1161

merged 3 commits into from
Jul 1, 2022

Conversation

ProfFan
Copy link
Collaborator

@ProfFan ProfFan commented Apr 8, 2022

This fixes #1129

@ProfFan ProfFan requested a review from yetongumich April 8, 2022 01:34
@ProfFan
Copy link
Collaborator Author

ProfFan commented Apr 8, 2022

@yetongumich

TEST(NoiseModel, robustNoiseL2WithDeadZone)
{
  double dead_zone_size = 1.0;
  SharedNoiseModel robust = noiseModel::Robust::Create(
      noiseModel::mEstimator::L2WithDeadZone::Create(dead_zone_size),
      Unit::Create(3));

  for (int i = 0; i < 5; i++) {
    Vector3 error = Vector3(i, 0, 0);
    DOUBLES_EQUAL(std::fmax(0, i - dead_zone_size) * i,
                  robust->squaredMahalanobisDistance(error), 1e-8);
  }
}

is failing

@dellaert
Copy link
Member

dellaert commented Apr 9, 2022

CI fails

@dellaert
Copy link
Member

@ProfFan CI stil fails :-)

@dellaert
Copy link
Member

@ProfFan CI fails. Also, would like a better PR comment.

@varunagrawal
Copy link
Collaborator

@ProfFan @yetongumich can we land this soon? @raabuchanan and I need to use robust noise models for a project we're working on and I would like to see the kinks worked out. :)

@ProfFan
Copy link
Collaborator Author

ProfFan commented Jun 29, 2022

@varunagrawal I worked on this for quite a long time trying to figure out why the unit test fails but ran out of time before internship, could you also give an eye on it? I can look at it tomorrow.

@yetongumich
Copy link
Contributor

@varunagrawal I worked on this for quite a long time trying to figure out why the unit test fails but ran out of time before internship, could you also give an eye on it? I can look at it tomorrow.

I already figured out why the test failed, maybe we can discuss about how to fix it tomorrow?

@yetongumich
Copy link
Contributor

Updated the previous failing unit test.

The previous test is actually testing the WhitenSystem function for robust noise, which was indirectly called through squaredMahalanobisDistance function.

According to the new factorization, WhitenSystem is no longer called for robust noisemodel, therefore this test shall explicitly call WhitenSystem function to get the correct results.

@ProfFan
Copy link
Collaborator Author

ProfFan commented Jul 1, 2022

Thanks @yetongumich ! I'll merge this @varunagrawal

@ProfFan ProfFan merged commit caf659f into develop Jul 1, 2022
@ProfFan ProfFan deleted the fix/robust_loss_factor branch July 1, 2022 04:44
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.

mEstimator Error with Trust Region Optimizers
4 participants