From 9b0af78cd93588ca503cc610b4d3324097f776f7 Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Wed, 5 May 2021 15:10:48 -0400 Subject: [PATCH 01/20] Add Model interface for MVB --- libecole/include/ecole/scip/model.hpp | 4 +++- libecole/include/ecole/scip/scimpl.hpp | 3 ++- libecole/src/scip/model.cpp | 17 ++++++++++++++--- libecole/src/scip/scimpl.cpp | 26 ++++++++++++++++++++++---- 4 files changed, 41 insertions(+), 9 deletions(-) diff --git a/libecole/include/ecole/scip/model.hpp b/libecole/include/ecole/scip/model.hpp index 1b1c32765..90f3ac2a9 100644 --- a/libecole/include/ecole/scip/model.hpp +++ b/libecole/include/ecole/scip/model.hpp @@ -124,7 +124,9 @@ class Model { [[nodiscard]] bool is_solved() const noexcept; void solve_iter(); - void solve_iter_branch(SCIP_VAR* var); + void solve_iter_branch(nonstd::span vars); + void solve_iter_branch(SCIP_VAR const* var); + void solve_iter_branch(); void solve_iter_stop(); [[nodiscard]] bool solve_iter_is_done(); diff --git a/libecole/include/ecole/scip/scimpl.hpp b/libecole/include/ecole/scip/scimpl.hpp index 4c4c3f2e8..8a6943b56 100644 --- a/libecole/include/ecole/scip/scimpl.hpp +++ b/libecole/include/ecole/scip/scimpl.hpp @@ -2,6 +2,7 @@ #include +#include #include #include "ecole/utility/reverse-control.hpp" @@ -22,7 +23,7 @@ class Scimpl { Scimpl copy_orig(); void solve_iter(); - void solve_iter_branch(SCIP_VAR* var); + void solve_iter_branch(nonstd::span vars); void solve_iter_stop(); bool solve_iter_is_done(); diff --git a/libecole/src/scip/model.cpp b/libecole/src/scip/model.cpp index f8dc31056..0828e26fe 100644 --- a/libecole/src/scip/model.cpp +++ b/libecole/src/scip/model.cpp @@ -3,8 +3,8 @@ #include #include #include -#include #include +#include #include #include @@ -201,8 +201,19 @@ void Model::solve_iter() { scimpl->solve_iter(); } -void Model::solve_iter_branch(SCIP_VAR* var) { - scimpl->solve_iter_branch(var); +void Model::solve_iter_branch() { + return solve_iter_branch(nonstd::span{}); +} + +void Model::solve_iter_branch(SCIP_VAR const* var) { + return solve_iter_branch({&var, 1}); +} + +void Model::solve_iter_branch(nonstd::span vars) { + if (std::any_of(vars.begin(), vars.end(), [](auto* var) { return var == nullptr; })) { + throw std::invalid_argument{"All variables must be non null."}; + } + scimpl->solve_iter_branch(vars); } void Model::solve_iter_stop() { diff --git a/libecole/src/scip/scimpl.cpp b/libecole/src/scip/scimpl.cpp index 4cb84f44b..bc164e4fb 100644 --- a/libecole/src/scip/scimpl.cpp +++ b/libecole/src/scip/scimpl.cpp @@ -1,3 +1,5 @@ +#include +#include #include #include @@ -94,12 +96,28 @@ void Scimpl::solve_iter() { m_controller->wait_thread(); } -void scip::Scimpl::solve_iter_branch(SCIP_VAR* var) { - m_controller->resume_thread([var](SCIP* scip_ptr, SCIP_RESULT* result) { - if (var == nullptr) { +SCIP_RETCODE +SCIPbranchGUB(SCIP* scip, SCIP_VAR** vars, int nvars, SCIP_NODE** downchild, SCIP_NODE** eqchild, SCIP_NODE** upchild) { + if (nvars <= 0) { + return SCIP_ERROR; + } + if (nvars == 1) { + SCIP_CALL(SCIPbranchVar(scip, *vars, downchild, eqchild, upchild)); + } else { + // Our implementation + return SCIP_NOTIMPLEMENTED; + } + return SCIP_OKAY; +} + +void scip::Scimpl::solve_iter_branch(nonstd::span vars) { + assert(std::none_of(vars.begin(), vars.end(), [](auto* var) { return var == nullptr; })); + m_controller->resume_thread([vars](SCIP* scip_ptr, SCIP_RESULT* result) { + if (vars.empty()) { *result = SCIP_DIDNOTRUN; } else { - SCIP_CALL(SCIPbranchVar(scip_ptr, var, nullptr, nullptr, nullptr)); + SCIP_CALL(SCIPbranchGUB( + scip_ptr, const_cast(vars.data()), static_cast(vars.size()), nullptr, nullptr, nullptr)); *result = SCIP_BRANCHED; } return SCIP_OKAY; From 81e52f651100b617fe2129ddda1964f8ffef6b40 Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Thu, 6 May 2021 16:47:05 -0400 Subject: [PATCH 02/20] First draft of GUB branching --- libecole/CMakeLists.txt | 1 + libecole/src/scip/gub-branch.cpp | 107 +++++++++++++++++++++++++++++++ libecole/src/scip/gub-branch.hpp | 6 ++ libecole/src/scip/scimpl.cpp | 27 +++----- 4 files changed, 124 insertions(+), 17 deletions(-) create mode 100644 libecole/src/scip/gub-branch.cpp create mode 100644 libecole/src/scip/gub-branch.hpp diff --git a/libecole/CMakeLists.txt b/libecole/CMakeLists.txt index 7b105f8ec..41b844c0a 100644 --- a/libecole/CMakeLists.txt +++ b/libecole/CMakeLists.txt @@ -15,6 +15,7 @@ add_library( src/scip/row.cpp src/scip/col.cpp src/scip/exception.cpp + src/scip/gub-branch.cpp # Temporary src/instance/files.cpp src/instance/set-cover.cpp diff --git a/libecole/src/scip/gub-branch.cpp b/libecole/src/scip/gub-branch.cpp new file mode 100644 index 000000000..e2d897a5e --- /dev/null +++ b/libecole/src/scip/gub-branch.cpp @@ -0,0 +1,107 @@ +#include + +#include +#include +#include +#include +#include +#include +#include + +#include "scip/gub-branch.hpp" + +static SCIP_RETCODE SCIPbranchGUB_validate(SCIP* scip, SCIP_VAR** vars, int nvars) { + assert(scip != nullptr); + if (SCIPgetStage(scip) != SCIP_STAGE_SOLVING) { + SCIPerrorMessage("cannot branch when not solving\n"); + return SCIP_INVALIDCALL; + } + + if (nvars <= 0) { + SCIPerrorMessage("cannot branch on empty variable set\n"); + return SCIP_INVALIDDATA; + } + + SCIP_VAR const* const* const vars_end = vars + nvars; + for (SCIP_VAR** var_iter = vars; var_iter < vars_end; ++var_iter) { + // assert((*var_iter)->scip == scip); FIXME only in SCIP debug builds + assert(SCIPvarIsActive(*var_iter)); + assert(SCIPvarGetProbindex(*var_iter) >= 0); + if (SCIPvarGetType(*var_iter) == SCIP_VARTYPE_CONTINUOUS) { + SCIPerrorMessage("cannot branch on constraint containing continuous variable <%s>\n", SCIPvarGetName(*var_iter)); + return SCIP_INVALIDDATA; + } + if (SCIPisEQ(scip, SCIPvarGetLbLocal(*var_iter), SCIPvarGetUbLocal(*var_iter))) { + SCIPerrorMessage( + "cannot branch on constraint containing variable <%s> with fixed domain [%.15g,%.15g]\n", + SCIPvarGetName(*var_iter), + SCIPvarGetLbLocal(*var_iter), + SCIPvarGetUbLocal(*var_iter)); + return SCIP_INVALIDDATA; + } + } + + return SCIP_OKAY; +} + +static SCIP_RETCODE SCIPbranchGUB_add_child( + SCIP* scip, + SCIP_VAR** vars, + SCIP_Real* ones, + int nvars, + SCIP_Real lhs, + SCIP_Real rhs, + SCIP_NODE** node_out) { + // TODO decide on priorities + SCIP_NODE* node = nullptr; + SCIP_CALL(SCIPcreateChild(scip, &node, 1., 0.)); + auto name = fmt::format("branching-{}", SCIPnodeGetNumber(node)); + SCIP_CONS* cons = nullptr; + SCIP_CALL(SCIPcreateConsBasicLinear(scip, &cons, name.c_str(), nvars, vars, ones, lhs, rhs)); + SCIP_CALL(SCIPaddConsNode(scip, node, cons, nullptr)); + if (node_out != nullptr) { + *node_out = node; + } + return SCIP_OKAY; +} + +SCIP_RETCODE +SCIPbranchGUB(SCIP* scip, SCIP_VAR** vars, int nvars, SCIP_NODE** downchild, SCIP_NODE** eqchild, SCIP_NODE** upchild) { + SCIPbranchGUB_validate(scip, vars, nvars); + + SCIP_Real pseudo_sol_sum = 0.; + SCIP_VAR const* const* const vars_end = vars + nvars; + for (SCIP_VAR** var_iter = vars; var_iter < vars_end; ++var_iter) { + SCIP_Real val = SCIPvarGetSol(*var_iter, SCIPhasCurrentNodeLP(scip)); + + /* avoid branching on infinite values in pseudo solution */ + if (SCIPisInfinity(scip, -val) || SCIPisInfinity(scip, val)) { + // FIXME not sure what to do here + return SCIP_INVALIDDATA; + } + pseudo_sol_sum += val; + } + + SCIP_Real inf = SCIPinfinity(scip); + SCIP_Real* ones = nullptr; + SCIP_Retcode retcode = SCIP_OKAY; + SCIP_CALL(SCIPallocBufferArray(scip, &ones, nvars)); + for (int i = 0; i < nvars; ++i) { + ones[i] = 1.; + } + + SCIP_Real const downbound = SCIPfeasFloor(scip, pseudo_sol_sum); + SCIP_Real const upbound = SCIPfeasCeil(scip, pseudo_sol_sum); + if (SCIPisEQ(scip, downbound, upbound)) { + SCIP_CALL_TERMINATE(retcode, SCIPbranchGUB_add_child(scip, vars, ones, nvars, upbound, upbound, eqchild), TERM); + SCIP_CALL_TERMINATE(retcode, SCIPbranchGUB_add_child(scip, vars, ones, nvars, -inf, upbound - 1, downchild), TERM); + SCIP_CALL_TERMINATE(retcode, SCIPbranchGUB_add_child(scip, vars, ones, nvars, upbound + 1, inf, upchild), TERM); + } else { + SCIP_CALL_TERMINATE(retcode, SCIPbranchGUB_add_child(scip, vars, ones, nvars, -inf, downbound, downchild), TERM); + SCIP_CALL_TERMINATE(retcode, SCIPbranchGUB_add_child(scip, vars, ones, nvars, upbound, inf, upchild), TERM); + } + +TERM: + SCIPfreeBufferArray(scip, &ones); + return retcode; +} diff --git a/libecole/src/scip/gub-branch.hpp b/libecole/src/scip/gub-branch.hpp new file mode 100644 index 000000000..bc2bceb04 --- /dev/null +++ b/libecole/src/scip/gub-branch.hpp @@ -0,0 +1,6 @@ +#pragma once + +#include + +SCIP_RETCODE +SCIPbranchGUB(SCIP* scip, SCIP_VAR** vars, int nvars, SCIP_NODE** downchild, SCIP_NODE** eqchild, SCIP_NODE** upchild); diff --git a/libecole/src/scip/scimpl.cpp b/libecole/src/scip/scimpl.cpp index bc164e4fb..35164abb5 100644 --- a/libecole/src/scip/scimpl.cpp +++ b/libecole/src/scip/scimpl.cpp @@ -7,9 +7,10 @@ #include #include "ecole/scip/scimpl.hpp" - #include "ecole/scip/utils.hpp" +#include "scip/gub-branch.hpp" + namespace ecole::scip { /****************************************** @@ -96,26 +97,18 @@ void Scimpl::solve_iter() { m_controller->wait_thread(); } -SCIP_RETCODE -SCIPbranchGUB(SCIP* scip, SCIP_VAR** vars, int nvars, SCIP_NODE** downchild, SCIP_NODE** eqchild, SCIP_NODE** upchild) { - if (nvars <= 0) { - return SCIP_ERROR; - } - if (nvars == 1) { - SCIP_CALL(SCIPbranchVar(scip, *vars, downchild, eqchild, upchild)); - } else { - // Our implementation - return SCIP_NOTIMPLEMENTED; - } - return SCIP_OKAY; -} - void scip::Scimpl::solve_iter_branch(nonstd::span vars) { assert(std::none_of(vars.begin(), vars.end(), [](auto* var) { return var == nullptr; })); m_controller->resume_thread([vars](SCIP* scip_ptr, SCIP_RESULT* result) { - if (vars.empty()) { + switch (vars.size()) { + case 0: *result = SCIP_DIDNOTRUN; - } else { + break; + case 1: + SCIP_CALL(SCIPbranchVar(scip_ptr, const_cast(vars.front()), nullptr, nullptr, nullptr)); + *result = SCIP_BRANCHED; + break; + default: SCIP_CALL(SCIPbranchGUB( scip_ptr, const_cast(vars.data()), static_cast(vars.size()), nullptr, nullptr, nullptr)); *result = SCIP_BRANCHED; From 9375ee424452a3ded75e9c564dc3d36e07c229ad Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Fri, 7 May 2021 12:27:06 -0400 Subject: [PATCH 03/20] Add estimate to GUB nodes --- libecole/src/scip/gub-branch.cpp | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/libecole/src/scip/gub-branch.cpp b/libecole/src/scip/gub-branch.cpp index e2d897a5e..345b97def 100644 --- a/libecole/src/scip/gub-branch.cpp +++ b/libecole/src/scip/gub-branch.cpp @@ -1,3 +1,5 @@ +#include + #include #include @@ -46,6 +48,8 @@ static SCIP_RETCODE SCIPbranchGUB_validate(SCIP* scip, SCIP_VAR** vars, int nvar static SCIP_RETCODE SCIPbranchGUB_add_child( SCIP* scip, + SCIP_Real priority, + SCIP_Real estimate, SCIP_VAR** vars, SCIP_Real* ones, int nvars, @@ -54,7 +58,7 @@ static SCIP_RETCODE SCIPbranchGUB_add_child( SCIP_NODE** node_out) { // TODO decide on priorities SCIP_NODE* node = nullptr; - SCIP_CALL(SCIPcreateChild(scip, &node, 1., 0.)); + SCIP_CALL(SCIPcreateChild(scip, &node, priority, estimate)); auto name = fmt::format("branching-{}", SCIPnodeGetNumber(node)); SCIP_CONS* cons = nullptr; SCIP_CALL(SCIPcreateConsBasicLinear(scip, &cons, name.c_str(), nvars, vars, ones, lhs, rhs)); @@ -92,13 +96,19 @@ SCIPbranchGUB(SCIP* scip, SCIP_VAR** vars, int nvars, SCIP_NODE** downchild, SCI SCIP_Real const downbound = SCIPfeasFloor(scip, pseudo_sol_sum); SCIP_Real const upbound = SCIPfeasCeil(scip, pseudo_sol_sum); + SCIP_Real const estimate = SCIPnodeGetLowerbound(SCIPgetCurrentNode(scip)); if (SCIPisEQ(scip, downbound, upbound)) { - SCIP_CALL_TERMINATE(retcode, SCIPbranchGUB_add_child(scip, vars, ones, nvars, upbound, upbound, eqchild), TERM); - SCIP_CALL_TERMINATE(retcode, SCIPbranchGUB_add_child(scip, vars, ones, nvars, -inf, upbound - 1, downchild), TERM); - SCIP_CALL_TERMINATE(retcode, SCIPbranchGUB_add_child(scip, vars, ones, nvars, upbound + 1, inf, upchild), TERM); + SCIP_CALL_TERMINATE( + retcode, SCIPbranchGUB_add_child(scip, 1, estimate, vars, ones, nvars, upbound, upbound, eqchild), TERM); + SCIP_CALL_TERMINATE( + retcode, SCIPbranchGUB_add_child(scip, 1, estimate, vars, ones, nvars, -inf, upbound - 1, downchild), TERM); + SCIP_CALL_TERMINATE( + retcode, SCIPbranchGUB_add_child(scip, 1, estimate, vars, ones, nvars, upbound + 1, inf, upchild), TERM); } else { - SCIP_CALL_TERMINATE(retcode, SCIPbranchGUB_add_child(scip, vars, ones, nvars, -inf, downbound, downchild), TERM); - SCIP_CALL_TERMINATE(retcode, SCIPbranchGUB_add_child(scip, vars, ones, nvars, upbound, inf, upchild), TERM); + SCIP_CALL_TERMINATE( + retcode, SCIPbranchGUB_add_child(scip, 1, estimate, vars, ones, nvars, -inf, downbound, downchild), TERM); + SCIP_CALL_TERMINATE( + retcode, SCIPbranchGUB_add_child(scip, 1, estimate, vars, ones, nvars, upbound, inf, upchild), TERM); } TERM: From 1c85fd05c1eee156eaedc3bcf69b42929bb10777 Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Fri, 7 May 2021 12:33:08 -0400 Subject: [PATCH 04/20] Fix GUB constraint not released --- libecole/src/scip/gub-branch.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/libecole/src/scip/gub-branch.cpp b/libecole/src/scip/gub-branch.cpp index 345b97def..0c8e0f9f5 100644 --- a/libecole/src/scip/gub-branch.cpp +++ b/libecole/src/scip/gub-branch.cpp @@ -56,17 +56,20 @@ static SCIP_RETCODE SCIPbranchGUB_add_child( SCIP_Real lhs, SCIP_Real rhs, SCIP_NODE** node_out) { - // TODO decide on priorities SCIP_NODE* node = nullptr; SCIP_CALL(SCIPcreateChild(scip, &node, priority, estimate)); auto name = fmt::format("branching-{}", SCIPnodeGetNumber(node)); SCIP_CONS* cons = nullptr; SCIP_CALL(SCIPcreateConsBasicLinear(scip, &cons, name.c_str(), nvars, vars, ones, lhs, rhs)); - SCIP_CALL(SCIPaddConsNode(scip, node, cons, nullptr)); + SCIP_RETCODE retcode = SCIP_OKAY; + SCIP_CALL_TERMINATE(retcode, SCIPaddConsNode(scip, node, cons, nullptr), TERM); if (node_out != nullptr) { *node_out = node; } - return SCIP_OKAY; + +TERM: + SCIP_CALL(SCIPreleaseCons(scip, &cons)); + return retcode; } SCIP_RETCODE From 09fd2caef4927eb77b3c98ab6bad4ea5299395cd Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Tue, 11 May 2021 13:56:12 -0400 Subject: [PATCH 05/20] Fix constraint in GUB --- libecole/src/scip/gub-branch.cpp | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/libecole/src/scip/gub-branch.cpp b/libecole/src/scip/gub-branch.cpp index 0c8e0f9f5..13a35dac4 100644 --- a/libecole/src/scip/gub-branch.cpp +++ b/libecole/src/scip/gub-branch.cpp @@ -60,7 +60,25 @@ static SCIP_RETCODE SCIPbranchGUB_add_child( SCIP_CALL(SCIPcreateChild(scip, &node, priority, estimate)); auto name = fmt::format("branching-{}", SCIPnodeGetNumber(node)); SCIP_CONS* cons = nullptr; - SCIP_CALL(SCIPcreateConsBasicLinear(scip, &cons, name.c_str(), nvars, vars, ones, lhs, rhs)); + SCIP_CALL(SCIPcreateConsLinear( + scip, + &cons, + name.c_str(), + nvars, + vars, + ones, + lhs, + rhs, + /*initial=*/TRUE, + /*separate=*/TRUE, + /*enforce=*/FALSE, + /*check=*/FALSE, + /*propagate=*/TRUE, + /*local=*/TRUE, + /*modifiable=*/FALSE, + /*dynamic=*/FALSE, + /*removable=*/FALSE, + /*stickingatnode=*/TRUE)); SCIP_RETCODE retcode = SCIP_OKAY; SCIP_CALL_TERMINATE(retcode, SCIPaddConsNode(scip, node, cons, nullptr), TERM); if (node_out != nullptr) { From 3a9ad8869f0bb2a4bdeb70aeff99d23c1854acf3 Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Fri, 21 May 2021 12:05:29 -0400 Subject: [PATCH 06/20] Add tests for iterative branching --- libecole/tests/src/scip/test-model.cpp | 35 ++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/libecole/tests/src/scip/test-model.cpp b/libecole/tests/src/scip/test-model.cpp index 9ad0cfe16..571c7938b 100644 --- a/libecole/tests/src/scip/test-model.cpp +++ b/libecole/tests/src/scip/test-model.cpp @@ -1,10 +1,12 @@ #include #include +#include #include #include #include +#include "ecole/random.hpp" #include "ecole/scip/exception.hpp" #include "ecole/scip/model.hpp" @@ -164,3 +166,36 @@ TEST_CASE("Map parameter management", "[scip]") { REQUIRE(vals[int_param] == scip::Param{model.get_param(int_param)}); } } + +TEST_CASE("Iterative branching", "[scip][slow]") { + auto model = get_model(); + model.solve_iter(); + + SECTION("Branch on a given variable") { + while (!model.solve_iter_is_done()) { + auto const cands = model.lp_branch_cands(); + REQUIRE_FALSE(cands.empty()); + model.solve_iter_branch(cands[0]); + } + } + + SECTION("Branch on SCIP default") { + while (!model.solve_iter_is_done()) { + model.solve_iter_branch(); + } + } + + SECTION("Branch on multiple candidates") { + // This is not a great way to select variables because if their sum is integer, an exeception will + // be raised. + auto rng = ecole::spawn_random_engine(); + while (!model.solve_iter_is_done()) { + auto const cands = model.lp_branch_cands(); + REQUIRE_FALSE(cands.empty()); + auto choice = std::uniform_int_distribution{0, cands.size() - 1}; + model.solve_iter_branch(cands[choice(rng)]); + } + } + + REQUIRE(model.is_solved()); +} From 8c450c36a9ebb77b04f4aac1dc6a19839ed94432 Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Fri, 21 May 2021 14:24:29 -0400 Subject: [PATCH 07/20] Throw on integer sum in SCIPbranchGUB --- libecole/src/scip/gub-branch.cpp | 94 ++++++++++++++------------------ libecole/src/scip/gub-branch.hpp | 2 +- libecole/src/scip/scimpl.cpp | 4 +- 3 files changed, 44 insertions(+), 56 deletions(-) diff --git a/libecole/src/scip/gub-branch.cpp b/libecole/src/scip/gub-branch.cpp index 13a35dac4..d10531938 100644 --- a/libecole/src/scip/gub-branch.cpp +++ b/libecole/src/scip/gub-branch.cpp @@ -12,40 +12,6 @@ #include "scip/gub-branch.hpp" -static SCIP_RETCODE SCIPbranchGUB_validate(SCIP* scip, SCIP_VAR** vars, int nvars) { - assert(scip != nullptr); - if (SCIPgetStage(scip) != SCIP_STAGE_SOLVING) { - SCIPerrorMessage("cannot branch when not solving\n"); - return SCIP_INVALIDCALL; - } - - if (nvars <= 0) { - SCIPerrorMessage("cannot branch on empty variable set\n"); - return SCIP_INVALIDDATA; - } - - SCIP_VAR const* const* const vars_end = vars + nvars; - for (SCIP_VAR** var_iter = vars; var_iter < vars_end; ++var_iter) { - // assert((*var_iter)->scip == scip); FIXME only in SCIP debug builds - assert(SCIPvarIsActive(*var_iter)); - assert(SCIPvarGetProbindex(*var_iter) >= 0); - if (SCIPvarGetType(*var_iter) == SCIP_VARTYPE_CONTINUOUS) { - SCIPerrorMessage("cannot branch on constraint containing continuous variable <%s>\n", SCIPvarGetName(*var_iter)); - return SCIP_INVALIDDATA; - } - if (SCIPisEQ(scip, SCIPvarGetLbLocal(*var_iter), SCIPvarGetUbLocal(*var_iter))) { - SCIPerrorMessage( - "cannot branch on constraint containing variable <%s> with fixed domain [%.15g,%.15g]\n", - SCIPvarGetName(*var_iter), - SCIPvarGetLbLocal(*var_iter), - SCIPvarGetUbLocal(*var_iter)); - return SCIP_INVALIDDATA; - } - } - - return SCIP_OKAY; -} - static SCIP_RETCODE SCIPbranchGUB_add_child( SCIP* scip, SCIP_Real priority, @@ -91,22 +57,56 @@ static SCIP_RETCODE SCIPbranchGUB_add_child( } SCIP_RETCODE -SCIPbranchGUB(SCIP* scip, SCIP_VAR** vars, int nvars, SCIP_NODE** downchild, SCIP_NODE** eqchild, SCIP_NODE** upchild) { - SCIPbranchGUB_validate(scip, vars, nvars); +SCIPbranchGUB(SCIP* scip, SCIP_VAR** vars, int nvars, SCIP_NODE** downchild, SCIP_NODE** upchild) { + /* Check input parameters */ + assert(scip != nullptr); + if (SCIPgetStage(scip) != SCIP_STAGE_SOLVING) { + SCIPerrorMessage("cannot branch when not solving\n"); + return SCIP_INVALIDCALL; + } + if (nvars <= 0) { + SCIPerrorMessage("cannot branch on empty variable set\n"); + return SCIP_INVALIDDATA; + } + /* Check individual variables and compute the sum of their LP relaxation value */ SCIP_Real pseudo_sol_sum = 0.; SCIP_VAR const* const* const vars_end = vars + nvars; for (SCIP_VAR** var_iter = vars; var_iter < vars_end; ++var_iter) { + /* Validate that variables are integer, not fixed */ + assert(SCIPvarIsActive(*var_iter)); + assert(SCIPvarGetProbindex(*var_iter) >= 0); + if (SCIPvarGetType(*var_iter) == SCIP_VARTYPE_CONTINUOUS) { + SCIPerrorMessage("cannot branch on constraint containing continuous variable <%s>\n", SCIPvarGetName(*var_iter)); + return SCIP_INVALIDDATA; + } + if (SCIPisEQ(scip, SCIPvarGetLbLocal(*var_iter), SCIPvarGetUbLocal(*var_iter))) { + SCIPerrorMessage( + "cannot branch on constraint containing variable <%s> with fixed domain [%.15g,%.15g]\n", + SCIPvarGetName(*var_iter), + SCIPvarGetLbLocal(*var_iter), + SCIPvarGetUbLocal(*var_iter)); + return SCIP_INVALIDDATA; + } + SCIP_Real val = SCIPvarGetSol(*var_iter, SCIPhasCurrentNodeLP(scip)); /* avoid branching on infinite values in pseudo solution */ if (SCIPisInfinity(scip, -val) || SCIPisInfinity(scip, val)) { - // FIXME not sure what to do here + SCIPerrorMessage("cannot branch on variables containing infinite values"); return SCIP_INVALIDDATA; } pseudo_sol_sum += val; } + SCIP_Real const downbound = SCIPfeasFloor(scip, pseudo_sol_sum); + SCIP_Real const upbound = SCIPfeasCeil(scip, pseudo_sol_sum); + SCIP_Real const estimate = SCIPnodeGetLowerbound(SCIPgetCurrentNode(scip)); + if (SCIPisEQ(scip, downbound, upbound)) { + SCIPerrorMessage("cannot branch on a variables whose sum of LP solution value is integer"); + return SCIP_INVALIDDATA; + } + SCIP_Real inf = SCIPinfinity(scip); SCIP_Real* ones = nullptr; SCIP_Retcode retcode = SCIP_OKAY; @@ -115,22 +115,10 @@ SCIPbranchGUB(SCIP* scip, SCIP_VAR** vars, int nvars, SCIP_NODE** downchild, SCI ones[i] = 1.; } - SCIP_Real const downbound = SCIPfeasFloor(scip, pseudo_sol_sum); - SCIP_Real const upbound = SCIPfeasCeil(scip, pseudo_sol_sum); - SCIP_Real const estimate = SCIPnodeGetLowerbound(SCIPgetCurrentNode(scip)); - if (SCIPisEQ(scip, downbound, upbound)) { - SCIP_CALL_TERMINATE( - retcode, SCIPbranchGUB_add_child(scip, 1, estimate, vars, ones, nvars, upbound, upbound, eqchild), TERM); - SCIP_CALL_TERMINATE( - retcode, SCIPbranchGUB_add_child(scip, 1, estimate, vars, ones, nvars, -inf, upbound - 1, downchild), TERM); - SCIP_CALL_TERMINATE( - retcode, SCIPbranchGUB_add_child(scip, 1, estimate, vars, ones, nvars, upbound + 1, inf, upchild), TERM); - } else { - SCIP_CALL_TERMINATE( - retcode, SCIPbranchGUB_add_child(scip, 1, estimate, vars, ones, nvars, -inf, downbound, downchild), TERM); - SCIP_CALL_TERMINATE( - retcode, SCIPbranchGUB_add_child(scip, 1, estimate, vars, ones, nvars, upbound, inf, upchild), TERM); - } + SCIP_CALL_TERMINATE( + retcode, SCIPbranchGUB_add_child(scip, 1, estimate, vars, ones, nvars, -inf, downbound, downchild), TERM); + SCIP_CALL_TERMINATE( + retcode, SCIPbranchGUB_add_child(scip, 1, estimate, vars, ones, nvars, upbound, inf, upchild), TERM); TERM: SCIPfreeBufferArray(scip, &ones); diff --git a/libecole/src/scip/gub-branch.hpp b/libecole/src/scip/gub-branch.hpp index bc2bceb04..d2a631d5d 100644 --- a/libecole/src/scip/gub-branch.hpp +++ b/libecole/src/scip/gub-branch.hpp @@ -3,4 +3,4 @@ #include SCIP_RETCODE -SCIPbranchGUB(SCIP* scip, SCIP_VAR** vars, int nvars, SCIP_NODE** downchild, SCIP_NODE** eqchild, SCIP_NODE** upchild); +SCIPbranchGUB(SCIP* scip, SCIP_VAR** vars, int nvars, SCIP_NODE** downchild, SCIP_NODE** upchild); diff --git a/libecole/src/scip/scimpl.cpp b/libecole/src/scip/scimpl.cpp index 35164abb5..7ed609eb6 100644 --- a/libecole/src/scip/scimpl.cpp +++ b/libecole/src/scip/scimpl.cpp @@ -109,8 +109,8 @@ void scip::Scimpl::solve_iter_branch(nonstd::span vars) { *result = SCIP_BRANCHED; break; default: - SCIP_CALL(SCIPbranchGUB( - scip_ptr, const_cast(vars.data()), static_cast(vars.size()), nullptr, nullptr, nullptr)); + SCIP_CALL( + SCIPbranchGUB(scip_ptr, const_cast(vars.data()), static_cast(vars.size()), nullptr, nullptr)); *result = SCIP_BRANCHED; } return SCIP_OKAY; From b9c6ff6d3d8f75958fcac6fa786badfe8a4cb48d Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Fri, 21 May 2021 15:40:58 -0400 Subject: [PATCH 08/20] Move branching logic to dynamics --- libecole/include/ecole/scip/model.hpp | 4 +--- libecole/include/ecole/scip/scimpl.hpp | 2 +- libecole/src/dynamics/branching.cpp | 5 +++-- libecole/src/scip/model.cpp | 15 ++------------- libecole/src/scip/scimpl.cpp | 19 +++---------------- libecole/tests/src/scip/test-model.cpp | 20 +++++--------------- 6 files changed, 15 insertions(+), 50 deletions(-) diff --git a/libecole/include/ecole/scip/model.hpp b/libecole/include/ecole/scip/model.hpp index 90f3ac2a9..679a3cfbf 100644 --- a/libecole/include/ecole/scip/model.hpp +++ b/libecole/include/ecole/scip/model.hpp @@ -124,9 +124,7 @@ class Model { [[nodiscard]] bool is_solved() const noexcept; void solve_iter(); - void solve_iter_branch(nonstd::span vars); - void solve_iter_branch(SCIP_VAR const* var); - void solve_iter_branch(); + void solve_iter_branch(SCIP_RESULT result); void solve_iter_stop(); [[nodiscard]] bool solve_iter_is_done(); diff --git a/libecole/include/ecole/scip/scimpl.hpp b/libecole/include/ecole/scip/scimpl.hpp index 8a6943b56..7924eab98 100644 --- a/libecole/include/ecole/scip/scimpl.hpp +++ b/libecole/include/ecole/scip/scimpl.hpp @@ -23,7 +23,7 @@ class Scimpl { Scimpl copy_orig(); void solve_iter(); - void solve_iter_branch(nonstd::span vars); + void solve_iter_branch(SCIP_RESULT result); void solve_iter_stop(); bool solve_iter_is_done(); diff --git a/libecole/src/dynamics/branching.cpp b/libecole/src/dynamics/branching.cpp index e976c819e..139d98ff6 100644 --- a/libecole/src/dynamics/branching.cpp +++ b/libecole/src/dynamics/branching.cpp @@ -7,7 +7,6 @@ #include "ecole/dynamics/branching.hpp" #include "ecole/exception.hpp" #include "ecole/scip/model.hpp" - #include "ecole/scip/utils.hpp" namespace ecole::dynamics { @@ -48,7 +47,9 @@ auto BranchingDynamics::step_dynamics(scip::Model& model, std::size_t const& act if (action >= lp_cols.size()) { throw Exception{"Branching index is larger than the number of columns."}; } - model.solve_iter_branch(SCIPcolGetVar(lp_cols[action])); + auto* const var = SCIPcolGetVar(lp_cols[action]); + scip::call(SCIPbranchVar, model.get_scip_ptr(), var, nullptr, nullptr, nullptr); + model.solve_iter_branch(SCIP_BRANCHED); auto const done = model.solve_iter_is_done(); if (done) { diff --git a/libecole/src/scip/model.cpp b/libecole/src/scip/model.cpp index 0828e26fe..b8d5b8887 100644 --- a/libecole/src/scip/model.cpp +++ b/libecole/src/scip/model.cpp @@ -201,19 +201,8 @@ void Model::solve_iter() { scimpl->solve_iter(); } -void Model::solve_iter_branch() { - return solve_iter_branch(nonstd::span{}); -} - -void Model::solve_iter_branch(SCIP_VAR const* var) { - return solve_iter_branch({&var, 1}); -} - -void Model::solve_iter_branch(nonstd::span vars) { - if (std::any_of(vars.begin(), vars.end(), [](auto* var) { return var == nullptr; })) { - throw std::invalid_argument{"All variables must be non null."}; - } - scimpl->solve_iter_branch(vars); +void Model::solve_iter_branch(SCIP_RESULT result) { + scimpl->solve_iter_branch(result); } void Model::solve_iter_stop() { diff --git a/libecole/src/scip/scimpl.cpp b/libecole/src/scip/scimpl.cpp index 7ed609eb6..6bc30d726 100644 --- a/libecole/src/scip/scimpl.cpp +++ b/libecole/src/scip/scimpl.cpp @@ -97,22 +97,9 @@ void Scimpl::solve_iter() { m_controller->wait_thread(); } -void scip::Scimpl::solve_iter_branch(nonstd::span vars) { - assert(std::none_of(vars.begin(), vars.end(), [](auto* var) { return var == nullptr; })); - m_controller->resume_thread([vars](SCIP* scip_ptr, SCIP_RESULT* result) { - switch (vars.size()) { - case 0: - *result = SCIP_DIDNOTRUN; - break; - case 1: - SCIP_CALL(SCIPbranchVar(scip_ptr, const_cast(vars.front()), nullptr, nullptr, nullptr)); - *result = SCIP_BRANCHED; - break; - default: - SCIP_CALL( - SCIPbranchGUB(scip_ptr, const_cast(vars.data()), static_cast(vars.size()), nullptr, nullptr)); - *result = SCIP_BRANCHED; - } +void scip::Scimpl::solve_iter_branch(SCIP_RESULT result) { + m_controller->resume_thread([result](SCIP* /* scip */, SCIP_RESULT* final_result) { + *final_result = result; return SCIP_OKAY; }); m_controller->wait_thread(); diff --git a/libecole/tests/src/scip/test-model.cpp b/libecole/tests/src/scip/test-model.cpp index 571c7938b..01cc23bd9 100644 --- a/libecole/tests/src/scip/test-model.cpp +++ b/libecole/tests/src/scip/test-model.cpp @@ -9,6 +9,7 @@ #include "ecole/random.hpp" #include "ecole/scip/exception.hpp" #include "ecole/scip/model.hpp" +#include "ecole/scip/utils.hpp" #include "conftest.hpp" @@ -171,29 +172,18 @@ TEST_CASE("Iterative branching", "[scip][slow]") { auto model = get_model(); model.solve_iter(); - SECTION("Branch on a given variable") { + SECTION("Branch outside of callback") { while (!model.solve_iter_is_done()) { auto const cands = model.lp_branch_cands(); REQUIRE_FALSE(cands.empty()); - model.solve_iter_branch(cands[0]); + scip::call(SCIPbranchVar, model.get_scip_ptr(), cands[0], nullptr, nullptr, nullptr); + model.solve_iter_branch(SCIP_BRANCHED); } } SECTION("Branch on SCIP default") { while (!model.solve_iter_is_done()) { - model.solve_iter_branch(); - } - } - - SECTION("Branch on multiple candidates") { - // This is not a great way to select variables because if their sum is integer, an exeception will - // be raised. - auto rng = ecole::spawn_random_engine(); - while (!model.solve_iter_is_done()) { - auto const cands = model.lp_branch_cands(); - REQUIRE_FALSE(cands.empty()); - auto choice = std::uniform_int_distribution{0, cands.size() - 1}; - model.solve_iter_branch(cands[choice(rng)]); + model.solve_iter_branch(SCIP_DIDNOTRUN); } } From d70d3ea7c71f335b9a07ea75f987aa4c1cb0b6dd Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Tue, 25 May 2021 12:36:57 -0400 Subject: [PATCH 09/20] Add GUB branching dynamics --- libecole/CMakeLists.txt | 2 +- .../include/ecole/dynamics/branching-gub.hpp | 24 ++++++ .../branching-gub.cpp} | 78 ++++++++++++++++++- libecole/src/scip/gub-branch.hpp | 6 -- libecole/src/scip/scimpl.cpp | 2 - 5 files changed, 100 insertions(+), 12 deletions(-) create mode 100644 libecole/include/ecole/dynamics/branching-gub.hpp rename libecole/src/{scip/gub-branch.cpp => dynamics/branching-gub.cpp} (60%) delete mode 100644 libecole/src/scip/gub-branch.hpp diff --git a/libecole/CMakeLists.txt b/libecole/CMakeLists.txt index 41b844c0a..351eaa793 100644 --- a/libecole/CMakeLists.txt +++ b/libecole/CMakeLists.txt @@ -15,7 +15,6 @@ add_library( src/scip/row.cpp src/scip/col.cpp src/scip/exception.cpp - src/scip/gub-branch.cpp # Temporary src/instance/files.cpp src/instance/set-cover.cpp @@ -36,6 +35,7 @@ add_library( src/observation/pseudocosts.cpp src/dynamics/branching.cpp + src/dynamics/branching-gub.cpp src/dynamics/configuring.cpp ) set_target_properties(ecole-lib PROPERTIES OUTPUT_NAME ecole) diff --git a/libecole/include/ecole/dynamics/branching-gub.hpp b/libecole/include/ecole/dynamics/branching-gub.hpp new file mode 100644 index 000000000..b94ff1ebb --- /dev/null +++ b/libecole/include/ecole/dynamics/branching-gub.hpp @@ -0,0 +1,24 @@ +#pragma once + +#include +#include + +#include +#include + +#include "ecole/dynamics/dynamics.hpp" + +namespace ecole::dynamics { + +class BranchingGUBDynamics : + public EnvironmentDynamics, std::optional>> { +public: + using Action = nonstd::span; + using ActionSet = std::optional>; + + std::tuple reset_dynamics(scip::Model& model) override; + + std::tuple step_dynamics(scip::Model& model, Action const& var_idx) override; +}; + +} // namespace ecole::dynamics diff --git a/libecole/src/scip/gub-branch.cpp b/libecole/src/dynamics/branching-gub.cpp similarity index 60% rename from libecole/src/scip/gub-branch.cpp rename to libecole/src/dynamics/branching-gub.cpp index d10531938..47cf4afa8 100644 --- a/libecole/src/scip/gub-branch.cpp +++ b/libecole/src/dynamics/branching-gub.cpp @@ -1,3 +1,7 @@ +/*********************************************************** + * Implementation of SCIPbranchGUB (aim to merge in SCIP) * + ***********************************************************/ + #include #include @@ -10,9 +14,9 @@ #include #include -#include "scip/gub-branch.hpp" +namespace { -static SCIP_RETCODE SCIPbranchGUB_add_child( +SCIP_RETCODE SCIPbranchGUB_add_child( SCIP* scip, SCIP_Real priority, SCIP_Real estimate, @@ -107,7 +111,7 @@ SCIPbranchGUB(SCIP* scip, SCIP_VAR** vars, int nvars, SCIP_NODE** downchild, SCI return SCIP_INVALIDDATA; } - SCIP_Real inf = SCIPinfinity(scip); + SCIP_Real const inf = SCIPinfinity(scip); SCIP_Real* ones = nullptr; SCIP_Retcode retcode = SCIP_OKAY; SCIP_CALL(SCIPallocBufferArray(scip, &ones, nvars)); @@ -124,3 +128,71 @@ SCIPbranchGUB(SCIP* scip, SCIP_VAR** vars, int nvars, SCIP_NODE** downchild, SCI SCIPfreeBufferArray(scip, &ones); return retcode; } + +} // namespace + +/******************************************** + * Implementation of BranchingGUBDynamics * + ********************************************/ + +#include +#include +#include + +#include + +#include "ecole/dynamics/branching-gub.hpp" +#include "ecole/scip/model.hpp" +#include "ecole/scip/utils.hpp" + +namespace ecole::dynamics { + +namespace { + +std::optional> action_set(scip::Model const& model) { + if (model.get_stage() != SCIP_STAGE_SOLVING) { + return {}; + } + auto const branch_cands = model.lp_branch_cands(); + auto branch_cols = xt::xtensor::from_shape({branch_cands.size()}); + auto const var_to_idx = [](auto const var) { return SCIPcolGetLPPos(SCIPvarGetCol(var)); }; + std::transform(branch_cands.begin(), branch_cands.end(), branch_cols.begin(), var_to_idx); + + assert(branch_cols.size() > 0); + return branch_cols; +} + +} // namespace + +auto BranchingGUBDynamics::reset_dynamics(scip::Model& model) -> std::tuple { + model.solve_iter(); + if (model.solve_iter_is_done()) { + return {true, {}}; + } + return {false, action_set(model)}; +} + +auto BranchingGUBDynamics::step_dynamics(scip::Model& model, Action const& var_idx) -> std::tuple { + auto const lp_cols = model.lp_columns(); + + // Check that input indices are within range + auto const is_out_of_bounds = [size = lp_cols.size()](auto idx) { return idx >= size; }; + if (std::any_of(var_idx.begin(), var_idx.end(), is_out_of_bounds)) { + throw std::invalid_argument{"Branching index is larger than the number of columns."}; + } + + // Get variables associated with indices + auto vars = std::vector(var_idx.size()); + auto const idx_to_var = [lp_cols](auto idx) { return SCIPcolGetVar(lp_cols[idx]); }; + std::transform(var_idx.begin(), var_idx.end(), vars.begin(), idx_to_var); + + scip::call(SCIPbranchGUB, model.get_scip_ptr(), vars.data(), static_cast(vars.size()), nullptr, nullptr); + model.solve_iter_branch(SCIP_BRANCHED); + + if (model.solve_iter_is_done()) { + return {true, {}}; + } + return {false, action_set(model)}; +} + +} // namespace ecole::dynamics diff --git a/libecole/src/scip/gub-branch.hpp b/libecole/src/scip/gub-branch.hpp deleted file mode 100644 index d2a631d5d..000000000 --- a/libecole/src/scip/gub-branch.hpp +++ /dev/null @@ -1,6 +0,0 @@ -#pragma once - -#include - -SCIP_RETCODE -SCIPbranchGUB(SCIP* scip, SCIP_VAR** vars, int nvars, SCIP_NODE** downchild, SCIP_NODE** upchild); diff --git a/libecole/src/scip/scimpl.cpp b/libecole/src/scip/scimpl.cpp index 6bc30d726..1cccc7a7e 100644 --- a/libecole/src/scip/scimpl.cpp +++ b/libecole/src/scip/scimpl.cpp @@ -9,8 +9,6 @@ #include "ecole/scip/scimpl.hpp" #include "ecole/scip/utils.hpp" -#include "scip/gub-branch.hpp" - namespace ecole::scip { /****************************************** From 5d1979a1f595aab7f5c230fe7a225542ee72c8bc Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Tue, 25 May 2021 12:40:07 -0400 Subject: [PATCH 10/20] Cleanup BranchingDynamics --- libecole/include/ecole/dynamics/branching.hpp | 2 +- libecole/src/dynamics/branching.cpp | 33 ++++++++----------- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/libecole/include/ecole/dynamics/branching.hpp b/libecole/include/ecole/dynamics/branching.hpp index 6459a48ed..e2c1e9798 100644 --- a/libecole/include/ecole/dynamics/branching.hpp +++ b/libecole/include/ecole/dynamics/branching.hpp @@ -19,7 +19,7 @@ class BranchingDynamics : public EnvironmentDynamics reset_dynamics(scip::Model& model) override; - std::tuple step_dynamics(scip::Model& model, std::size_t const& action) override; + std::tuple step_dynamics(scip::Model& model, std::size_t const& var_idx) override; }; } // namespace ecole::dynamics diff --git a/libecole/src/dynamics/branching.cpp b/libecole/src/dynamics/branching.cpp index 139d98ff6..53fece27d 100644 --- a/libecole/src/dynamics/branching.cpp +++ b/libecole/src/dynamics/branching.cpp @@ -1,11 +1,9 @@ #include -#include -#include +#include #include #include "ecole/dynamics/branching.hpp" -#include "ecole/exception.hpp" #include "ecole/scip/model.hpp" #include "ecole/scip/utils.hpp" @@ -21,11 +19,8 @@ std::optional> action_set(scip::Model const& model, } auto const branch_cands = pseudo ? model.pseudo_branch_cands() : model.lp_branch_cands(); auto branch_cols = xt::xtensor::from_shape({branch_cands.size()}); - std::transform( // - branch_cands.begin(), - branch_cands.end(), - branch_cols.begin(), - [](auto const var) { return SCIPcolGetLPPos(SCIPvarGetCol(var)); }); + auto const var_to_idx = [](auto const var) { return SCIPcolGetLPPos(SCIPvarGetCol(var)); }; + std::transform(branch_cands.begin(), branch_cands.end(), branch_cols.begin(), var_to_idx); assert(branch_cols.size() > 0); return branch_cols; @@ -35,27 +30,25 @@ std::optional> action_set(scip::Model const& model, auto BranchingDynamics::reset_dynamics(scip::Model& model) -> std::tuple { model.solve_iter(); - auto const done = model.solve_iter_is_done(); - if (done) { - return {done, {}}; + if (model.solve_iter_is_done()) { + return {true, {}}; } - return {done, action_set(model, pseudo_candidates)}; + return {false, action_set(model, pseudo_candidates)}; } -auto BranchingDynamics::step_dynamics(scip::Model& model, std::size_t const& action) -> std::tuple { +auto BranchingDynamics::step_dynamics(scip::Model& model, std::size_t const& var_idx) -> std::tuple { auto const lp_cols = model.lp_columns(); - if (action >= lp_cols.size()) { - throw Exception{"Branching index is larger than the number of columns."}; + if (var_idx >= lp_cols.size()) { + throw std::invalid_argument{"Branching index is larger than the number of columns."}; } - auto* const var = SCIPcolGetVar(lp_cols[action]); + auto* const var = SCIPcolGetVar(lp_cols[var_idx]); scip::call(SCIPbranchVar, model.get_scip_ptr(), var, nullptr, nullptr, nullptr); model.solve_iter_branch(SCIP_BRANCHED); - auto const done = model.solve_iter_is_done(); - if (done) { - return {done, {}}; + if (model.solve_iter_is_done()) { + return {true, {}}; } - return {done, action_set(model, pseudo_candidates)}; + return {false, action_set(model, pseudo_candidates)}; } } // namespace ecole::dynamics From da0314146afbd641d181916d934000c14f8f215a Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Tue, 25 May 2021 17:05:44 -0400 Subject: [PATCH 11/20] Refactor dynamics::unit_test to take model in policy --- libecole/tests/src/dynamics/test-branching.cpp | 2 +- libecole/tests/src/dynamics/test-configuring.cpp | 3 ++- libecole/tests/src/dynamics/unit-tests.hpp | 8 ++++---- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/libecole/tests/src/dynamics/test-branching.cpp b/libecole/tests/src/dynamics/test-branching.cpp index 4e2616edc..f381ec23d 100644 --- a/libecole/tests/src/dynamics/test-branching.cpp +++ b/libecole/tests/src/dynamics/test-branching.cpp @@ -16,7 +16,7 @@ using namespace ecole; TEST_CASE("BranchingDynamics unit tests", "[unit][dynamics]") { bool const pseudo_candidates = GENERATE(true, false); bool const branch_first = GENERATE(true, false); - auto const policy = [branch_first](auto const& action_set) { + auto const policy = [branch_first](auto const& action_set, auto const& /*model*/) { auto const branch_idx = branch_first ? 0 : action_set.value().size() - 1; return action_set.value()[branch_idx]; }; diff --git a/libecole/tests/src/dynamics/test-configuring.cpp b/libecole/tests/src/dynamics/test-configuring.cpp index 57972c85c..a196aff83 100644 --- a/libecole/tests/src/dynamics/test-configuring.cpp +++ b/libecole/tests/src/dynamics/test-configuring.cpp @@ -11,7 +11,8 @@ using namespace ecole; TEST_CASE("ConfiguringDynamics unit tests", "[unit][dynamics]") { - auto const policy = [](auto const& /*action_set*/) -> trait::action_of_t { + auto const policy = + [](auto const& /*action_set*/, auto const& /*model*/) -> trait::action_of_t { return {{"branching/scorefunc", 's'}}; }; dynamics::unit_tests(dynamics::ConfiguringDynamics{}, policy); diff --git a/libecole/tests/src/dynamics/unit-tests.hpp b/libecole/tests/src/dynamics/unit-tests.hpp index a5b8a3864..0c633839d 100644 --- a/libecole/tests/src/dynamics/unit-tests.hpp +++ b/libecole/tests/src/dynamics/unit-tests.hpp @@ -11,7 +11,7 @@ namespace ecole::dynamics { -template void unit_tests(Dynamics&& dyn, Func const policy) { +template void unit_tests(Dynamics&& dyn, Func policy) { auto model = get_model(); SECTION("Has default constructor") { Dynamics{}; } @@ -34,20 +34,20 @@ template void unit_tests(Dynamics&& dyn, Func SECTION("Reset, step, and delete") { auto [done, action_set] = dyn.reset_dynamics(model); REQUIRE_FALSE(done); - std::tie(done, action_set) = dyn.step_dynamics(model, policy(action_set)); + std::tie(done, action_set) = dyn.step_dynamics(model, policy(action_set, model)); } SECTION("Run full trajectory") { auto [done, action_set] = dyn.reset_dynamics(model); while (!done) { - std::tie(done, action_set) = dyn.step_dynamics(model, policy(action_set)); + std::tie(done, action_set) = dyn.step_dynamics(model, policy(action_set, model)); } SECTION("Run another trajectory") { model = get_model(); std::tie(done, action_set) = dyn.reset_dynamics(model); while (!done) { - std::tie(done, action_set) = dyn.step_dynamics(model, policy(action_set)); + std::tie(done, action_set) = dyn.step_dynamics(model, policy(action_set, model)); } } } From 5ff7c2299f2ab06aabfdb515173978f51c3ef26c Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Tue, 25 May 2021 17:07:47 -0400 Subject: [PATCH 12/20] Add BranchingGUBDynamics C++ tests --- libecole/tests/CMakeLists.txt | 1 + .../tests/src/dynamics/test-branching-gub.cpp | 62 +++++++++++++++++++ 2 files changed, 63 insertions(+) create mode 100644 libecole/tests/src/dynamics/test-branching-gub.cpp diff --git a/libecole/tests/CMakeLists.txt b/libecole/tests/CMakeLists.txt index 1ac328c6f..0b6e29288 100644 --- a/libecole/tests/CMakeLists.txt +++ b/libecole/tests/CMakeLists.txt @@ -45,6 +45,7 @@ add_executable( src/observation/test-khalil-2016.cpp src/dynamics/test-branching.cpp + src/dynamics/test-branching-gub.cpp src/dynamics/test-configuring.cpp src/environment/test-environment.cpp diff --git a/libecole/tests/src/dynamics/test-branching-gub.cpp b/libecole/tests/src/dynamics/test-branching-gub.cpp new file mode 100644 index 000000000..e71da4601 --- /dev/null +++ b/libecole/tests/src/dynamics/test-branching-gub.cpp @@ -0,0 +1,62 @@ +#include +#include +#include +#include +#include + +#include +#include +#include + +#include "ecole/dynamics/branching-gub.hpp" +#include "ecole/exception.hpp" +#include "ecole/random.hpp" +#include "ecole/traits.hpp" + +#include "conftest.hpp" +#include "dynamics/unit-tests.hpp" + +using namespace ecole; + +TEST_CASE("BranchingGUBDynamics unit tests with single branching", "[unit][dynamics]") { + auto const policy = [](auto const& action_set, auto const& /*model*/) { return std::array{action_set.value()[0]}; }; + dynamics::unit_tests(dynamics::BranchingGUBDynamics{}, policy); +} + +/** + * A policy for multivariable branching. + * + * Try randomly to find two variables whose LP solution value sum is not integer. + * As it may not always exists, stop otherwise and branch on a single variable. + */ +struct MultiBrancingPolicy { + std::size_t n_multi = 0; + + auto operator()(trait::action_set_of_t const& action_set, scip::Model& model) + -> std::vector { + auto as = action_set.value(); + auto rng = RandomEngine{as[0] + as.size()}; + auto choice = std::uniform_int_distribution{0, as.size() - 1}; + auto indices = std::array{as[choice(rng)], as[choice(rng)]}; + auto const is_lp_sum_integral = [&model](auto idx1, auto idx2) { + auto const idx_to_var = [&](auto idx) { return SCIPcolGetVar(model.lp_columns()[idx]); }; + auto const is_lp = SCIPhasCurrentNodeLP(model.get_scip_ptr()); + auto const sum = SCIPvarGetSol(idx_to_var(idx1), is_lp) + SCIPvarGetSol(idx_to_var(idx2), is_lp); + return SCIPfeasFloor(model.get_scip_ptr(), sum) == SCIPfeasCeil(model.get_scip_ptr(), sum); + }; + auto constexpr n_trials = 10; + for (std::size_t n = 0; n < n_trials; ++n) { + if ((indices[0] != indices[1]) && (!is_lp_sum_integral(indices[0], indices[1]))) { + n_multi++; + return {indices[0], indices[1]}; + } + indices = {as[choice(rng)], as[choice(rng)]}; + } + return {indices[0]}; + } +}; + +TEST_CASE("BranchingGUBDynamics unit tests with multi branching", "[unit][dynamics]") { + auto policy = MultiBrancingPolicy{}; + dynamics::unit_tests(dynamics::BranchingGUBDynamics{}, policy); +} From 206752590c72ace4c48567c6782fef8c776b2922 Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Thu, 27 May 2021 11:38:54 -0400 Subject: [PATCH 13/20] Add BranchigGUB environment alias --- .../ecole/environment/branching-gub.hpp | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) create mode 100644 libecole/include/ecole/environment/branching-gub.hpp diff --git a/libecole/include/ecole/environment/branching-gub.hpp b/libecole/include/ecole/environment/branching-gub.hpp new file mode 100644 index 000000000..aab97d748 --- /dev/null +++ b/libecole/include/ecole/environment/branching-gub.hpp @@ -0,0 +1,18 @@ +#pragma once + +#include "ecole/dynamics/branching-gub.hpp" +#include "ecole/environment/environment.hpp" +#include "ecole/information/nothing.hpp" +#include "ecole/observation/nodebipartite.hpp" +#include "ecole/reward/isdone.hpp" + +namespace ecole::environment { + +template < + typename ObservationFunction = observation::NodeBipartite, + typename RewardFunction = reward::IsDone, + typename InformationFunction = information::Nothing> +using BranchingGUB = + Environment; + +} // namespace ecole::environment From 7df4c63003211adba5ff554d4a3bcf86f9feff92 Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Thu, 27 May 2021 15:59:21 -0400 Subject: [PATCH 14/20] Test BranchingGUB --- .../tests/src/dynamics/test-branching-gub.cpp | 58 +++++++++++++++++-- .../tests/src/dynamics/test-branching.cpp | 2 +- 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/libecole/tests/src/dynamics/test-branching-gub.cpp b/libecole/tests/src/dynamics/test-branching-gub.cpp index e71da4601..83cdf30eb 100644 --- a/libecole/tests/src/dynamics/test-branching-gub.cpp +++ b/libecole/tests/src/dynamics/test-branching-gub.cpp @@ -29,13 +29,13 @@ TEST_CASE("BranchingGUBDynamics unit tests with single branching", "[unit][dynam * Try randomly to find two variables whose LP solution value sum is not integer. * As it may not always exists, stop otherwise and branch on a single variable. */ -struct MultiBrancingPolicy { +struct MultiBranchingPolicy { std::size_t n_multi = 0; + RandomEngine rng{0}; // NOLINT(cert-msc32-c, cert-msc51-cpp) We want reproducibility in tests auto operator()(trait::action_set_of_t const& action_set, scip::Model& model) -> std::vector { - auto as = action_set.value(); - auto rng = RandomEngine{as[0] + as.size()}; + auto const& as = action_set.value(); auto choice = std::uniform_int_distribution{0, as.size() - 1}; auto indices = std::array{as[choice(rng)], as[choice(rng)]}; auto const is_lp_sum_integral = [&model](auto idx1, auto idx2) { @@ -57,6 +57,56 @@ struct MultiBrancingPolicy { }; TEST_CASE("BranchingGUBDynamics unit tests with multi branching", "[unit][dynamics]") { - auto policy = MultiBrancingPolicy{}; + auto policy = MultiBranchingPolicy{}; dynamics::unit_tests(dynamics::BranchingGUBDynamics{}, policy); } + +TEST_CASE("BranchingGUBDynamics can solve instance", "[dynamics][slow]") { + auto dyn = dynamics::BranchingGUBDynamics{}; + auto model = get_model(); + auto policy = MultiBranchingPolicy{}; + + SECTION("Return valid action set") { + auto const [done, action_set] = dyn.reset_dynamics(model); + REQUIRE(action_set.has_value()); + auto const& branch_cands = action_set.value(); + REQUIRE(branch_cands.size() > 0); + REQUIRE(branch_cands.size() <= model.lp_columns().size()); + REQUIRE(xt::all(branch_cands >= 0)); + REQUIRE(xt::all(branch_cands < model.lp_columns().size())); + REQUIRE(xt::unique(branch_cands).size() == branch_cands.size()); + } + + SECTION("Solve instance with multiple variables") { + auto [done, action_set] = dyn.reset_dynamics(model); + while (!done) { + REQUIRE(action_set.has_value()); + auto const action = policy(action_set, model); + std::tie(done, action_set) = dyn.step_dynamics(model, action); + } + REQUIRE(model.is_solved()); + } + + SECTION("Solve instance with single variables") { + auto [done, action_set] = dyn.reset_dynamics(model); + while (!done) { + REQUIRE(action_set.has_value()); + auto const action = action_set.value()[0]; + std::tie(done, action_set) = dyn.step_dynamics(model, {&action, 1}); + } + REQUIRE(model.is_solved()); + } +} + +TEST_CASE("BranchingGUBDynamics handles invalid inputs", "[dynamics]") { + auto dyn = dynamics::BranchingGUBDynamics{}; + auto model = get_model(); + + SECTION("Throw on invalid branching variable") { + auto const [done, action_set] = dyn.reset_dynamics(model); + REQUIRE_FALSE(done); + REQUIRE(action_set.has_value()); + auto const action = model.lp_columns().size() + 1; + REQUIRE_THROWS_AS(dyn.step_dynamics(model, {&action, 1}), std::invalid_argument); + } +} diff --git a/libecole/tests/src/dynamics/test-branching.cpp b/libecole/tests/src/dynamics/test-branching.cpp index f381ec23d..d5ca49933 100644 --- a/libecole/tests/src/dynamics/test-branching.cpp +++ b/libecole/tests/src/dynamics/test-branching.cpp @@ -53,7 +53,7 @@ TEST_CASE("BranchingDynamics functional tests", "[dynamics]") { REQUIRE_FALSE(done); REQUIRE(action_set.has_value()); auto const action = model.lp_columns().size() + 1; - REQUIRE_THROWS_AS(dyn.step_dynamics(model, action), std::exception); + REQUIRE_THROWS_AS(dyn.step_dynamics(model, action), std::invalid_argument); } } From c48a06a4e7737f690b453455b3a98c058fd0c177 Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Thu, 27 May 2021 16:20:59 -0400 Subject: [PATCH 15/20] Refactor dynamics bindings --- python/src/ecole/core/auto-class.hpp | 1 + python/src/ecole/core/dynamics.cpp | 56 ++++++++++++++++++++++------ 2 files changed, 45 insertions(+), 12 deletions(-) diff --git a/python/src/ecole/core/auto-class.hpp b/python/src/ecole/core/auto-class.hpp index 8d8fd959e..d59b89789 100644 --- a/python/src/ecole/core/auto-class.hpp +++ b/python/src/ecole/core/auto-class.hpp @@ -3,6 +3,7 @@ #include #include #include +#include #include diff --git a/python/src/ecole/core/dynamics.cpp b/python/src/ecole/core/dynamics.cpp index 243675cf6..ec434665a 100644 --- a/python/src/ecole/core/dynamics.cpp +++ b/python/src/ecole/core/dynamics.cpp @@ -1,5 +1,4 @@ -#include -#include +#include #include #include @@ -15,25 +14,58 @@ namespace ecole::dynamics { namespace py = pybind11; -template auto dynamics_class(py::module_ const& m, char const* name) { - return py::class_(m, name) // - .def("reset_dynamics", &Dynamics::reset_dynamics, py::arg("model"), py::call_guard()) - .def( +template struct dynamics_class : public pybind11::class_ { + using pybind11::class_::class_; + + /** Bind reset_dynamics */ + template auto def_reset_dynamics(Args&&... args) -> auto& { + this->def( + "reset_dynamics", + &Class::reset_dynamics, + py::arg("model"), + py::call_guard(), + std::forward(args)...); + return *this; + } + + /** Bind step_dynamics */ + template auto def_step_dynamics(Args&&... args) -> auto& { + this->def( "step_dynamics", - &Dynamics::step_dynamics, + &Class::step_dynamics, py::arg("model"), py::arg("action"), - py::call_guard()) - .def("set_dynamics_random_state", &Dynamics::set_dynamics_random_state, py::arg("model"), py::arg("random_engine")); -} + py::call_guard(), + std::forward(args)...); + return *this; + } + + /** Bind set_dynamics_random_state */ + template auto def_set_dynamics_random_state(Args&&... args) -> auto& { + this->def( + "set_dynamics_random_state", + &Class::set_dynamics_random_state, + py::arg("model"), + py::arg("random_engine"), + py::call_guard(), + std::forward(args)...); + return *this; + } +}; void bind_submodule(pybind11::module_ const& m) { m.doc() = "Ecole collection of environment dynamics."; - dynamics_class(m, "BranchingDynamics") // + dynamics_class{m, "BranchingDynamics"} + .def_reset_dynamics() + .def_step_dynamics() + .def_set_dynamics_random_state() .def(py::init(), py::arg("pseudo_candidates") = false); - dynamics_class(m, "ConfiguringDynamics") // + dynamics_class{m, "ConfiguringDynamics"} + .def_reset_dynamics() + .def_step_dynamics() + .def_set_dynamics_random_state() .def(py::init<>()); } From fbe66cc7dd2d73245f1bea60e02afae82603ece0 Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Mon, 7 Jun 2021 09:33:57 -0400 Subject: [PATCH 16/20] Bind BranchingGUB to Python and test --- python/src/ecole/core/dynamics.cpp | 17 +++++++++ python/src/ecole/environment.py | 5 +++ python/tests/test_dynamics.py | 55 +++++++++++++++++++++++------- 3 files changed, 64 insertions(+), 13 deletions(-) diff --git a/python/src/ecole/core/dynamics.cpp b/python/src/ecole/core/dynamics.cpp index ec434665a..d05329f5e 100644 --- a/python/src/ecole/core/dynamics.cpp +++ b/python/src/ecole/core/dynamics.cpp @@ -4,6 +4,7 @@ #include #include +#include "ecole/dynamics/branching-gub.hpp" #include "ecole/dynamics/branching.hpp" #include "ecole/dynamics/configuring.hpp" #include "ecole/scip/model.hpp" @@ -62,6 +63,22 @@ void bind_submodule(pybind11::module_ const& m) { .def_set_dynamics_random_state() .def(py::init(), py::arg("pseudo_candidates") = false); + using idx_t = typename BranchingGUBDynamics::Action::value_type; + using array_t = py::array_t; + dynamics_class{m, "BranchingGUBDynamics"} + .def_reset_dynamics() + .def_set_dynamics_random_state() + .def( + "step_dynamics", + [](BranchingGUBDynamics& self, scip::Model& model, array_t const& action) { + auto const vars = nonstd::span{action.data(), static_cast(action.size())}; + auto const release = py::gil_scoped_release{}; + return self.step_dynamics(model, vars); + }, + py::arg("model"), + py::arg("action")) + .def(py::init<>()); + dynamics_class{m, "ConfiguringDynamics"} .def_reset_dynamics() .def_step_dynamics() diff --git a/python/src/ecole/environment.py b/python/src/ecole/environment.py index 63bc41e8c..05bbe942f 100644 --- a/python/src/ecole/environment.py +++ b/python/src/ecole/environment.py @@ -195,5 +195,10 @@ class Branching(Environment): __DefaultObservationFunction__ = ecole.observation.NodeBipartite +class BranchingGUB(Environment): + __Dynamics__ = ecole.dynamics.BranchingGUBDynamics + __DefaultObservationFunction__ = ecole.observation.NodeBipartite + + class Configuring(Environment): __Dynamics__ = ecole.dynamics.ConfiguringDynamics diff --git a/python/tests/test_dynamics.py b/python/tests/test_dynamics.py index 48c8ef924..6a9c5054a 100644 --- a/python/tests/test_dynamics.py +++ b/python/tests/test_dynamics.py @@ -43,9 +43,9 @@ def test_full_trajectory(self, model): def test_exception(self, model): """Bad action raise exceptions.""" - with pytest.raises((ecole.Exception, ecole.scip.Exception)): - self.dynamics.reset_dynamics(model) - self.dynamics.step_dynamics(model, self.bad_action) + with pytest.raises((ecole.scip.Exception, ValueError)): + _, action_set = self.dynamics.reset_dynamics(model) + self.dynamics.step_dynamics(model, self.bad_policy(action_set)) def test_set_random_state(self, model): """Random engine is consumed.""" @@ -62,13 +62,24 @@ def assert_action_set(action_set): assert action_set.size > 0 assert action_set.dtype == np.uint64 + @staticmethod + def policy(action_set): + return action_set[0] + + @staticmethod + def bad_policy(action_set): + return 1 << 31 + def setup_method(self, method): self.dynamics = ecole.dynamics.BranchingDynamics(False) - self.policy = lambda action_set: action_set[0] - self.bad_action = 1 << 31 -class TestBranchingPseudocost(DynamicsUnitTests): +class TestBranching_Pseudocandidate(TestBranching): + def setup_method(self, method): + self.dynamics = ecole.dynamics.BranchingDynamics(True) + + +class TestBranchingGUB_List(DynamicsUnitTests): @staticmethod def assert_action_set(action_set): assert isinstance(action_set, np.ndarray) @@ -76,10 +87,22 @@ def assert_action_set(action_set): assert action_set.size > 0 assert action_set.dtype == np.uint64 + @staticmethod + def policy(action_set): + return [action_set[0]] + + @staticmethod + def bad_policy(action_set): + return [1 << 31] + def setup_method(self, method): - self.dynamics = ecole.dynamics.BranchingDynamics(True) - self.policy = lambda action_set: action_set[0] - self.bad_action = 1 << 31 + self.dynamics = ecole.dynamics.BranchingGUBDynamics() + + +class TestBranchingGUB_Numpy(TestBranchingGUB_List): + @staticmethod + def policy(action_set): + return np.array([action_set[0]]) class TestConfiguring(DynamicsUnitTests): @@ -87,13 +110,19 @@ class TestConfiguring(DynamicsUnitTests): def assert_action_set(action_set): assert action_set is None - def setup_method(self, method): - self.dynamics = ecole.dynamics.ConfiguringDynamics() - self.policy = lambda _: { + @staticmethod + def policy(action_set): + return { "branching/scorefunc": "s", "branching/scorefac": 0.1, "branching/divingpscost": False, "conflict/lpiterations": 0, "heuristics/undercover/fixingalts": "ln", } - self.bad_action = {"not/a/parameter": 44} + + @staticmethod + def bad_policy(action_set): + return {"not/a/parameter": 44} + + def setup_method(self, method): + self.dynamics = ecole.dynamics.ConfiguringDynamics() From 4d0348789c5547621fedbfb42fc4acac62cb59e2 Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Tue, 8 Jun 2021 11:42:35 -0400 Subject: [PATCH 17/20] Document BranchingDynamics --- python/src/ecole/core/dynamics.cpp | 75 ++++++++++++++++++++++++++++-- 1 file changed, 70 insertions(+), 5 deletions(-) diff --git a/python/src/ecole/core/dynamics.cpp b/python/src/ecole/core/dynamics.cpp index d05329f5e..eca2b5606 100644 --- a/python/src/ecole/core/dynamics.cpp +++ b/python/src/ecole/core/dynamics.cpp @@ -57,11 +57,76 @@ template struct dynamics_class : public void bind_submodule(pybind11::module_ const& m) { m.doc() = "Ecole collection of environment dynamics."; - dynamics_class{m, "BranchingDynamics"} - .def_reset_dynamics() - .def_step_dynamics() - .def_set_dynamics_random_state() - .def(py::init(), py::arg("pseudo_candidates") = false); + dynamics_class{m, "BranchingDynamics", R"( + Single variable branching Dynamics. + + Based on a SCIP branching callback with maximal priority and no depth limit. + The dynamics give the control back to the user every time the callback would be called. + The user recieves as an action set the list of branching candiates, and is expected to select + one of them as the action. + )"} + .def_reset_dynamics(R"( + Start solving up to first branching node. + + Start solving with SCIP defaults (``SCIPsolve``) and give back control to the user on the + first branching decision. + Users can inherit from this dynamics to change the defaults settings such as presolving + and cutting planes. + + Parameters + ---------- + model: + The state of the Markov Decision Process. Passed by the environment. + + Returns + ------- + done: + Whether the instance is solved. + This can happen without branching, for instance if the instance is solved during presolving. + action_set: + List of branching candidates. Candidates depend on parameters in :py:meth:`__init__`. + )") + .def_step_dynamics(R"( + Branch and resume solving until next branching. + + Branching is done on a single variable using ``SCIPbranchVar``. + The control is given back to the user on the next branching decision or when done. + + Parameters + ---------- + model: + The state of the Markov Decision Process. Passed by the environment. + action: + The index the LP column of the variable to branch on. One element of the action set. + + Returns + ------- + done: + Whether the instance is solved. + action_set: + List of branching canddidates. Candidates depend on parameters in :py:meth:`__init__`. + )") + .def_set_dynamics_random_state(R"( + Set seeds on the :py:class:`~ecole.scip.Model`. + + Set seed parameters, inculing permutation, LP, and shift. + + Parameters + ---------- + model: + The state of the Markov Decision Process. Passed by the environment. + random_engine: + The source of randomness. Passed by the environment. + )") + .def(py::init(), py::arg("pseudo_candidates") = false, R"( + Create new dynamics. + + Parameters + ---------- + pseudo_candidates: + Whether the action set contains pseudo branching variable candidates (``SCIPgetPseudoBranchCands``) + or LP branching variable candidates (``SCIPgetPseudoBranchCands``). + )"); using idx_t = typename BranchingGUBDynamics::Action::value_type; using array_t = py::array_t; From 3195496060e52358f8808218c32829b16b135e16 Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Tue, 8 Jun 2021 12:01:25 -0400 Subject: [PATCH 18/20] Document ConfiguringDynamics --- python/src/ecole/core/dynamics.cpp | 56 +++++++++++++++++++++++++++--- 1 file changed, 52 insertions(+), 4 deletions(-) diff --git a/python/src/ecole/core/dynamics.cpp b/python/src/ecole/core/dynamics.cpp index eca2b5606..ae3bf2764 100644 --- a/python/src/ecole/core/dynamics.cpp +++ b/python/src/ecole/core/dynamics.cpp @@ -144,10 +144,58 @@ void bind_submodule(pybind11::module_ const& m) { py::arg("action")) .def(py::init<>()); - dynamics_class{m, "ConfiguringDynamics"} - .def_reset_dynamics() - .def_step_dynamics() - .def_set_dynamics_random_state() + dynamics_class{m, "ConfiguringDynamics", R"( + Setting solving parameters dynamics. + + This dynamics are meant to be used as a (contextual) bandit to find good parameters for SCIP. + )"} + .def_reset_dynamics(R"( + Does nothing. + + Users can inherit from this dynamics to change when in the solving process parameters will be set + (for instance after presolving). + + Parameters + ---------- + model: + The state of the Markov Decision Process. Passed by the environment. + + Returns + ------- + done: + Whether the instance is solved. Always false. + action_set: + Unused. + )") + .def_step_dynamics(R"( + Set parameters and solve the instance. + + Parameters + ---------- + model: + The state of the Markov Decision Process. Passed by the environment. + action: + A mapping of parameter names and values. + + Returns + ------- + done: + Whether the instance is solved. Always true. + action_set: + Unused. + )") + .def_set_dynamics_random_state(R"( + Set seeds on the :py:class:`~ecole.scip.Model`. + + Set seed parameters, inculing permutation, LP, and shift. + + Parameters + ---------- + model: + The state of the Markov Decision Process. Passed by the environment. + random_engine: + The source of randomness.Passed by the environment. + )") .def(py::init<>()); } From 2080c80bacd1728aa4fa1b99222a307a55c11cf1 Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Tue, 8 Jun 2021 12:19:53 -0400 Subject: [PATCH 19/20] Document BranchingGUBDynamics --- docs/reference/environments.rst | 5 ++ python/src/ecole/core/dynamics.cpp | 74 ++++++++++++++++++++++++++++-- 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/docs/reference/environments.rst b/docs/reference/environments.rst index a634e0157..2bd082846 100644 --- a/docs/reference/environments.rst +++ b/docs/reference/environments.rst @@ -16,6 +16,11 @@ Branching .. autoclass:: ecole.environment.Branching .. autoclass:: ecole.dynamics.BranchingDynamics +BranchingGUB +^^^^^^^^^^^^ +.. autoclass:: ecole.environment.BranchingGUB +.. autoclass:: ecole.dynamics.BranchingGUBDynamics + Configuring ^^^^^^^^^^^ .. autoclass:: ecole.environment.Configuring diff --git a/python/src/ecole/core/dynamics.cpp b/python/src/ecole/core/dynamics.cpp index ae3bf2764..359a189ac 100644 --- a/python/src/ecole/core/dynamics.cpp +++ b/python/src/ecole/core/dynamics.cpp @@ -130,9 +130,51 @@ void bind_submodule(pybind11::module_ const& m) { using idx_t = typename BranchingGUBDynamics::Action::value_type; using array_t = py::array_t; - dynamics_class{m, "BranchingGUBDynamics"} - .def_reset_dynamics() - .def_set_dynamics_random_state() + dynamics_class{m, "BranchingGUBDynamics", R"( + Global Upper Bound branching Dynamics. + + Based on a SCIP branching callback with maximal priority and no depth limit. + The dynamics give the control back to the user every time the callback would be called. + The user recieves as an action set the list of branching candiates, and is expected to select + a subset of them to branch on their sum. + + .. warning:: + The function used to perform branching is provided by Ecole and has not been extensively tested on + a large variety of problem instances. + )"} + .def_reset_dynamics(R"( + Start solving up to first branching node. + + Start solving with SCIP defaults (``SCIPsolve``) and give back control to the user on the + first branching decision. + Users can inherit from this dynamics to change the defaults settings such as presolving + and cutting planes. + + Parameters + ---------- + model: + The state of the Markov Decision Process. Passed by the environment. + + Returns + ------- + done: + Whether the instance is solved. + This can happen without branching, for instance if the instance is solved during presolving. + action_set: + List of branching candidates (``SCIPgetPseudoBranchCands``). + )") + .def_set_dynamics_random_state(R"( + Set seeds on the :py:class:`~ecole.scip.Model`. + + Set seed parameters, inculing permutation, LP, and shift. + + Parameters + ---------- + model: + The state of the Markov Decision Process. Passed by the environment. + random_engine: + The source of randomness. Passed by the environment. + )") .def( "step_dynamics", [](BranchingGUBDynamics& self, scip::Model& model, array_t const& action) { @@ -141,7 +183,31 @@ void bind_submodule(pybind11::module_ const& m) { return self.step_dynamics(model, vars); }, py::arg("model"), - py::arg("action")) + py::arg("action"), + R"( + Branch and resume solving until next branching. + + Branching is done on a the sum of the given variables using their LP or pseudo solution value. + To make a valid branching, the sum of the LP or pseudo solution value of the given variables + must be non integer. + In the opposite case, an exception will be thrown, + The control is given back to the user on the next branching decision or when done. + + Parameters + ---------- + model: + The state of the Markov Decision Process. Passed by the environment. + action: + A subset of of the variables of given in the aaction set. + Not all subsets are valid. + + Returns + ------- + done: + Whether the instance is solved. + action_set: + List of branching candidates (``SCIPgetPseudoBranchCands``). + )") .def(py::init<>()); dynamics_class{m, "ConfiguringDynamics", R"( From 6db3e5469f078e47ba51b313d38e426ae8c56e2d Mon Sep 17 00:00:00 2001 From: AntoinePrv Date: Tue, 15 Jun 2021 11:00:49 -0400 Subject: [PATCH 20/20] Include @gasse feedback in BranchingGUB --- .../include/ecole/dynamics/branching-gub.hpp | 2 +- libecole/src/dynamics/branching-gub.cpp | 17 +++++++------- python/src/ecole/core/dynamics.cpp | 22 +++++++++---------- 3 files changed, 21 insertions(+), 20 deletions(-) diff --git a/libecole/include/ecole/dynamics/branching-gub.hpp b/libecole/include/ecole/dynamics/branching-gub.hpp index b94ff1ebb..8db1c67d6 100644 --- a/libecole/include/ecole/dynamics/branching-gub.hpp +++ b/libecole/include/ecole/dynamics/branching-gub.hpp @@ -18,7 +18,7 @@ class BranchingGUBDynamics : std::tuple reset_dynamics(scip::Model& model) override; - std::tuple step_dynamics(scip::Model& model, Action const& var_idx) override; + std::tuple step_dynamics(scip::Model& model, Action const& var_indices) override; }; } // namespace ecole::dynamics diff --git a/libecole/src/dynamics/branching-gub.cpp b/libecole/src/dynamics/branching-gub.cpp index 47cf4afa8..3d22207e1 100644 --- a/libecole/src/dynamics/branching-gub.cpp +++ b/libecole/src/dynamics/branching-gub.cpp @@ -172,22 +172,23 @@ auto BranchingGUBDynamics::reset_dynamics(scip::Model& model) -> std::tuple std::tuple { +auto BranchingGUBDynamics::step_dynamics(scip::Model& model, Action const& var_indices) -> std::tuple { auto const lp_cols = model.lp_columns(); // Check that input indices are within range auto const is_out_of_bounds = [size = lp_cols.size()](auto idx) { return idx >= size; }; - if (std::any_of(var_idx.begin(), var_idx.end(), is_out_of_bounds)) { + if (std::any_of(var_indices.begin(), var_indices.end(), is_out_of_bounds)) { throw std::invalid_argument{"Branching index is larger than the number of columns."}; } - // Get variables associated with indices - auto vars = std::vector(var_idx.size()); - auto const idx_to_var = [lp_cols](auto idx) { return SCIPcolGetVar(lp_cols[idx]); }; - std::transform(var_idx.begin(), var_idx.end(), vars.begin(), idx_to_var); + { // Get variables associated with indices and branch + auto vars = std::vector(var_indices.size()); + auto const idx_to_var = [lp_cols](auto idx) { return SCIPcolGetVar(lp_cols[idx]); }; + std::transform(var_indices.begin(), var_indices.end(), vars.begin(), idx_to_var); - scip::call(SCIPbranchGUB, model.get_scip_ptr(), vars.data(), static_cast(vars.size()), nullptr, nullptr); - model.solve_iter_branch(SCIP_BRANCHED); + scip::call(SCIPbranchGUB, model.get_scip_ptr(), vars.data(), static_cast(vars.size()), nullptr, nullptr); + model.solve_iter_branch(SCIP_BRANCHED); + } if (model.solve_iter_is_done()) { return {true, {}}; diff --git a/python/src/ecole/core/dynamics.cpp b/python/src/ecole/core/dynamics.cpp index 359a189ac..28d62c75a 100644 --- a/python/src/ecole/core/dynamics.cpp +++ b/python/src/ecole/core/dynamics.cpp @@ -62,7 +62,7 @@ void bind_submodule(pybind11::module_ const& m) { Based on a SCIP branching callback with maximal priority and no depth limit. The dynamics give the control back to the user every time the callback would be called. - The user recieves as an action set the list of branching candiates, and is expected to select + The user recieves as an action set the list of branching candidates, and is expected to select one of them as the action. )"} .def_reset_dynamics(R"( @@ -104,12 +104,12 @@ void bind_submodule(pybind11::module_ const& m) { done: Whether the instance is solved. action_set: - List of branching canddidates. Candidates depend on parameters in :py:meth:`__init__`. + List of branching candidates. Candidates depend on parameters in :py:meth:`__init__`. )") .def_set_dynamics_random_state(R"( Set seeds on the :py:class:`~ecole.scip.Model`. - Set seed parameters, inculing permutation, LP, and shift. + Set seed parameters, including permutation, LP, and shift. Parameters ---------- @@ -135,7 +135,7 @@ void bind_submodule(pybind11::module_ const& m) { Based on a SCIP branching callback with maximal priority and no depth limit. The dynamics give the control back to the user every time the callback would be called. - The user recieves as an action set the list of branching candiates, and is expected to select + The user recieves as an action set the list of branching candidates, and is expected to select a subset of them to branch on their sum. .. warning:: @@ -166,7 +166,7 @@ void bind_submodule(pybind11::module_ const& m) { .def_set_dynamics_random_state(R"( Set seeds on the :py:class:`~ecole.scip.Model`. - Set seed parameters, inculing permutation, LP, and shift. + Set seed parameters, including permutation, LP, and shift. Parameters ---------- @@ -187,7 +187,7 @@ void bind_submodule(pybind11::module_ const& m) { R"( Branch and resume solving until next branching. - Branching is done on a the sum of the given variables using their LP or pseudo solution value. + Branching is done on the sum of the given variables using their LP or pseudo solution value. To make a valid branching, the sum of the LP or pseudo solution value of the given variables must be non integer. In the opposite case, an exception will be thrown, @@ -198,8 +198,8 @@ void bind_submodule(pybind11::module_ const& m) { model: The state of the Markov Decision Process. Passed by the environment. action: - A subset of of the variables of given in the aaction set. - Not all subsets are valid. + A subset of of the variables of given in the action set. + Not all subsets are valid (see above). Returns ------- @@ -213,7 +213,7 @@ void bind_submodule(pybind11::module_ const& m) { dynamics_class{m, "ConfiguringDynamics", R"( Setting solving parameters dynamics. - This dynamics are meant to be used as a (contextual) bandit to find good parameters for SCIP. + These dynamics are meant to be used as a (contextual) bandit to find good parameters for SCIP. )"} .def_reset_dynamics(R"( Does nothing. @@ -253,14 +253,14 @@ void bind_submodule(pybind11::module_ const& m) { .def_set_dynamics_random_state(R"( Set seeds on the :py:class:`~ecole.scip.Model`. - Set seed parameters, inculing permutation, LP, and shift. + Set seed parameters, including permutation, LP, and shift. Parameters ---------- model: The state of the Markov Decision Process. Passed by the environment. random_engine: - The source of randomness.Passed by the environment. + The source of randomness. Passed by the environment. )") .def(py::init<>()); }