Merge pull request #1362 from arcondello/QuadraticModelBase.remove_va…

…riables

Add QuadraticModelBase::remove_variables() method

dimod/include/dimod/abc.h

@@ -22,6 +22,7 @@
 #include <utility>
 #include <vector>
 
+#include "dimod/utils.h"
 #include "dimod/vartypes.h"
 
 namespace dimod {
@@ -338,6 +339,9 @@ class QuadraticModelBase {
      */
     virtual void remove_variable(index_type v);
 
+    /// Remove multiple variables from the model and reindex accordingly.
+    virtual void remove_variables(const std::vector<index_type>& variables);
+
     /// Multiply all biases by the value of `scalar`.
     void scale(bias_type scalar);
 
@@ -918,6 +922,58 @@ void QuadraticModelBase<bias_type, index_type>::remove_variable(index_type v) {
     }
 }
 
+template <class bias_type, class index_type>
+void QuadraticModelBase<bias_type, index_type>::remove_variables(
+        const std::vector<index_type>& variables) {
+    if (!variables.size()) return;  // shortcut
+
+    if (!std::is_sorted(variables.begin(), variables.end())) {
+        // create a copy and sort it
+        std::vector<index_type> sorted_indices = variables;
+        std::sort(sorted_indices.begin(), sorted_indices.end());
+        QuadraticModelBase<bias_type, index_type>::remove_variables(sorted_indices);
+        return;
+    }
+
+    linear_biases_.erase(utils::remove_by_index(linear_biases_.begin(), linear_biases_.end(),
+                                                variables.begin(), variables.end()),
+                         linear_biases_.end());
+
+    if (has_adj()) {
+        // clean up the remaining neighborhoods
+        // in this case we need a reindexing scheme, so we do the expensive O(num_variables)
+        // thing once to save time later on
+        std::vector<int> reindex(adj_ptr_->size());
+        for (const auto& v : variables) {
+            if (v > static_cast<int>(reindex.size())) break;  // we can break because it's sorted
+            reindex[v] = -1;
+        }
+        int label = 0;
+        for (auto& v : reindex) {
+            if (v == -1) continue;  // the removed variables
+            v = label;
+            ++label;
+        }
+
+        // remove the relevant neighborhoods
+        adj_ptr_->erase(utils::remove_by_index(adj_ptr_->begin(), adj_ptr_->end(), variables.begin(),
+                                               variables.end()),
+                        adj_ptr_->end());
+
+        // now go through and adjust the remaining neighborhoods
+        auto pred = [&reindex](OneVarTerm<bias_type, index_type>& term) {
+            if (reindex[term.v] == -1) return true;  // remove
+            // otherwise apply the new label
+            term.v = reindex[term.v];
+            return false;
+        };
+        for (auto& n : *adj_ptr_) {
+            // we modify the indices and remove the variables we need to remove
+            n.erase(std::remove_if(n.begin(), n.end(), pred), n.end());
+        }
+    }
+}
+
 template <class bias_type, class index_type>
 void QuadraticModelBase<bias_type, index_type>::resize(index_type n) {
     assert(n >= 0);

dimod/include/dimod/expression.h

@@ -14,13 +14,15 @@
 
 #pragma once
 
+#include <algorithm>
 #include <limits>
 #include <unordered_map>
 #include <unordered_set>
 #include <utility>
 #include <vector>
 
 #include "dimod/abc.h"
+#include "dimod/utils.h"
 #include "dimod/vartypes.h"
 
 namespace dimod {
@@ -272,6 +274,10 @@ class Expression : public abc::QuadraticModelBase<Bias, Index> {
     template<class Iter>
     void remove_variables(Iter first, Iter last);
 
+    void remove_variables(const std::vector<index_type>& variables) {
+        return remove_variables(variables.begin(), variables.end());
+    }
+
     /// Set the linear bias of variable `v`.
     void set_linear(index_type v, bias_type bias);
 
@@ -632,27 +638,23 @@ void Expression<bias_type, index_type>::remove_variable(index_type v) {
 template <class bias_type, class index_type>
 template <class Iter>
 void Expression<bias_type, index_type>::remove_variables(Iter first, Iter last) {
-    std::unordered_set<index_type> to_remove;
+    // get the indices of any variables that need to be removed
+    std::vector<index_type> to_remove;
     for (auto it = first; it != last; ++it) {
-        if (indices_.find(*it) != indices_.end()) {
-            to_remove.emplace(*it);
+        auto search = indices_.find(*it);
+        if (search != indices_.end()) {
+            to_remove.emplace_back(search->second);
         }
     }
+    std::sort(to_remove.begin(), to_remove.end());
 
-    if (!to_remove.size()) {
-        return;  // nothing to remove
-    }
+    // remove the indices from variables_ and the underlying
+    variables_.erase(utils::remove_by_index(variables_.begin(), variables_.end(), to_remove.begin(),
+                                            to_remove.end()),
+                     variables_.end());
 
-    // now remove any variables found in to_remove
-    size_type i = 0;
-    while (i < this->num_variables()) {
-        if (to_remove.count(variables_[i])) {
-            base_type::remove_variable(i);
-            variables_.erase(variables_.begin() + i);
-        } else {
-            ++i;
-        }
-    }
+    // remove the indices from the underlying quadratic model
+    base_type::remove_variables(to_remove);
 
     // finally fix the indices by rebuilding from scratch
     indices_.clear();

dimod/include/dimod/quadratic_model.h

@@ -14,6 +14,7 @@
 
 #pragma once
 
+#include <algorithm>
 #include <stdexcept>
 #include <utility>
 #include <vector>
@@ -87,6 +88,9 @@ class QuadraticModel : public abc::QuadraticModelBase<Bias, Index> {
     /// Remove variable `v`.
     void remove_variable(index_type v);
 
+    /// Remove variables.
+    void remove_variables(const std::vector<index_type>& variables);
+
     // Resize the model to contain `n` variables.
     void resize(index_type n);
 
@@ -269,6 +273,19 @@ void QuadraticModel<bias_type, index_type>::remove_variable(index_type v) {
     varinfo_.erase(varinfo_.begin() + v);
 }
 
+template <class bias_type, class index_type>
+void QuadraticModel<bias_type, index_type>::remove_variables(const std::vector<index_type>& variables) {
+    if (!std::is_sorted(variables.begin(), variables.end())) {
+        // create a copy and sort it
+        std::vector<index_type> sorted_indices = variables;
+        std::sort(sorted_indices.begin(), sorted_indices.end());
+        QuadraticModel<bias_type, index_type>::remove_variables(sorted_indices);
+        return;
+    }
+    base_type::remove_variables(variables);
+    varinfo_.erase(utils::remove_by_index(varinfo_.begin(), varinfo_.end(), variables.begin(), variables.end()), varinfo_.end());
+}
+
 template <class bias_type, class index_type>
 void QuadraticModel<bias_type, index_type>::resize(index_type n) {
     // we could do this as an assert, but let's be careful since

dimod/include/dimod/utils.h

@@ -22,6 +22,34 @@
 namespace dimod {
 namespace utils {
 
+// Remove all elements in the range defined by vfirst to vlast at indices
+// specified by ifirst to ilast.
+// All iterators must be forward iterators
+// Indices must be non-negative, sorted, and unique.
+template <class ValueIter, class IndexIter>
+ValueIter remove_by_index(ValueIter vfirst, ValueIter vlast, IndexIter ifirst, IndexIter ilast) {
+    assert(std::is_sorted(ifirst, ilast));
+    assert((ifirst == ilast || *ifirst >= 0));
+
+    using value_type = typename std::iterator_traits<ValueIter>::value_type;
+
+    typename std::iterator_traits<IndexIter>::value_type loc = 0;  // location in the values
+    IndexIter it = ifirst;
+    auto pred = [&](const value_type&) {
+        if (it != ilast && *it == loc) {
+            ++loc;
+            ++it;
+            return true;
+        } else {
+            ++loc;
+            return false;
+        }
+    };
+
+    // relies on this being executed sequentially
+    return std::remove_if(vfirst, vlast, pred);
+}
+
     // zip_sort is a modification of the code found here :
     // https://www.geeksforgeeks.org/iterative-quick-sort/
 

releasenotes/notes/QuadraticModelBase.remove_variables-e65ed0c138c3ed75.yaml

@@ -0,0 +1,4 @@
+---
+features:
+  - Add C++ ``dimod::abc::QuadraticModelBase::remove_variables()`` method and accompanying overloads.
+  - Speed up C++ ``dimod::Expression::remove_variables()`` method.

testscpp/tests/test_binary_quadratic_model.cpp

@@ -38,6 +38,19 @@ TEST_CASE("BinaryQuadraticModel tests") {
             }
         }
 
+        WHEN("we use remove_variables()") {
+            bqm.remove_variables(std::vector<int>{3, 1});
+
+            THEN("The variables are removed and the model is reindexed") {
+                REQUIRE(bqm.num_variables() == 3);
+                REQUIRE(bqm.num_interactions() == 0);
+                CHECK(bqm.linear(0) == 0);
+                CHECK(bqm.linear(1) == 2);  // this was reindexed
+                CHECK(bqm.linear(2) == 4);  // this was reindexed twice
+                CHECK(bqm.offset() == 5);
+            }
+        }
+
         WHEN("we use fix_variable()") {
             bqm.fix_variable(2, -1);
             THEN("the variable is removed, its biases distributed and the model is reindexed") {
@@ -94,6 +107,52 @@ TEST_CASE("BinaryQuadraticModel tests") {
             }
         }
 
+        WHEN("we use remove_variables()") {
+            bqm.remove_variables(std::vector<int>{3, 1});
+
+            THEN("The variables are removed and the model is reindexed") {
+                REQUIRE(bqm.num_variables() == 3);
+                CHECK(bqm.linear(0) == 0);
+                CHECK(bqm.linear(1) == 2);  // this was reindexed
+                CHECK(bqm.linear(2) == 4);  // this was reindexed twice
+                CHECK(bqm.offset() == 5);
+                CHECK(bqm.num_interactions() == 0);  // no remaining quadratic
+            }
+        }
+
+        WHEN("we use remove_variables() to remove one variable") {
+            bqm.remove_variables({2});
+
+            THEN("everything is reindexed") {
+                REQUIRE(bqm.num_variables() == 4);
+                REQUIRE(bqm.num_interactions() == 2);
+                CHECK(bqm.linear(0) == 0);
+                CHECK(bqm.linear(1) == -1);
+                CHECK(bqm.linear(2) == -3);  // this was reindexed
+                CHECK(bqm.linear(3) == 4);   // this was reindexed
+                CHECK(bqm.quadratic(0, 1) == 1);
+                CHECK(bqm.quadratic(2, 3) == 34);  // this was reindexed
+                CHECK(bqm.offset() == 5);
+            }
+        }
+
+        WHEN("we use remove_variables() to remove no variables") {
+            bqm.remove_variables({});
+
+            THEN("nothing has changed") {
+                REQUIRE(bqm.num_variables() == 5);
+                REQUIRE(bqm.num_interactions() == 4);
+                CHECK(bqm.linear(0) == 0);
+                CHECK(bqm.linear(1) == -1);
+                CHECK(bqm.linear(2) == 2);
+                CHECK(bqm.linear(3) == -3);
+                CHECK(bqm.linear(4) == 4);
+                CHECK(bqm.quadratic(0, 1) == 1);
+                CHECK(bqm.quadratic(3, 4) == 34);
+                CHECK(bqm.offset() == 5);
+            }
+        }
+
         AND_GIVEN("another identical BQM") {
             auto bqm2 = BinaryQuadraticModel<double>(5, Vartype::SPIN);
             bqm2.set_offset(5);

testscpp/tests/test_quadratic_model.cpp

@@ -53,6 +53,28 @@ TEST_CASE("QuadraticModel tests") {
             }
         }
 
+        WHEN("we use remove_variables()") {
+            qm.remove_variables({2});
+
+            THEN("the variable is removed and the model is reindexed") {
+                REQUIRE(qm.num_variables() == 4);
+                REQUIRE(qm.num_interactions() == 0);
+                CHECK(qm.offset() == 5);
+                CHECK(qm.linear(0) == 0);
+                CHECK(qm.linear(1) == -1);
+                CHECK(qm.linear(2) == -3);  // this was reindexed
+                CHECK(qm.linear(3) == 4);   // this was reindexed
+                CHECK(qm.vartype(0) == Vartype::BINARY);
+                CHECK(qm.vartype(1) == Vartype::INTEGER);
+                CHECK(qm.vartype(2) == Vartype::REAL);  // this was reindexed
+                CHECK(qm.vartype(3) == Vartype::SPIN);  // this was reindexed
+                CHECK(qm.lower_bound(1) == -1);
+                CHECK(qm.lower_bound(2) == -3);  // this was reindexed
+                CHECK(qm.upper_bound(1) == 1);
+                CHECK(qm.upper_bound(2) == 3);  // this was reindexed
+            }
+        }
+
         WHEN("we use fix_variable()") {
             qm.fix_variable(2, -1);
             THEN("the variable is removed, its biases distributed and the model is reindexed") {

testscpp/tests/test_utils.cpp

@@ -14,13 +14,56 @@
 
 #include <iostream>
 #include <random>
+#include <vector>
 
 #include "catch2/catch.hpp"
 #include "dimod/utils.h"
 
 namespace dimod {
 namespace utils {
 
+TEST_CASE("remove_by_index()") {
+    GIVEN("A vector") {
+        auto v = std::vector<int>{0, 1, 2, 3, 4, 5, 6};
+
+        AND_GIVEN("some indices") {
+            auto i = std::vector<int>{1, 3, 4};
+
+            WHEN("We use remove_by_index() to shrink the vector") {
+                v.erase(remove_by_index(v.begin(), v.end(), i.begin(), i.end()), v.end());
+
+                THEN("The vector has the values we expect") {
+                    REQUIRE_THAT(v, Catch::Approx(std::vector<int>{0, 2, 5, 6}));
+                }
+            }
+        }
+
+        AND_GIVEN("Some indices that are out-of-range") {
+            auto i = std::vector<int>{5, 105};
+
+            WHEN("We use remove_by_index() to shrink the vector") {
+                v.erase(remove_by_index(v.begin(), v.end(), i.begin(), i.end()), v.end());
+
+                THEN("The vector has the values we expect") {
+                    REQUIRE_THAT(v, Catch::Approx(std::vector<int>{0, 1, 2, 3, 4, 6}));
+                }
+            }
+        }
+
+        AND_GIVEN("An empty indices vector") {
+            auto i = std::vector<int>{};
+
+            WHEN("We use remove_by_index() to shrink the vector") {
+                v.erase(remove_by_index(v.begin(), v.end(), i.begin(), i.end()), v.end());
+
+                THEN("The vector has the values we expect") {
+                    REQUIRE_THAT(v, Catch::Approx(std::vector<int>{0, 1, 2, 3, 4, 5, 6}));
+                }
+            }
+        }
+    }
+}
+
     TEST_CASE("Two vectors are zip-sorted", "[utils]") {
         std::default_random_engine generator;
         std::uniform_int_distribution<int> int_distribution(0, 100);