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

Alter interface for registration robst linear estimator #2700

Merged
merged 3 commits into from
Feb 28, 2024

Conversation

Lestropie
Copy link
Member

This is the commencement of addressing #2586.

The lone commit is a manual replication of the effects of 9f3d624, which is erroneously in the history of #2678; I'm happy for that commit to stay there, but I'd rather resolve this issue explicitly in its own PR before merging #2678.

Prompted to get the issue resolved due to potential relevance in #2696.


I most definitely don't currently have sufficient understanding of the relevant code to understand exactly what's going on. I'm crossing my fingers that @maxpietsch might be able to provide some insight. All I recall is that in the process of doing #2678, I saw some stuff in population_template that struck me as odd (seemingly one option wasn't present in the list of choices, and reported default differed from mrregister's reported default), and that prompted me to post #2586. If need be, I may need to wrap my head around this. That may include 9f3d624, which from memory was a partial merge of a feature branch into dev?

@Lestropie Lestropie self-assigned this Aug 30, 2023
@Lestropie Lestropie mentioned this pull request Jan 28, 2024
@Lestropie Lestropie added this to the 3.1.0 updates milestone Feb 21, 2024
@Lestropie Lestropie marked this pull request as ready for review February 21, 2024 01:36
@Lestropie Lestropie requested a review from a team February 21, 2024 01:37
Lestropie added a commit that referenced this pull request Feb 21, 2024
Resolves changes proposed in e073be5 (#2700) against introduction of clang-format (#2652).
Resolves changes proposed in e073be5 (#2700) against introduction of clang-format (#2652).
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

clang-tidy made some suggestions

@@ -25,7 +25,7 @@ const char *initialisation_translation_choices[] = {"mass", "geometric", "none",
const char *initialisation_rotation_choices[] = {"search", "moments", "none", nullptr};

const char *linear_metric_choices[] = {"diff", "ncc", nullptr};
const char *linear_robust_estimator_choices[] = {"l1", "l2", "lp", nullptr};
const char *linear_robust_estimator_choices[] = {"l1", "l2", "lp", "none", nullptr};

Choose a reason for hiding this comment

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

warning: do not declare C-style arrays, use std::array<> instead [cppcoreguidelines-avoid-c-arrays]

const char *linear_robust_estimator_choices[] = {"l1", "l2", "lp", "none", nullptr};
      ^

@@ -25,7 +25,7 @@
const char *initialisation_rotation_choices[] = {"search", "moments", "none", nullptr};

const char *linear_metric_choices[] = {"diff", "ncc", nullptr};
const char *linear_robust_estimator_choices[] = {"l1", "l2", "lp", nullptr};
const char *linear_robust_estimator_choices[] = {"l1", "l2", "lp", "none", nullptr};

Choose a reason for hiding this comment

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

warning: variable 'linear_robust_estimator_choices' is non-const and globally accessible, consider making it const [cppcoreguidelines-avoid-non-const-global-variables]

const char *linear_robust_estimator_choices[] = {"l1", "l2", "lp", "none", nullptr};
            ^

@Lestropie Lestropie merged commit 4df4015 into dev Feb 28, 2024
6 checks passed
@Lestropie Lestropie deleted the registration_linear_estimator branch February 28, 2024 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant