Alter interface for registration robst linear estimator#2700
Merged
Conversation
Manual duplication of 9f3d624.
Merged
Lestropie
force-pushed
the
registration_linear_estimator
branch
from
259b6d7 to
93de5a2
Compare
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| 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}; |
There was a problem hiding this comment.
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};
^|
|
||
| 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}; |
There was a problem hiding this comment.
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};
^
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_templatethat struck me as odd (seemingly one option wasn't present in the list of choices, and reported default differed frommrregister'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 intodev?