Rewrote NodeBipartite in terms of variables rather than columns

Author: dchetelat

libecole/include/ecole/observation/nodebipartite.hpp

@@ -12,10 +12,10 @@ namespace ecole::observation {
 struct NodeBipartiteObs {
 	using value_type = double;
 
-	static inline std::size_t constexpr n_static_column_features = 5;
-	static inline std::size_t constexpr n_dynamic_column_features = 14;
-	static inline std::size_t constexpr n_column_features = n_static_column_features + n_dynamic_column_features;
-	enum struct ColumnFeatures : std::size_t {
+	static inline std::size_t constexpr n_static_variable_features = 5;
+	static inline std::size_t constexpr n_dynamic_variable_features = 14;
+	static inline std::size_t constexpr n_variable_features = n_static_variable_features + n_dynamic_variable_features;
+	enum struct VariableFeatures : std::size_t {
 		/** Static features */
 		objective = 0,
 		is_type_binary,            // One hot encoded
@@ -54,7 +54,7 @@ struct NodeBipartiteObs {
 		scaled_age,
 	};
 
-	xt::xtensor<value_type, 2> column_features;
+	xt::xtensor<value_type, 2> variable_features;
 	xt::xtensor<value_type, 2> row_features;
 	utility::coo_matrix<value_type> edge_features;
 };

libecole/src/observation/nodebipartite.cpp

@@ -20,10 +20,10 @@ namespace {
  *  Common helpers   *
  *********************/
 
-using xmatrix = decltype(NodeBipartiteObs::column_features);
+using xmatrix = decltype(NodeBipartiteObs::variable_features);
 using value_type = xmatrix::value_type;
 
-using ColumnFeatures = NodeBipartiteObs::ColumnFeatures;
+using VariableFeatures = NodeBipartiteObs::VariableFeatures;
 using RowFeatures = NodeBipartiteObs::RowFeatures;
 
 value_type constexpr cste = 5.;
@@ -34,9 +34,9 @@ SCIP_Real obj_l2_norm(SCIP* const scip) noexcept {
 	return norm > 0 ? norm : 1.;
 }
 
-/******************************************
- *  Column features extraction functions  *
- ******************************************/
+/*******************************************
+ *  Variable features extraction functions *
+ *******************************************/
 
 std::optional<SCIP_Real> upper_bound(SCIP* const scip, SCIP_COL* const col) noexcept {
 	auto const ub_val = SCIPcolGetUb(col);
@@ -85,11 +85,11 @@ std::optional<SCIP_Real> avg_sol(SCIP* const scip, SCIP_VAR* const var) noexcept
 	return {};
 }
 
-std::optional<SCIP_Real> feas_frac(SCIP* const scip, SCIP_VAR* const var, SCIP_COL* const col) noexcept {
+std::optional<SCIP_Real> feas_frac(SCIP* const scip, SCIP_VAR* const var) noexcept {
 	if (SCIPvarGetType(var) == SCIP_VARTYPE_CONTINUOUS) {
 		return {};
 	}
-	return SCIPfeasFrac(scip, SCIPcolGetPrimsol(col));
+	return SCIPfeasFrac(scip, SCIPvarGetLPSol(var));
 }
 
 /** Convert an enum to its underlying index. */
@@ -98,89 +98,89 @@ template <typename E> constexpr auto idx(E e) {
 }
 
 template <typename Features>
-void set_static_features_for_col(Features&& out, SCIP_VAR* const var, SCIP_COL* const col, value_type obj_norm) {
-	out[idx(ColumnFeatures::objective)] = SCIPcolGetObj(col) / obj_norm;
+void set_static_features_for_var(Features&& out, SCIP_VAR* const var, value_type obj_norm) {
+	out[idx(VariableFeatures::objective)] = SCIPvarGetObj(var) / obj_norm;
 	// On-hot enconding of varaible type
-	out[idx(ColumnFeatures::is_type_binary)] = 0.;
-	out[idx(ColumnFeatures::is_type_integer)] = 0.;
-	out[idx(ColumnFeatures::is_type_implicit_integer)] = 0.;
-	out[idx(ColumnFeatures::is_type_continuous)] = 0.;
+	out[idx(VariableFeatures::is_type_binary)] = 0.;
+	out[idx(VariableFeatures::is_type_integer)] = 0.;
+	out[idx(VariableFeatures::is_type_implicit_integer)] = 0.;
+	out[idx(VariableFeatures::is_type_continuous)] = 0.;
 	switch (SCIPvarGetType(var)) {
 	case SCIP_VARTYPE_BINARY:
-		out[idx(ColumnFeatures::is_type_binary)] = 1.;
+		out[idx(VariableFeatures::is_type_binary)] = 1.;
 		break;
 	case SCIP_VARTYPE_INTEGER:
-		out[idx(ColumnFeatures::is_type_integer)] = 1.;
+		out[idx(VariableFeatures::is_type_integer)] = 1.;
 		break;
 	case SCIP_VARTYPE_IMPLINT:
-		out[idx(ColumnFeatures::is_type_implicit_integer)] = 1.;
+		out[idx(VariableFeatures::is_type_implicit_integer)] = 1.;
 		break;
 	case SCIP_VARTYPE_CONTINUOUS:
-		out[idx(ColumnFeatures::is_type_continuous)] = 1.;
+		out[idx(VariableFeatures::is_type_continuous)] = 1.;
 		break;
 	default:
 		assert(false);  // All enum cases must be handled
 	}
 }
 
 template <typename Features>
-void set_dynamic_features_for_col(
+void set_dynamic_features_for_var(
 	Features&& out,
 	SCIP* const scip,
 	SCIP_VAR* const var,
 	SCIP_COL* const col,
 	value_type obj_norm,
 	value_type n_lps) {
-	out[idx(ColumnFeatures::has_lower_bound)] = static_cast<value_type>(lower_bound(scip, col).has_value());
-	out[idx(ColumnFeatures::has_upper_bound)] = static_cast<value_type>(upper_bound(scip, col).has_value());
-	out[idx(ColumnFeatures::normed_reduced_cost)] = SCIPgetColRedcost(scip, col) / obj_norm;
-	out[idx(ColumnFeatures::solution_value)] = SCIPcolGetPrimsol(col);
-	out[idx(ColumnFeatures::solution_frac)] = feas_frac(scip, var, col).value_or(0.);
-	out[idx(ColumnFeatures::is_solution_at_lower_bound)] = static_cast<value_type>(is_prim_sol_at_lb(scip, col));
-	out[idx(ColumnFeatures::is_solution_at_upper_bound)] = static_cast<value_type>(is_prim_sol_at_ub(scip, col));
-	out[idx(ColumnFeatures::scaled_age)] = static_cast<value_type>(SCIPcolGetAge(col)) / (n_lps + cste);
-	out[idx(ColumnFeatures::incumbent_value)] = best_sol_val(scip, var).value_or(nan);
-	out[idx(ColumnFeatures::average_incumbent_value)] = avg_sol(scip, var).value_or(nan);
+	out[idx(VariableFeatures::has_lower_bound)] = static_cast<value_type>(lower_bound(scip, col).has_value());
+	out[idx(VariableFeatures::has_upper_bound)] = static_cast<value_type>(upper_bound(scip, col).has_value());
+	out[idx(VariableFeatures::normed_reduced_cost)] = SCIPgetVarRedcost(scip, var) / obj_norm;
+	out[idx(VariableFeatures::solution_value)] = SCIPvarGetLPSol(var);
+	out[idx(VariableFeatures::solution_frac)] = feas_frac(scip, var).value_or(0.);
+	out[idx(VariableFeatures::is_solution_at_lower_bound)] = static_cast<value_type>(is_prim_sol_at_lb(scip, col));
+	out[idx(VariableFeatures::is_solution_at_upper_bound)] = static_cast<value_type>(is_prim_sol_at_ub(scip, col));
+	out[idx(VariableFeatures::scaled_age)] = static_cast<value_type>(SCIPcolGetAge(col)) / (n_lps + cste);
+	out[idx(VariableFeatures::incumbent_value)] = best_sol_val(scip, var).value_or(nan);
+	out[idx(VariableFeatures::average_incumbent_value)] = avg_sol(scip, var).value_or(nan);
 	// On-hot encoding
-	out[idx(ColumnFeatures::is_basis_lower)] = 0.;
-	out[idx(ColumnFeatures::is_basis_basic)] = 0.;
-	out[idx(ColumnFeatures::is_basis_upper)] = 0.;
-	out[idx(ColumnFeatures::is_basis_zero)] = 0.;
+	out[idx(VariableFeatures::is_basis_lower)] = 0.;
+	out[idx(VariableFeatures::is_basis_basic)] = 0.;
+	out[idx(VariableFeatures::is_basis_upper)] = 0.;
+	out[idx(VariableFeatures::is_basis_zero)] = 0.;
 	switch (SCIPcolGetBasisStatus(col)) {
 	case SCIP_BASESTAT_LOWER:
-		out[idx(ColumnFeatures::is_basis_lower)] = 1.;
+		out[idx(VariableFeatures::is_basis_lower)] = 1.;
 		break;
 	case SCIP_BASESTAT_BASIC:
-		out[idx(ColumnFeatures::is_basis_basic)] = 1.;
+		out[idx(VariableFeatures::is_basis_basic)] = 1.;
 		break;
 	case SCIP_BASESTAT_UPPER:
-		out[idx(ColumnFeatures::is_basis_upper)] = 1.;
+		out[idx(VariableFeatures::is_basis_upper)] = 1.;
 		break;
 	case SCIP_BASESTAT_ZERO:
-		out[idx(ColumnFeatures::is_basis_zero)] = 1.;
+		out[idx(VariableFeatures::is_basis_zero)] = 1.;
 		break;
 	default:
 		assert(false);  // All enum cases must be handled
 	}
 }
 
-void set_features_for_all_cols(xmatrix& out, scip::Model& model, bool const update_static) {
+void set_features_for_all_vars(xmatrix& out, scip::Model& model, bool const update_static) {
 	auto* const scip = model.get_scip_ptr();
 
 	// Contant reused in every iterations
 	auto const n_lps = static_cast<value_type>(SCIPgetNLPs(scip));
 	auto const obj_norm = obj_l2_norm(scip);
 
-	auto const columns = model.lp_columns();
-	auto const n_columns = columns.size();
-	for (std::size_t col_idx = 0; col_idx < n_columns; ++col_idx) {
-		auto* const col = columns[col_idx];
-		auto* const var = SCIPcolGetVar(col);
-		auto features = xt::row(out, static_cast<std::ptrdiff_t>(col_idx));
+	auto const variables = model.variables();
+	auto const n_vars = variables.size();
+	for (std::size_t var_idx = 0; var_idx < n_vars; ++var_idx) {
+		auto* const var = variables[var_idx];
+		auto* const col = SCIPvarGetCol(var);
+		auto features = xt::row(out, static_cast<std::ptrdiff_t>(var_idx));
 		if (update_static) {
-			set_static_features_for_col(features, var, col, obj_norm);
+			set_static_features_for_var(features, var, obj_norm);
 		}
-		set_dynamic_features_for_col(features, scip, var, col, obj_norm, n_lps);
+		set_dynamic_features_for_var(features, scip, var, col, obj_norm, n_lps);
 	}
 }
 
@@ -326,7 +326,7 @@ utility::coo_matrix<value_type> extract_edge_features(scip::Model& model) {
 		if (scip::get_unshifted_lhs(scip, row).has_value()) {
 			for (std::size_t k = 0; k < row_nnz; ++k) {
 				indices(0, j + k) = i;
-				indices(1, j + k) = static_cast<std::size_t>(SCIPcolGetLPPos(row_cols[k]));
+				indices(1, j + k) = static_cast<std::size_t>(SCIPcolGetVarProbindex(row_cols[k]));
 				values[j + k] = -row_vals[k];
 			}
 			j += row_nnz;
@@ -335,7 +335,7 @@ utility::coo_matrix<value_type> extract_edge_features(scip::Model& model) {
 		if (scip::get_unshifted_rhs(scip, row).has_value()) {
 			for (std::size_t k = 0; k < row_nnz; ++k) {
 				indices(0, j + k) = i;
-				indices(1, j + k) = static_cast<std::size_t>(SCIPcolGetLPPos(row_cols[k]));
+				indices(1, j + k) = static_cast<std::size_t>(SCIPcolGetVarProbindex(row_cols[k]));
 				values[j + k] = row_vals[k];
 			}
 			j += row_nnz;
@@ -344,8 +344,9 @@ utility::coo_matrix<value_type> extract_edge_features(scip::Model& model) {
 	}
 
 	auto const n_rows = n_ineq_rows(model);
-	auto const n_cols = static_cast<std::size_t>(SCIPgetNLPCols(scip));
-	return {values, indices, {n_rows, n_cols}};
+	// Change this here for variables
+	auto const n_vars = static_cast<std::size_t>(SCIPgetNVars(scip));
+	return {values, indices, {n_rows, n_vars}};
 }
 
 auto is_on_root_node(scip::Model& model) -> bool {
@@ -355,17 +356,18 @@ auto is_on_root_node(scip::Model& model) -> bool {
 
 auto extract_observation_fully(scip::Model& model) -> NodeBipartiteObs {
 	auto obs = NodeBipartiteObs{
-		xmatrix::from_shape({model.lp_columns().size(), NodeBipartiteObs::n_column_features}),
+		// Change this here for variables
+		xmatrix::from_shape({model.variables().size(), NodeBipartiteObs::n_variable_features}),
 		xmatrix::from_shape({n_ineq_rows(model), NodeBipartiteObs::n_row_features}),
 		extract_edge_features(model),
 	};
-	set_features_for_all_cols(obs.column_features, model, true);
+	set_features_for_all_vars(obs.variable_features, model, true);
 	set_features_for_all_rows(obs.row_features, model, true);
 	return obs;
 }
 
 auto extract_observation_from_cache(scip::Model& model, NodeBipartiteObs obs) -> NodeBipartiteObs {
-	set_features_for_all_cols(obs.column_features, model, false);
+	set_features_for_all_vars(obs.variable_features, model, false);
 	set_features_for_all_rows(obs.row_features, model, false);
 	return obs;
 }

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

@@ -30,30 +30,30 @@ TEST_CASE("NodeBipartite return correct observation", "[obs]") {
 
 	SECTION("Observation features are not empty") {
 		auto const& obs = optional_obs.value();
-		REQUIRE(obs.column_features.size() > 0);
+		REQUIRE(obs.variable_features.size() > 0);
 		REQUIRE(obs.row_features.size() > 0);
 		REQUIRE(obs.edge_features.nnz() > 0);
 	}
 
 	SECTION("Observation features have matching shape") {
 		auto const& obs = optional_obs.value();
 		REQUIRE(obs.row_features.shape()[0] == obs.edge_features.shape[0]);
-		REQUIRE(obs.column_features.shape()[0] == obs.edge_features.shape[1]);
+		REQUIRE(obs.variable_features.shape()[0] == obs.edge_features.shape[1]);
 		REQUIRE(obs.edge_features.indices.shape()[0] == 2);
 		REQUIRE(obs.edge_features.indices.shape()[1] == obs.edge_features.nnz());
 	}
 
-	SECTION("Columns features are not all nan") {
-		auto const& col_feat = optional_obs.value().column_features;
-		for (std::size_t i = 0; i < col_feat.shape()[1]; ++i) {
-			REQUIRE_FALSE(xt::all(xt::isnan(xt::col(col_feat, static_cast<std::ptrdiff_t>(i)))));
+	SECTION("Variable features are not all nan") {
+		auto const& var_feat = optional_obs.value().variable_features;
+		for (std::size_t i = 0; i < var_feat.shape()[1]; ++i) {
+			REQUIRE_FALSE(xt::all(xt::isnan(xt::col(var_feat, static_cast<std::ptrdiff_t>(i)))));
 		}
 	}
 
 	SECTION("Row features are not all nan") {
 		auto const& row_feat = optional_obs.value().row_features;
 		for (std::size_t i = 0; i < row_feat.shape()[1]; ++i) {
-			REQUIRE_FALSE(xt::all(xt::isnan(xt::row(row_feat, static_cast<std::ptrdiff_t>(i)))));
+			REQUIRE_FALSE(xt::all(xt::isnan(xt::col(row_feat, static_cast<std::ptrdiff_t>(i)))));
 		}
 	}
 }

python/src/ecole/core/observation.cpp

@@ -93,10 +93,10 @@ void bind_submodule(py::module_ const& m) {
 		Each edge is associated with the coefficient of the variable in the constraint.
 	)")
 			.def_auto_copy()
-			.def_auto_pickle(std::array{"column_features", "row_features", "edge_features"})
+			.def_auto_pickle(std::array{"variable_features", "row_features", "edge_features"})
 			.def_readwrite_xtensor(
-				"column_features",
-				&NodeBipartiteObs::column_features,
+				"variable_features",
+				&NodeBipartiteObs::variable_features,
 				"A matrix where each row is represents a variable, and each column a feature of the variables.")
 			.def_readwrite_xtensor(
 				"row_features",
@@ -108,26 +108,26 @@ void bind_submodule(py::module_ const& m) {
 				"The constraint matrix of the optimization problem, with rows for contraints and "
 				"columns for variables.");
 
-	py::enum_<NodeBipartiteObs::ColumnFeatures>(node_bipartite_obs, "ColumnFeatures")
-		.value("objective", NodeBipartiteObs::ColumnFeatures::objective)
-		.value("is_type_binary", NodeBipartiteObs::ColumnFeatures::is_type_binary)
-		.value("is_type_integer", NodeBipartiteObs::ColumnFeatures::is_type_integer)
-		.value("is_type_implicit_integer", NodeBipartiteObs::ColumnFeatures::is_type_implicit_integer)
-		.value("is_type_continuous", NodeBipartiteObs::ColumnFeatures::is_type_continuous)
-		.value("has_lower_bound", NodeBipartiteObs::ColumnFeatures::has_lower_bound)
-		.value("has_upper_bound", NodeBipartiteObs::ColumnFeatures::has_upper_bound)
-		.value("normed_reduced_cost", NodeBipartiteObs::ColumnFeatures::normed_reduced_cost)
-		.value("solution_value", NodeBipartiteObs::ColumnFeatures::solution_value)
-		.value("solution_frac", NodeBipartiteObs::ColumnFeatures::solution_frac)
-		.value("is_solution_at_lower_bound", NodeBipartiteObs::ColumnFeatures::is_solution_at_lower_bound)
-		.value("is_solution_at_upper_bound", NodeBipartiteObs::ColumnFeatures::is_solution_at_upper_bound)
-		.value("scaled_age", NodeBipartiteObs::ColumnFeatures::scaled_age)
-		.value("incumbent_value", NodeBipartiteObs::ColumnFeatures::incumbent_value)
-		.value("average_incumbent_value", NodeBipartiteObs::ColumnFeatures::average_incumbent_value)
-		.value("is_basis_lower", NodeBipartiteObs::ColumnFeatures::is_basis_lower)
-		.value("is_basis_basic", NodeBipartiteObs::ColumnFeatures::is_basis_basic)
-		.value("is_basis_upper", NodeBipartiteObs::ColumnFeatures::is_basis_upper)
-		.value("is_basis_zero", NodeBipartiteObs::ColumnFeatures ::is_basis_zero);
+	py::enum_<NodeBipartiteObs::VariableFeatures>(node_bipartite_obs, "VariableFeatures")
+		.value("objective", NodeBipartiteObs::VariableFeatures::objective)
+		.value("is_type_binary", NodeBipartiteObs::VariableFeatures::is_type_binary)
+		.value("is_type_integer", NodeBipartiteObs::VariableFeatures::is_type_integer)
+		.value("is_type_implicit_integer", NodeBipartiteObs::VariableFeatures::is_type_implicit_integer)
+		.value("is_type_continuous", NodeBipartiteObs::VariableFeatures::is_type_continuous)
+		.value("has_lower_bound", NodeBipartiteObs::VariableFeatures::has_lower_bound)
+		.value("has_upper_bound", NodeBipartiteObs::VariableFeatures::has_upper_bound)
+		.value("normed_reduced_cost", NodeBipartiteObs::VariableFeatures::normed_reduced_cost)
+		.value("solution_value", NodeBipartiteObs::VariableFeatures::solution_value)
+		.value("solution_frac", NodeBipartiteObs::VariableFeatures::solution_frac)
+		.value("is_solution_at_lower_bound", NodeBipartiteObs::VariableFeatures::is_solution_at_lower_bound)
+		.value("is_solution_at_upper_bound", NodeBipartiteObs::VariableFeatures::is_solution_at_upper_bound)
+		.value("scaled_age", NodeBipartiteObs::VariableFeatures::scaled_age)
+		.value("incumbent_value", NodeBipartiteObs::VariableFeatures::incumbent_value)
+		.value("average_incumbent_value", NodeBipartiteObs::VariableFeatures::average_incumbent_value)
+		.value("is_basis_lower", NodeBipartiteObs::VariableFeatures::is_basis_lower)
+		.value("is_basis_basic", NodeBipartiteObs::VariableFeatures::is_basis_basic)
+		.value("is_basis_upper", NodeBipartiteObs::VariableFeatures::is_basis_upper)
+		.value("is_basis_zero", NodeBipartiteObs::VariableFeatures ::is_basis_zero);
 
 	py::enum_<NodeBipartiteObs::RowFeatures>(node_bipartite_obs, "RowFeatures")
 		.value("bias", NodeBipartiteObs::RowFeatures::bias)

python/tests/test_observation.py

@@ -91,13 +91,13 @@ def test_NodeBipartite_observation(model):
     """Observation of NodeBipartite is a type with array attributes."""
     obs = make_obs(ecole.observation.NodeBipartite(), model)
     assert isinstance(obs, ecole.observation.NodeBipartiteObs)
-    assert_array(obs.column_features, ndim=2)
+    assert_array(obs.variable_features, ndim=2)
     assert_array(obs.row_features, ndim=2)
     assert_array(obs.edge_features.values)
     assert_array(obs.edge_features.indices, ndim=2, dtype=np.uint64)
 
     # Check that there are enums describing feeatures
-    assert len(obs.ColumnFeatures.__members__) == obs.column_features.shape[1]
+    assert len(obs.VariableFeatures.__members__) == obs.variable_features.shape[1]
     assert len(obs.RowFeatures.__members__) == obs.row_features.shape[1]