Merge pull request #1140 from PowerGridModel/feature/counting-sort-swap-to-avoid-copying

Performance: Using swapping in places explicit copying could been avoided

Author: Jerry-Jinfeng-Guo

power_grid_model_c/power_grid_model/include/power_grid_model/index_mapping.hpp

@@ -103,11 +103,12 @@ inline auto build_dense_mapping_comparison_sort(IdxVector const& idx_B_in_A, Idx
     DenseIndexMapping result;
     result.indvector.reserve(mapping_to_from.size());
     result.reorder.reserve(mapping_to_from.size());
-    std::ranges::transform(mapping_to_from, std::back_inserter(result.indvector),
-                           [](DenseEntry const& to_from) { return to_from.first; });
 
-    std::ranges::transform(mapping_to_from, std::back_inserter(result.reorder),
-                           [](DenseEntry const& to_from) { return to_from.second; });
+    // Use structured bindings to avoid copying from pairs
+    for (auto [value, orig_idx] : mapping_to_from) {
+        result.indvector.push_back(value);
+        result.reorder.push_back(orig_idx);
+    }
 
     return result;
 }

power_grid_model_c/power_grid_model/include/power_grid_model/math_solver/y_bus.hpp

@@ -40,30 +40,38 @@ inline void append_element_vector(std::vector<YBusElementMap>& vec, Idx first_bu
 
 // counting sort element
 inline void counting_sort_element(std::vector<YBusElementMap>& vec, Idx n_bus) {
-    // count vec
-    std::vector<YBusElementMap> count_vec(vec.size());
+    // temp vec for swapping
+    std::vector<YBusElementMap> temp_vec(vec.size());
     IdxVector counter(n_bus, 0);
-    // sort column
+
+    // sort column first
     for (YBusElementMap const& element : vec) {
         ++counter[element.pos.second];
     }
     for (size_t i = 1, n = counter.size(); i != n; ++i) {
         counter[i] += counter[i - 1];
     }
-    for (auto it_element = vec.crbegin(); it_element != vec.crend(); ++it_element) {
-        count_vec[--counter[it_element->pos.second]] = *it_element;
+    for (auto it_element = vec.rbegin(); it_element != vec.rend(); ++it_element) {
+        temp_vec[--counter[it_element->pos.second]] = std::move(*it_element);
     }
-    // sort row
+
+    // swap vectors to avoid copying
+    vec.swap(temp_vec);
+
+    // sort row second
     std::ranges::fill(counter, 0);
-    for (YBusElementMap const& element : count_vec) {
+    for (YBusElementMap const& element : vec) {
         ++counter[element.pos.first];
     }
     for (size_t i = 1, n = counter.size(); i != n; ++i) {
         counter[i] += counter[i - 1];
     }
-    for (auto it_element = count_vec.crbegin(); it_element != count_vec.crend(); ++it_element) {
-        vec[--counter[it_element->pos.first]] = *it_element;
+    for (auto it_element = vec.rbegin(); it_element != vec.rend(); ++it_element) {
+        temp_vec[--counter[it_element->pos.first]] = std::move(*it_element);
     }
+
+    // final swap to get result back in vec
+    vec.swap(temp_vec);
 }
 
 // y bus structure

tests/cpp_unit_tests/test_y_bus.cpp

@@ -426,7 +426,129 @@ TEST_CASE("Incremental update y-bus") {
     }
 }
 
-// TODO:
-// - test counting_sort_element()
+TEST_CASE("Test counting_sort_element") {
+    using math_solver::counting_sort_element;
+    using math_solver::YBusElementMap;
+    using enum power_grid_model::YBusElementType;
+
+    SUBCASE("Test basic sorting") {
+        // Create test data: elements at various matrix positions
+        std::vector<YBusElementMap> vec = {
+            {{2, 1}, {bft, 5}},   // pos (2,1)
+            {{0, 0}, {bff, 0}},   // pos (0,0)
+            {{1, 2}, {btf, 3}},   // pos (1,2)
+            {{0, 1}, {bft, 1}},   // pos (0,1)
+            {{2, 1}, {shunt, 6}}, // pos (2,1) - same position as first
+            {{1, 0}, {btf, 2}},   // pos (1,0)
+            {{2, 2}, {btt, 7}},   // pos (2,2)
+        };
+
+        // Expected sorted order: by row first, then by column
+        // (0,0), (0,1), (1,0), (1,2), (2,1), (2,1), (2,2)
+        std::vector<std::pair<Idx, Idx>> expected_positions = {{0, 0}, {0, 1}, {1, 0}, {1, 2}, {2, 1}, {2, 1}, {2, 2}};
+
+        Idx const n_bus = 3;
+        counting_sort_element(vec, n_bus);
+
+        // Verify sorting
+        CHECK(vec.size() == 7);
+        for (size_t i = 0; i < vec.size(); ++i) {
+            CHECK(vec[i].pos.first == expected_positions[i].first);
+            CHECK(vec[i].pos.second == expected_positions[i].second);
+        }
+
+        // Verify specific elements are preserved correctly
+        CHECK(vec[0].element.element_type == bff);
+        CHECK(vec[0].element.idx == 0);
+        CHECK(vec[1].element.element_type == bft);
+        CHECK(vec[1].element.idx == 1);
+    }
+
+    SUBCASE("Test with single bus") {
+        std::vector<YBusElementMap> vec = {{{0, 0}, {shunt, 10}}};
+
+        counting_sort_element(vec, 1);
+
+        CHECK(vec.size() == 1);
+        CHECK(vec[0].pos == std::make_pair(0, 0));
+        CHECK(vec[0].element.element_type == shunt);
+        CHECK(vec[0].element.idx == 10);
+    }
+
+    SUBCASE("Test with empty vector") {
+        std::vector<YBusElementMap> vec;
+        counting_sort_element(vec, 5);
+        CHECK(vec.empty());
+    }
+
+    SUBCASE("Test stability - elements with same position maintain relative order") {
+        std::vector<YBusElementMap> vec = {
+            {{1, 1}, {bff, 100}},
+            {{1, 1}, {bft, 200}},
+            {{1, 1}, {shunt, 300}},
+        };
+
+        counting_sort_element(vec, 2);
+
+        CHECK(vec.size() == 3);
+        // All should be at position (1,1)
+        for (const auto& element : vec) {
+            CHECK(element.pos == std::make_pair(1, 1));
+        }
+        // Original relative order should be preserved (stable sort)
+        CHECK(vec[0].element.idx == 100);
+        CHECK(vec[1].element.idx == 200);
+        CHECK(vec[2].element.idx == 300);
+    }
+
+    SUBCASE("Test large sparse matrix scenario") {
+        std::vector<YBusElementMap> vec;
+        Idx const n_bus = 10;
+
+        // Add elements in reverse order to test sorting thoroughly
+        for (Idx i = n_bus * n_bus - 1; i != static_cast<Idx>(-1); --i) {
+            Idx const row = i / n_bus;
+            Idx const col = i % n_bus;
+            if ((row + col) % 3 == 0) { // Sparse pattern
+                vec.push_back({{row, col}, {bff, row * n_bus + col}});
+            }
+        }
+
+        size_t original_size = vec.size();
+        counting_sort_element(vec, n_bus);
+
+        CHECK(vec.size() == original_size);
+
+        // Verify sorted order
+        for (size_t i = 1; i < vec.size(); ++i) {
+            auto [prev_row, prev_col] = vec[i - 1].pos;
+            auto [curr_row, curr_col] = vec[i].pos;
+
+            // Check row-major order
+            if (prev_row == curr_row) {
+                CHECK(prev_col <= curr_col); // Same row, column should be non-decreasing
+            } else {
+                CHECK(prev_row < curr_row); // Different row, previous row should be smaller
+            }
+        }
+    }
+
+    SUBCASE("Test all YBusElementType values") {
+        std::vector<YBusElementMap> vec = {
+            {{1, 1}, {fill_in_tf, 6}}, {{0, 1}, {bft, 1}},   {{1, 0}, {btf, 2}},        {{0, 0}, {bff, 0}},
+            {{1, 1}, {btt, 3}},        {{2, 2}, {shunt, 4}}, {{1, 2}, {fill_in_ft, 5}},
+        };
+
+        counting_sort_element(vec, 3);
+
+        // Verify positions are sorted correctly
+        std::vector<std::pair<Idx, Idx>> expected_positions = {{0, 0}, {0, 1}, {1, 0}, {1, 1}, {1, 1}, {1, 2}, {2, 2}};
+
+        CHECK(vec.size() == 7);
+        for (size_t i = 0; i < vec.size(); ++i) {
+            CHECK(vec[i].pos == expected_positions[i]);
+        }
+    }
+}
 
 } // namespace power_grid_model