diff --git a/dimod/include/dimod/constrained_quadratic_model.h b/dimod/include/dimod/constrained_quadratic_model.h index df20632ab..a42b2f3a4 100644 --- a/dimod/include/dimod/constrained_quadratic_model.h +++ b/dimod/include/dimod/constrained_quadratic_model.h @@ -545,9 +545,8 @@ template void ConstrainedQuadraticModel::fix_variables_expr( const Expression& src, Expression& dst, const std::vector& old_to_new, const std::vector& assignments) { - // We'll want to access the expressions by index for speed + // We'll want to access the source expression by index for speed const abc::QuadraticModelBase& isrc = src; - abc::QuadraticModelBase& idst = dst; // offset dst.add_offset(src.offset()); @@ -565,23 +564,27 @@ void ConstrainedQuadraticModel::fix_variables_expr( } } - // quadratic, and it's safe to do everything by index! - for (auto it = isrc.cbegin_quadratic(); it != isrc.cend_quadratic(); ++it) { - auto u = src.variables()[it->u]; - auto v = src.variables()[it->v]; + // quadratic + for (auto it = isrc.cbegin_quadratic(), end = isrc.cend_quadratic(); it != end; ++it) { + const index_type u = src.variables()[it->u]; // variable u in the source + const index_type v = src.variables()[it->v]; // variable v in the source + const bias_type bias = it->bias; // bias in the source - if (old_to_new[u] < 0 && old_to_new[v] < 0) { + const index_type new_u = old_to_new[u]; // variable u in the destination + const index_type new_v = old_to_new[v]; // variable v in the destination + + if (new_u < 0 && new_v < 0) { // both fixed, becomes offset - idst.add_offset(assignments[u] * assignments[v] * it->bias); - } else if (old_to_new[u] < 0) { + dst.add_offset(assignments[u] * assignments[v] * bias); + } else if (new_u < 0) { // u fixed, v unfixed - idst.add_linear(old_to_new[it->v], assignments[u] * it->bias); - } else if (old_to_new[v] < 0) { + dst.add_linear(new_v, assignments[u] * bias); + } else if (new_v < 0) { // u unfixed, v fixed - idst.add_linear(old_to_new[it->u], assignments[v] * it->bias); + dst.add_linear(new_u, assignments[v] * bias); } else { // neither fixed - idst.add_quadratic_back(old_to_new[it->u], old_to_new[it->v], it->bias); + dst.add_quadratic_back(new_u, new_v, bias); } } } diff --git a/dimod/include/dimod/expression.h b/dimod/include/dimod/expression.h index 3d0ea013e..255a64583 100644 --- a/dimod/include/dimod/expression.h +++ b/dimod/include/dimod/expression.h @@ -366,7 +366,7 @@ void Expression::add_quadratic(ItRow row_iterator, ItCol template void Expression::add_quadratic_back(index_type u, index_type v, bias_type bias) { - throw std::logic_error("not implemented - add_quadratic_back"); + base_type::add_quadratic_back(enforce_variable(u), enforce_variable(v), bias); } template diff --git a/releasenotes/notes/fix-quadratic-cqm-fix-variables-43f21519357c228f.yaml b/releasenotes/notes/fix-quadratic-cqm-fix-variables-43f21519357c228f.yaml new file mode 100644 index 000000000..be353a235 --- /dev/null +++ b/releasenotes/notes/fix-quadratic-cqm-fix-variables-43f21519357c228f.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fix ``dimod::ConstrainedQuadraticModel::fix_variables()`` C++ method to work + correctly with quadratic objectives and constraints. + This fixes a bug introduced in dimod 0.12.5. + See `#1351 `_. diff --git a/testscpp/tests/test_constrained_quadratic_model.cpp b/testscpp/tests/test_constrained_quadratic_model.cpp index 67cab9686..d3155e227 100644 --- a/testscpp/tests/test_constrained_quadratic_model.cpp +++ b/testscpp/tests/test_constrained_quadratic_model.cpp @@ -871,6 +871,40 @@ TEST_CASE("Test CQM.constraint_weak_ptr()") { } } +TEST_CASE("Test ConstrainedQuadraticModel::fix_variables()") { + GIVEN("A CQM with a quadratic constraint") { + auto cqm = ConstrainedQuadraticModel(); + + cqm.add_variables(Vartype::BINARY, 5); + + auto c = cqm.add_linear_constraint({3, 1, 4, 0}, {1, 2, 3, 4}, Sense::LE, 5); + cqm.constraint_ref(c).add_quadratic(3, 1, -15); + cqm.constraint_ref(c).add_quadratic(4, 0, 14); + cqm.constraint_ref(c).add_quadratic(4, 3, 17); + + WHEN("we fix some variables") { + std::vector variables{1, 2, 3}; + std::vector values{1, 1, 1}; + + auto fixed = cqm.fix_variables(variables.begin(), variables.end(), values.begin()); + // relabels x0 -> x0 + // relabels x4 -> x1 + + THEN("the offset, linear, and quadratic biases are updated as expected") { + REQUIRE(fixed.num_constraints() == 1); + + CHECK(fixed.constraint_ref(c).num_variables() == 2); + CHECK(fixed.constraint_ref(c).num_interactions() == 1); + + CHECK(fixed.constraint_ref(c).offset() == -12); + CHECK(fixed.constraint_ref(c).linear(1) == 20); + CHECK(fixed.constraint_ref(c).linear(0) == 4); + CHECK(fixed.constraint_ref(c).quadratic(1, 0) == 14); + } + } + } +} + TEST_CASE("Test Expression::add_quadratic()") { GIVEN("A CQM with two variables with vartypes") { auto cqm = dimod::ConstrainedQuadraticModel(); @@ -901,6 +935,33 @@ TEST_CASE("Test Expression::add_quadratic()") { } } +TEST_CASE("Test Expression::add_quadratic_back()") { + GIVEN("A CQM with three variables and one constraints") { + auto cqm = dimod::ConstrainedQuadraticModel(); + auto i = cqm.add_variable(Vartype::INTEGER); + auto x = cqm.add_variable(Vartype::BINARY); + auto y = cqm.add_variable(Vartype::BINARY); + + auto c0 = cqm.add_linear_constraint({i, x, y}, {1, 1, 1}, Sense::LE, 1); + + WHEN("add_quadratic_back() is used on an empty constraint") { + cqm.constraint_ref(c0).add_quadratic_back(i, x, 1.5); + + THEN("we can read the value out as expected") { + CHECK(cqm.constraint_ref(c0).quadratic(i, x) == 1.5); + } + + AND_WHEN("we add another quadratic") { + cqm.constraint_ref(c0).add_quadratic_back(y, x, 2.5); + + THEN("we can read the value out as expected") { + CHECK(cqm.constraint_ref(c0).quadratic(y, x) == 2.5); + } + } + } + } +} + TEST_CASE("Test Expression::remove_variables()") { GIVEN("A CQM with five variables, an objective, and one constraint") { auto cqm = ConstrainedQuadraticModel();