Merge pull request #181 from arcondello/feature/minor-Update-improvem…

…ents

Refactor `Update`'s comparison methods slightly

dwave/optimization/include/dwave-optimization/array.hpp

@@ -189,15 +189,11 @@ struct Update {
         return Update(index, old, nothing);
     }
 
-    // We use the index to compare updates. This is useful because we can
-    // stablely sort a vector of updates and preserve the other of operations
-    // on a single index.
-    friend constexpr bool operator<(const Update& lhs, const Update& rhs) {
-        return lhs.index < rhs.index;
+    // We want to be able to stably sort updates based on the index.
+    friend constexpr auto operator<=>(const Update& lhs, const Update& rhs) {
+        return lhs.index <=> rhs.index;
     }
-
-    // Added for easier unit testing, checks that all members are equal (or nan and nan)
-    static bool equals(const Update& lhs, const Update& rhs) {
+    friend constexpr bool operator==(const Update& lhs, const Update& rhs) noexcept {
         return (lhs.index == rhs.index &&
                 (lhs.old == rhs.old || (std::isnan(lhs.old) && std::isnan(rhs.old))) &&
                 (lhs.value == rhs.value || (std::isnan(lhs.value) && std::isnan(rhs.value))));
@@ -224,10 +220,8 @@ struct Update {
 
     double value_or(double val) const { return std::isnan(value) ? val : value; }
 
-    // Returns whether this is a identity update, i.e. whether the old value equals the
-    // value. In the case of both old and value being NaN, this should still return
-    // false.
-    bool identity() const { return old == value; }
+    // Return true if the update does nothing - that is old and value are the same.
+    bool identity() const { return null() || old == value; }
 
     // Use NaN to represent the "nothing" value used in placements/removals
     static constexpr double nothing = std::numeric_limits<double>::signaling_NaN();

dwave/optimization/src/utils.cpp

@@ -23,7 +23,7 @@ namespace dwave::optimization {
 void deduplicate_diff(std::vector<Update>& diff) {
     if (diff.empty()) return;
 
-    std::stable_sort(diff.begin(), diff.end());
+    std::ranges::stable_sort(diff);
 
     // Find the index of first non-noop Update. If there are none, leave it as -1
     // to represent no final updates.

releasenotes/notes/Update-totally-ordered-59a3c2e3c9c24fd2.yaml

@@ -0,0 +1,10 @@
+---
+features:
+  - |
+    Implement C++ ``operator<=>(const Update&, const Update&)``
+    and ``operator==(const Update&, const Update&)``.
+    This allows ``Update`` to be used with ``std::ranges::stable_sort()``.
+upgrade:
+  - |
+    Remove C++ ``Update::equals(const Update&)`` method in favour of
+    ``operator==(const Update&, const Update&)``.

tests/cpp/test_array.cpp

@@ -578,6 +578,8 @@ TEST_CASE("Dynamically Sized 2d Array") {
 }
 
 TEST_CASE("Update") {
+    static_assert(std::totally_ordered<Update>);
+
     SECTION("Update::removal()") {
         auto update = Update::removal(105, 56.5);
         CHECK(update.index == 105);

tests/cpp/test_utils.cpp

@@ -131,8 +131,6 @@ TEST_CASE("Test fraction") {
 }
 
 TEST_CASE("Test deduplicate_diff") {
-    auto pred = [](Update& a, Update& b) { return Update::equals(a, b); };
-
     GIVEN("An empty vector of updates") {
         std::vector<Update> updates;
 
@@ -150,8 +148,7 @@ TEST_CASE("Test deduplicate_diff") {
             THEN("deduplicate_diff() sorts them") {
                 CHECK(std::ranges::equal(
                         updates,
-                        std::vector<Update>{Update(2, 1, 0), Update(3, 0, 1), Update(5, 0, -1)},
-                        pred));
+                        std::vector<Update>{Update(2, 1, 0), Update(3, 0, 1), Update(5, 0, -1)}));
             }
         }
     }
@@ -163,7 +160,7 @@ TEST_CASE("Test deduplicate_diff") {
             deduplicate_diff(updates);
             THEN("deduplicate_diff() sorts them and removes noops") {
                 CHECK(std::ranges::equal(
-                        updates, std::vector<Update>{Update(2, 1, 0), Update(3, 0, 1)}, pred));
+                        updates, std::vector<Update>{Update(2, 1, 0), Update(3, 0, 1)}));
             }
         }
     }
@@ -196,8 +193,7 @@ TEST_CASE("Test deduplicate_diff") {
             THEN("deduplicate_diff() removes the noop") {
                 CHECK(std::ranges::equal(
                         updates,
-                        std::vector<Update>{Update(3, 1, 5), Update(4, 1, 5), Update(6, -1, -2)},
-                        pred));
+                        std::vector<Update>{Update(3, 1, 5), Update(4, 1, 5), Update(6, -1, -2)}));
             }
         }
     }
@@ -211,8 +207,7 @@ TEST_CASE("Test deduplicate_diff") {
             THEN("deduplicate_diff() removes the noop") {
                 CHECK(std::ranges::equal(
                         updates,
-                        std::vector<Update>{Update(3, 1, 5), Update(4, 1, 5), Update(6, -1, -2)},
-                        pred));
+                        std::vector<Update>{Update(3, 1, 5), Update(4, 1, 5), Update(6, -1, -2)}));
             }
         }
     }
@@ -226,8 +221,7 @@ TEST_CASE("Test deduplicate_diff") {
             THEN("deduplicate_diff() removes the noop") {
                 CHECK(std::ranges::equal(
                         updates,
-                        std::vector<Update>{Update(2, 0, 1), Update(3, 1, -4), Update(6, -1, 57)},
-                        pred));
+                        std::vector<Update>{Update(2, 0, 1), Update(3, 1, -4), Update(6, -1, 57)}));
             }
         }
     }
@@ -241,8 +235,7 @@ TEST_CASE("Test deduplicate_diff") {
             deduplicate_diff(updates);
             THEN("deduplicate_diff() trims the diff properly") {
                 CHECK(std::ranges::equal(
-                        updates, std::vector<Update>{Update(3, 1, 5), Update::placement(5, 4)},
-                        pred));
+                        updates, std::vector<Update>{Update(3, 1, 5), Update::placement(5, 4)}));
             }
         }
     }