From 6b20571fe08811e8f000bc48d023297ee1f901b6 Mon Sep 17 00:00:00 2001 From: Erwan MATHIEU Date: Fri, 22 Nov 2024 09:06:30 +0100 Subject: [PATCH] Fix wrong scoring tendancy CURA-12324 When processing the last criterion pass, we used to keep all the best candidates in regard to the scoring threshold, then the first one of them would be returned, which is not always the one with the actual best score. Now we just early-out with the best candidate in case of the last pass. This is also more efficient. --- include/utils/scoring/BestElementFinder.h | 3 ++- src/utils/scoring/BestElementFinder.cpp | 29 ++++++++++++++++++----- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/include/utils/scoring/BestElementFinder.h b/include/utils/scoring/BestElementFinder.h index e2ede975d5..07daf055d2 100644 --- a/include/utils/scoring/BestElementFinder.h +++ b/include/utils/scoring/BestElementFinder.h @@ -15,7 +15,7 @@ class ScoringCriterion; /*! * This class implements an algorithm to find an element amongst a list, regarding one or multiple scoring criteria. The - * criteria are implemented by subclassing the CriterionScoring class. It is also possible to setup multiple passes of + * criteria are implemented by subclassing the CriterionScoring class. It is also possible to set up multiple passes of * criteria. Thus, if the first pass gives a few best candidates that are too close to each other, we keep only the best * candidates and process a second pass with different criteria, and so on until we have a single outsider, or no more * criteria. @@ -60,6 +60,7 @@ class BestElementFinder * Once we have calculated the global scores of each element for this pass, we calculate the score difference * between the best candidate and the following ones. If the following ones have a score close enough to the * best, within this threshold, they will also be considered outsiders and will be run for the next pass. + * This value will be ignored for the last pass. */ double outsider_delta_threshold{ 0.0 }; }; diff --git a/src/utils/scoring/BestElementFinder.cpp b/src/utils/scoring/BestElementFinder.cpp index ecd63e89cb..1b116f62b7 100644 --- a/src/utils/scoring/BestElementFinder.cpp +++ b/src/utils/scoring/BestElementFinder.cpp @@ -5,6 +5,8 @@ #include +#include + #include "utils/scoring/ScoringCriterion.h" namespace cura @@ -33,11 +35,11 @@ std::optional cura::BestElementFinder::findBestElement(const size_t cand const auto begin = best_candidates.begin(); auto end = best_candidates.end(); - // Now run the criteria passes until we have a single outsider or no more cirteria - for (const CriteriaPass& criteria_pass : criteria_) + // Now run the criteria passes until we have a single outsider or no more criteria + for (const auto& [pass_index, criteria_pass] : criteria_ | ranges::views::enumerate) { // For each element, reset score, process each criterion and apply weights to get the global score - double best_score = 0.0; + auto best_candidate_iterator = end; for (auto iterator = begin; iterator != end; ++iterator) { iterator->score = 0.0; @@ -47,7 +49,22 @@ std::optional cura::BestElementFinder::findBestElement(const size_t cand iterator->score += weighed_criterion.criterion->computeScore(iterator->candidate_index) * weighed_criterion.weight; } - best_score = std::max(best_score, iterator->score); + if (best_candidate_iterator == end || iterator->score > best_candidate_iterator->score) + { + best_candidate_iterator = iterator; + } + } + + if (best_candidate_iterator == end) + { + // Something got wrong, we have not best candidate + return std::nullopt; + } + + // Early out for last pass, just keep the best candidate + if (pass_index == criteria_.size() - 1) + { + return best_candidate_iterator->candidate_index; } // Skip candidates that have a score too far from the actual best one @@ -55,9 +72,9 @@ std::optional cura::BestElementFinder::findBestElement(const size_t cand end = std::remove_if( begin, end, - [&best_score, &delta_threshold](const Candidate& candidate) + [&best_candidate_iterator, &delta_threshold](const Candidate& candidate) { - return best_score - candidate.score > delta_threshold; + return best_candidate_iterator->score - candidate.score > delta_threshold; }); if (std::distance(begin, end) == 1)