From 4939943e0e00ffad0bac04473f24b21913bbae55 Mon Sep 17 00:00:00 2001 From: Riccardo Milani Date: Tue, 9 Apr 2024 18:16:55 +0200 Subject: [PATCH] fix(aniso)!: Fix bugs in lines computations Both conceptual and implementation bugs. I don't know how that could work (using weights instead of indices and vice versa) --- include/CoMMA/Agglomerator.h | 73 +++++++++++++++++++++++------------- tests/test_anisoagglo.cpp | 14 ++----- 2 files changed, 50 insertions(+), 37 deletions(-) diff --git a/include/CoMMA/Agglomerator.h b/include/CoMMA/Agglomerator.h index 56442d8..f6e12a0 100644 --- a/include/CoMMA/Agglomerator.h +++ b/include/CoMMA/Agglomerator.h @@ -586,8 +586,6 @@ class Agglomerator_Anisotropic : // Flag to determine if we arrived at the end of an extreme of a line bool opposite_direction_check = false; // Info about growth direction - std::vector prev_cen = - this->_fc_graph->_centers[seed]; // OK copy std::vector prev_dir(pts_dim); bool empty_line = true; // Start the check from the seed @@ -595,10 +593,6 @@ class Agglomerator_Anisotropic : while (!end) { // for the seed (that is updated each time end!= true) we fill the // neighbours and the weights - const std::vector v_neighbours = - this->_fc_graph->get_neighbours(seed); - const std::vector v_w_neighbours = - this->_fc_graph->get_weights(seed); auto n_it = this->_fc_graph->neighbours_cbegin(seed); auto w_it = this->_fc_graph->weights_cbegin(seed); // std::vector of the candidates to continue the line @@ -610,7 +604,7 @@ class Agglomerator_Anisotropic : for (; n_it != this->_fc_graph->neighbours_cend(seed); ++n_it, ++w_it) { if (to_treat[*n_it] && *w_it > 0.90 * max_weights[seed]) { - candidates.emplace(*w_it, *n_it); + candidates.emplace(*n_it, *w_it); } } // end for loop } else { @@ -621,11 +615,14 @@ class Agglomerator_Anisotropic : if (to_treat[*n_it] && *w_it > 0.90 * max_weights[seed]) { std::vector cur_dir(pts_dim); get_direction( - prev_cen, this->_fc_graph->_centers[*n_it], cur_dir + this->_fc_graph->_centers[seed], + this->_fc_graph->_centers[*n_it], + cur_dir ); const auto dot = dot_product(prev_dir, cur_dir); - if (!dot_deviate(dot)) - candidates.emplace(fabs(dot), *n_it); + if (!dot_deviate(dot)) { + candidates.emplace(*n_it, fabs(dot)); + } } } // end for loop } @@ -638,7 +635,8 @@ class Agglomerator_Anisotropic : * (we overwrite the seed). This is not proper */ // update the seed to the actual candidate - seed = candidates.begin()->second; + const auto old_seed = seed; + seed = candidates.begin()->first; if (!opposite_direction_check) { cur_line->push_back(seed); } else { @@ -646,9 +644,11 @@ class Agglomerator_Anisotropic : } to_treat[seed] = false; empty_line = false; - const auto &cur_cen = this->_fc_graph->_centers[seed]; - get_direction(prev_cen, cur_cen, prev_dir); - prev_cen = cur_cen; // this->_fc_graph->_centers[seed]; + get_direction( + this->_fc_graph->_centers[old_seed], + this->_fc_graph->_centers[seed], + prev_dir + ); if (!primal_dir.has_value()) primal_dir = prev_dir; } // 0 candidate, we are at the end of the line or at the end of one @@ -666,17 +666,21 @@ class Agglomerator_Anisotropic : if (to_treat[*it]) { std::vector cur_dir(pts_dim); get_direction( - prev_cen, this->_fc_graph->_centers[*it], cur_dir + this->_fc_graph->_centers[seed], + this->_fc_graph->_centers[*it], + cur_dir ); const auto dot = dot_product(prev_dir, cur_dir); - if (!dot_deviate(dot)) - candidates.emplace(fabs(dot), *it); + if (!dot_deviate(dot)) { + candidates.emplace(*it, fabs(dot)); + } } } // end for loop if (!candidates.empty()) { // We found one! Keep going! - seed = candidates.begin()->second; + const auto old_seed = seed; + seed = candidates.begin()->first; if (!opposite_direction_check) { cur_line->push_back(seed); } else { @@ -684,23 +688,40 @@ class Agglomerator_Anisotropic : } to_treat[seed] = false; empty_line = false; - const auto &cur_cen = this->_fc_graph->_centers[seed]; - get_direction(prev_cen, cur_cen, prev_dir); - prev_cen = cur_cen; // this->_fc_graph->_centers[seed]; + get_direction( + this->_fc_graph->_centers[old_seed], + this->_fc_graph->_centers[seed], + prev_dir + ); if (!primal_dir.has_value()) primal_dir = prev_dir; } else { - if (opposite_direction_check) { - end = true; - } else { + // If we have already looked at the other side of the line or if + // the primal seed is where we are already at, there is nothing + // that we can do + if (!opposite_direction_check && seed != primal_seed) { seed = primal_seed; + // The check sed != prima_seed should ensure that + // primal_dir != null prev_dir = primal_dir.value(); - prev_cen = this->_fc_graph->_centers[seed]; opposite_direction_check = true; + } else { + end = true; } } } // End last step check on neighbours else { - end = true; + // If we have already looked at the other side of the line or if + // the primal seed is where we are already at, there is nothing + // that we can do + if (!opposite_direction_check && seed != primal_seed) { + seed = primal_seed; + // The check sed != prima_seed should ensure that + // primal_dir != null + prev_dir = primal_dir.value(); + opposite_direction_check = true; + } else { + end = true; + } } } // End of no candidates case } // End of a line diff --git a/tests/test_anisoagglo.cpp b/tests/test_anisoagglo.cpp index 2b56096..ce75fb6 100644 --- a/tests/test_anisoagglo.cpp +++ b/tests/test_anisoagglo.cpp @@ -564,8 +564,8 @@ the line grows vertically const auto max_w = *max_element(Data.weights.begin(), Data.weights.end()); for (const auto idx : bottom_layer) Data.weights[idx] = max_w + 1.; - const CoMMAWeightT aniso_thresh{-4. - }; // Negative so that every cell is considered + // Negative so that every cell is considered + const CoMMAWeightT aniso_thresh{-4.}; const bool build_lines = true; const vector agglomerationLines_Idx{}; const vector agglomerationLines{}; @@ -597,14 +597,6 @@ the line grows vertically MAX_CELLS_IN_LINE, Data.dim ); - const Agglomerator_Biconnected iso_agg( - fc_graph, - cc_graph, - seeds_pool, - CoMMANeighbourhoodT::EXTENDED, - FC_ITER, - Data.dim - ); WHEN("We agglomerate the mesh") { aniso_agg.agglomerate_one_level(4, 4, 4, Data.weights, false); THEN("All cells have been agglomerated") { @@ -617,7 +609,7 @@ the line grows vertically )) ); } - const auto f2c = cc_graph->_fc_2_cc; + const auto &f2c = cc_graph->_fc_2_cc; THEN("The anisotropic coarse cells at the bottom are of cardinality 2") { REQUIRE(CHECK2CELLS(f2c, 0, 1)); REQUIRE(CHECK2CELLS(f2c, 2, 3));