Adapt BranchingDynamics to use new spans

Author: AntoinePrv

libecole/include/ecole/scip/model.hpp

@@ -112,6 +112,7 @@ class Model {
 
 	nonstd::span<Var*> variables() const noexcept;
 	nonstd::span<Var*> lp_branch_cands() const;
+	nonstd::span<Var*> pseudo_branch_cands() const;
 	nonstd::span<Col*> lp_columns() const;
 	nonstd::span<Row*> lp_rows() const;
 

libecole/src/environment/branching-dynamics.cpp

@@ -2,7 +2,6 @@
 #include <memory>
 #include <utility>
 
-#include <nonstd/span.hpp>
 #include <xtensor/xtensor.hpp>
 
 #include "ecole/environment/branching-dynamics.hpp"
@@ -17,29 +16,13 @@ namespace environment {
 BranchingDynamics::BranchingDynamics(bool pseudo_candidates_) noexcept :
 	pseudo_candidates(pseudo_candidates_) {}
 
-static nonstd::span<SCIP_VAR*> lp_branch_cands(SCIP* scip) {
-	SCIP_VAR** cands = nullptr;
-	int n_cands = 0;
-	scip::call(SCIPgetLPBranchCands, scip, &cands, nullptr, nullptr, &n_cands, nullptr, nullptr);
-	assert(n_cands >= 0);
-	return {cands, static_cast<std::size_t>(n_cands)};
-}
-
-static nonstd::span<SCIP_VAR*> pseudo_branch_cands(SCIP* scip) {
-	SCIP_VAR** cands = nullptr;
-	int n_cands = 0;
-	scip::call(SCIPgetPseudoBranchCands, scip, &cands, &n_cands, nullptr);
-	assert(n_cands >= 0);
-	return {cands, static_cast<std::size_t>(n_cands)};
-}
+namespace {
 
-static nonstd::optional<xt::xtensor<std::size_t, 1>>
-action_set(scip::Model const& model, bool pseudo) {
+nonstd::optional<xt::xtensor<std::size_t, 1>> action_set(scip::Model const& model, bool pseudo) {
 	if (model.get_stage() != SCIP_STAGE_SOLVING) {
 		return {};
 	}
-	auto const scip = model.get_scip_ptr();
-	auto const branch_cands = pseudo ? pseudo_branch_cands(scip) : lp_branch_cands(scip);
+	auto const branch_cands = pseudo ? model.pseudo_branch_cands() : model.lp_branch_cands();
 	auto branch_cols = xt::xtensor<std::size_t, 1>::from_shape({branch_cands.size()});
 	std::transform(  //
 		branch_cands.begin(),
@@ -51,6 +34,8 @@ action_set(scip::Model const& model, bool pseudo) {
 	return branch_cols;
 }
 
+}  // namespace
+
 auto BranchingDynamics::reset_dynamics(scip::Model& model) -> std::tuple<bool, ActionSet> {
 	model.solve_iter();
 	return {model.solve_iter_is_done(), action_set(model, pseudo_candidates)};
@@ -59,10 +44,10 @@ auto BranchingDynamics::reset_dynamics(scip::Model& model) -> std::tuple<bool, A
 auto BranchingDynamics::step_dynamics(scip::Model& model, std::size_t const& action)
 	-> std::tuple<bool, ActionSet> {
 	auto const lp_cols = model.lp_columns();
-	if (action >= lp_cols.size) {
+	if (action >= lp_cols.size()) {
 		throw Exception("Branching index is larger than the number of columns.");
 	}
-	model.solve_iter_branch(lp_cols[action].var());
+	model.solve_iter_branch(SCIPcolGetVar(lp_cols[action]));
 
 	return {model.solve_iter_is_done(), action_set(model, pseudo_candidates)};
 }

libecole/src/scip/model.cpp

@@ -189,13 +189,20 @@ nonstd::span<Var*> Model::variables() const noexcept {
 }
 
 nonstd::span<Var*> Model::lp_branch_cands() const {
-	int n_vars{};
-	SCIP_VAR** vars{};
+	int n_vars = 0;
+	SCIP_VAR** vars = nullptr;
 	scip::call(
 		SCIPgetLPBranchCands, get_scip_ptr(), &vars, nullptr, nullptr, &n_vars, nullptr, nullptr);
 	return {vars, static_cast<std::size_t>(n_vars)};
 }
 
+nonstd::span<Var*> Model::pseudo_branch_cands() const {
+	int n_vars = 0;
+	SCIP_VAR** vars = nullptr;
+	scip::call(SCIPgetPseudoBranchCands, get_scip_ptr(), &vars, &n_vars, nullptr);
+	return {vars, static_cast<std::size_t>(n_vars)};
+}
+
 nonstd::span<Col*> Model::lp_columns() const {
 	auto const scip_ptr = get_scip_ptr();
 	if (SCIPgetStage(scip_ptr) != SCIP_STAGE_SOLVING) {

libecole/tests/src/environment/test-branching-dynamics.cpp

@@ -35,9 +35,9 @@ TEST_CASE("BranchingDynamics functional tests", "[dyn]") {
 		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(branch_cands.size() < model.lp_columns().size());
 		REQUIRE(xt::all(branch_cands >= 0));
-		REQUIRE(xt::all(branch_cands < model.lp_columns().size));
+		REQUIRE(xt::all(branch_cands < model.lp_columns().size()));
 		REQUIRE(xt::unique(branch_cands).size() == branch_cands.size());
 	}
 
@@ -54,7 +54,7 @@ TEST_CASE("BranchingDynamics functional tests", "[dyn]") {
 		std::tie(done, action_set) = dyn.reset_dynamics(model);
 		REQUIRE_FALSE(done);
 		REQUIRE(action_set.has_value());
-		auto const action = model.lp_columns().size + 1;
+		auto const action = model.lp_columns().size() + 1;
 		REQUIRE_THROWS_AS(dyn.step_dynamics(model, action), environment::Exception);
 	}
 }

libecole/tests/src/observation/test-strongbranchingscores.cpp

@@ -23,7 +23,7 @@ TEST_CASE("StrongBranchingScores return correct branchig scores", "[obs]") {
 
 	REQUIRE(obs.has_value());
 	auto const scores = obs.value();
-	REQUIRE(scores.size() == model.lp_columns().size);
+	REQUIRE(scores.size() == model.lp_columns().size());
 	auto const not_nan_scores = xt::filter(scores, !xt::isnan(scores));
 	REQUIRE(not_nan_scores.size() > 0);
 	REQUIRE(xt::all(not_nan_scores >= 0));