Merge pull request #234 from benoitsteiner/master

Fix for issue 224

Author: AntoinePrv

libecole/include/ecole/observation/khalil-2016.hpp

@@ -108,11 +108,14 @@ struct Khalil2016Obs {
 
 class Khalil2016 : public ObservationFunction<std::optional<Khalil2016Obs>> {
 public:
+	Khalil2016(bool pseudo_candidates = false) noexcept;
+
 	void before_reset(scip::Model& model) override;
 
 	std::optional<Khalil2016Obs> extract(scip::Model& model, bool done) override;
 
 private:
+	bool pseudo_candidates;
 	xt::xtensor<double, 2> static_features;
 };
 

libecole/src/observation/khalil-2016.cpp

@@ -554,33 +554,31 @@ void set_dynamic_features(
 template <typename Tensor>
 void set_precomputed_static_features(
 	Tensor&& out,
-	SCIP_VAR* const var,
+	std::size_t var_idx,
 	xt::xtensor<value_type, 2> const& static_features) {
 
-	auto const col_idx = static_cast<std::ptrdiff_t>(SCIPcolGetIndex(SCIPvarGetCol(var)));
 	using namespace xt::placeholders;
-	xt::view(out, xt::range(_, Khalil2016Obs::n_static_features)) = xt::row(static_features, col_idx);
+	xt::view(out, xt::range(_, Khalil2016Obs::n_static_features)) =
+		xt::row(static_features, static_cast<std::ptrdiff_t>(var_idx));
 }
 
 /******************************
  *  Main extraction function  *
  ******************************/
 
-auto extract_all_features(scip::Model& model, xt::xtensor<value_type, 2> const& static_features) {
-	xt::xtensor<value_type, 2> observation{
-		{model.pseudo_branch_cands().size(), Khalil2016Obs::n_features},
-		std::nan(""),
-	};
+auto extract_all_features(scip::Model& model, bool pseudo, xt::xtensor<value_type, 2> const& static_features) {
+	auto const branch_cands = pseudo ? model.pseudo_branch_cands() : model.lp_branch_cands();
+	auto const n_branch_cands = branch_cands.size();
+
+	auto observation = xt::xtensor<value_type, 2>{{n_branch_cands, Khalil2016Obs::n_features}, std::nan("")};
 
 	auto* const scip = model.get_scip_ptr();
 	auto const active_rows_weights = stats_for_active_constraint_coefficients_weights(model);
 
-	auto const pseudo_branch_cands = model.pseudo_branch_cands();
-	auto const n_pseudo_branch_cands = pseudo_branch_cands.size();
-	for (std::size_t var_idx = 0; var_idx < n_pseudo_branch_cands; ++var_idx) {
-		auto* const var = pseudo_branch_cands[var_idx];
+	for (std::size_t var_idx = 0; var_idx < n_branch_cands; ++var_idx) {
+		auto* const var = branch_cands[var_idx];
 		auto features = xt::row(observation, static_cast<std::ptrdiff_t>(var_idx));
-		set_precomputed_static_features(features, var, static_features);
+		set_precomputed_static_features(features, var_idx, static_features);
 		set_dynamic_features(features, scip, var, active_rows_weights);
 	}
 
@@ -598,6 +596,8 @@ auto is_on_root_node(scip::Model& model) -> bool {
  *  Observation extracting function  *
  *************************************/
 
+Khalil2016::Khalil2016(bool pseudo_candidates_) noexcept : pseudo_candidates(pseudo_candidates_) {}
+
 void Khalil2016::before_reset(scip::Model& /* model */) {
 	static_features = decltype(static_features){};
 }
@@ -607,7 +607,7 @@ auto Khalil2016::extract(scip::Model& model, bool /* done */) -> std::optional<K
 		if (is_on_root_node(model)) {
 			static_features = extract_static_features(model);
 		}
-		return {{extract_all_features(model, static_features)}};
+		return {{extract_all_features(model, pseudo_candidates, static_features)}};
 	}
 	return {};
 }

libecole/tests/src/observation/test-khalil-2016.cpp

@@ -10,7 +10,8 @@
 using namespace ecole;
 
 TEST_CASE("Khalil2016 unit tests", "[unit][obs]") {
-	observation::unit_tests(observation::Khalil2016{});
+	auto const pseudo = GENERATE(true, false);
+	observation::unit_tests(observation::Khalil2016{pseudo});
 }
 
 template <typename Tensor, typename T = typename Tensor::value_type>
@@ -22,7 +23,8 @@ auto in_interval(Tensor const& tensor, T const& lower, T const& upper) {
 TEST_CASE("Khalil2016 return correct observation", "[obs]") {
 	using Features = observation::Khalil2016Obs::Features;
 
-	auto obs_func = observation::Khalil2016{};
+	auto const pseudo = GENERATE(true, false);
+	auto obs_func = observation::Khalil2016{pseudo};
 	auto model = get_model();
 	obs_func.before_reset(model);
 	advance_to_stage(model, SCIP_STAGE_SOLVING);
@@ -32,7 +34,8 @@ TEST_CASE("Khalil2016 return correct observation", "[obs]") {
 
 	SECTION("Observation features has correct shape") {
 		auto const& obs = optional_obs.value();
-		REQUIRE(obs.features.shape(0) == model.pseudo_branch_cands().size());
+		auto const branch_cands = pseudo ? model.pseudo_branch_cands() : model.lp_branch_cands();
+		REQUIRE(obs.features.shape(0) == branch_cands.size());
 		REQUIRE(obs.features.shape(1) == observation::Khalil2016Obs::n_features);
 	}
 

python/src/ecole/core/observation.cpp

@@ -364,7 +364,15 @@ void bind_submodule(py::module_ const& m) {
 
 		This observation function extract structured :py:class:`Khalil2016Obs`.
 	)");
-	khalil2016.def(py::init<>());
+	khalil2016.def(py::init<bool>(), py::arg("pseudo_candidates") = false, R"(
+		Create new observation.
+
+		Parameters
+		----------
+		pseudo_candidates:
+				Whether the pseudo branching variable candidates (``SCIPgetPseudoBranchCands``)
+				or LP branching variable candidates (``SCIPgetPseudoBranchCands``) are observed.
+	)");
 	def_before_reset(khalil2016, R"(Reset static features cache.)");
 	def_extract(khalil2016, "Extract the observation matrix.");