Expose 'SufficientCoalitions::upset_roots'

lincs/liblincs/io/model.cpp

@@ -101,6 +101,22 @@ JsonValidator validator(schema);
 
 }  // namespace
 
+std::vector<std::vector<unsigned>> SufficientCoalitions::get_upset_roots() const {
+  std::vector<std::vector<unsigned>> roots;
+
+  roots.reserve(upset_roots.size());
+  for (const auto& upset_root : upset_roots) {
+    std::vector<unsigned>& root = roots.emplace_back();
+    for (unsigned criterion_index = 0; criterion_index != upset_root.size(); ++criterion_index) {
+      if (upset_root[criterion_index]) {
+        root.emplace_back(criterion_index);
+      }
+    }
+  }
+
+  return roots;
+}
+
 void Model::dump(const Problem& problem, std::ostream& os) const {
   CHRONE();
 
@@ -134,19 +150,13 @@ void Model::dump(const Problem& problem, std::ostream& os) const {
           break;
         case SufficientCoalitions::Kind::roots:
           out << YAML::Key << "upset_roots" << YAML::Value;
-          const auto upset_roots = boundary.sufficient_coalitions.upset_roots;
+          const std::vector<std::vector<unsigned>> upset_roots = boundary.sufficient_coalitions.get_upset_roots();
           if (upset_roots.empty()) {
             out << YAML::Flow;
           }
           out << YAML::BeginSeq;
-          for (const auto& root : upset_roots) {
-            out << YAML::Flow << YAML::BeginSeq;
-            for (unsigned criterion_index = 0; criterion_index != problem.criteria.size(); ++criterion_index) {
-              if (root[criterion_index]) {
-                out << criterion_index;
-              }
-            }
-            out << YAML::EndSeq;
+          for (const std::vector<unsigned>& upset_root : upset_roots) {
+            out << YAML::Flow << upset_root;
           }
           out << YAML::EndSeq;
           break;

lincs/liblincs/io/model.hpp

@@ -36,9 +36,9 @@ struct SufficientCoalitions {
   SufficientCoalitions(Roots, const unsigned criteria_count, const std::vector<std::vector<unsigned>>& upset_roots_) : kind(Kind::roots), upset_roots() {
     upset_roots.reserve(upset_roots_.size());
     for (const auto& root: upset_roots_) {
-      upset_roots.emplace_back(criteria_count);
+      boost::dynamic_bitset<>& upset_root = upset_roots.emplace_back(criteria_count);
       for (unsigned criterion_index: root) {
-        upset_roots.back()[criterion_index] = true;
+        upset_root[criterion_index] = true;
       }
     }
   }
@@ -48,6 +48,8 @@ struct SufficientCoalitions {
     upset_roots(upset_roots_)
   {}
 
+  std::vector<std::vector<unsigned>> get_upset_roots() const;
+
   bool operator==(const SufficientCoalitions& other) const {
     return kind == other.kind && criterion_weights == other.criterion_weights && upset_roots == other.upset_roots;
   }

lincs/liblincs/liblincs_module.cpp

@@ -88,42 +88,80 @@ lincs::Alternatives load_alternatives(const lincs::Problem& problem, bp::object&
   return lincs::Alternatives::load(problem, in_stream);
 }
 
-// @todo(Project management, soon) Thoroughly review all conversions between Python and C++ types.
-// - read Boost.Python doc in details and understand the contract
-// - homogenize converters (some were copy-pasted from SO answers and even ChatGPT)
-// - double-check if/when we need to increment reference counts on Python objects
-// https://stackoverflow.com/a/15940413/905845
-// https://misspent.wordpress.com/2009/09/27/how-to-write-boost-python-converters/
-struct iterable_converter {
-  template <typename Container>
-  iterable_converter& from_python() {
+template<typename T>
+struct std_vector_converter {
+  static void* convertible(PyObject* obj) {
+    if (PyObject_GetIter(obj)) {
+      return obj;
+    } else {
+      return nullptr;
+    }
+  }
+
+  static void construct(PyObject* obj, bp::converter::rvalue_from_python_stage1_data* data) {
+    bp::handle<> handle(bp::borrowed(obj));
+
+    typedef bp::converter::rvalue_from_python_storage<std::vector<T>> storage_type;
+    void* storage = reinterpret_cast<storage_type*>(data)->storage.bytes;
+
+    typedef bp::stl_input_iterator<typename std::vector<T>::value_type> iterator;
+
+    new (storage) std::vector<T>(iterator(bp::object(handle)), iterator());
+    data->convertible = storage;
+  }
+
+  static void enroll() {
+    // No need for 'bp::to_python_converter': already implemented by Boost.Python
     bp::converter::registry::push_back(
-      &iterable_converter::convertible,
-      &iterable_converter::construct<Container>,
-      bp::type_id<Container>());
+      &std_vector_converter<T>::convertible,
+      &std_vector_converter<T>::construct,
+      bp::type_id<std::vector<T>>()
+    );
+  }
+};
 
-    return *this;
+template<typename T>
+struct std_vector_converter<std::vector<T>> {
+  static PyObject* convert(const std::vector<std::vector<T>>& vvv) {
+    bp::list result;
+    for (const std::vector<T>& vv : vvv) {
+      bp::list sublist;
+      for (const T& v : vv) {
+        sublist.append(v);
+      }
+      result.append(sublist);
+    }
+    return bp::incref(result.ptr());
   }
 
-  static void* convertible(PyObject* object) {
-    return PyObject_GetIter(object) ? object : NULL;
+  static void* convertible(PyObject* obj) {
+    if (PyObject_GetIter(obj)) {
+      return obj;
+    } else {
+      return nullptr;
+    }
   }
 
-  template <typename Container>
-  static void construct(
-    PyObject* object,
-    bp::converter::rvalue_from_python_stage1_data* data
-  ) {
-    bp::handle<> handle(bp::borrowed(object));
+  static void construct(PyObject* obj, bp::converter::rvalue_from_python_stage1_data* data) {
+    bp::handle<> handle(bp::borrowed(obj));
 
-    typedef bp::converter::rvalue_from_python_storage<Container> storage_type;
+    typedef bp::converter::rvalue_from_python_storage<std::vector<std::vector<T>>> storage_type;
     void* storage = reinterpret_cast<storage_type*>(data)->storage.bytes;
 
-    typedef bp::stl_input_iterator<typename Container::value_type> iterator;
+    typedef bp::stl_input_iterator<typename std::vector<std::vector<T>>::value_type> iterator;
 
-    new (storage) Container(iterator(bp::object(handle)), iterator());
+    new (storage) std::vector<std::vector<T>>(iterator(bp::object(handle)), iterator());
     data->convertible = storage;
   }
+
+  static void enroll() {
+    bp::to_python_converter<std::vector<std::vector<T>>, std_vector_converter<std::vector<T>>>();
+    bp::converter::registry::push_back(
+      &std_vector_converter<std::vector<T>>::convertible,
+      &std_vector_converter<std::vector<T>>::construct,
+      bp::type_id<std::vector<std::vector<T>>>()
+    );
+  }
 };
 
 template<typename T>
@@ -196,18 +234,16 @@ auto auto_enum(const std::string& name) {
 }
 
 BOOST_PYTHON_MODULE(liblincs) {
-  iterable_converter()
-    .from_python<std::vector<float>>()
-    .from_python<std::vector<unsigned>>()
-    .from_python<std::vector<std::vector<unsigned>>>()
-    .from_python<std::vector<lincs::Category>>()
-    .from_python<std::vector<lincs::Criterion>>()
-    .from_python<std::vector<lincs::Model::Boundary>>()
-    .from_python<std::vector<lincs::SufficientCoalitions>>()
-    .from_python<std::vector<lincs::Alternative>>()
-    .from_python<std::vector<lincs::LearnMrsortByWeightsProfilesBreed::TerminationStrategy*>>()
-    .from_python<std::vector<lincs::LearnMrsortByWeightsProfilesBreed::Observer*>>()
-  ;
+  std_vector_converter<float>::enroll();
+  std_vector_converter<unsigned>::enroll();
+  std_vector_converter<std::vector<unsigned>>::enroll();
+  std_vector_converter<lincs::Category>::enroll();
+  std_vector_converter<lincs::Criterion>::enroll();
+  std_vector_converter<lincs::Model::Boundary>::enroll();
+  std_vector_converter<lincs::SufficientCoalitions>::enroll();
+  std_vector_converter<lincs::Alternative>::enroll();
+  std_vector_converter<lincs::LearnMrsortByWeightsProfilesBreed::TerminationStrategy*>::enroll();
+  std_vector_converter<lincs::LearnMrsortByWeightsProfilesBreed::Observer*>::enroll();
 
   std_optional_converter<float>::enroll();
   std_optional_converter<unsigned>::enroll();
@@ -275,6 +311,7 @@ BOOST_PYTHON_MODULE(liblincs) {
     .def(bp::init<lincs::SufficientCoalitions::Roots, unsigned, std::vector<std::vector<unsigned>>>())
     .def_readwrite("kind", &lincs::SufficientCoalitions::kind)
     .def_readwrite("criterion_weights", &lincs::SufficientCoalitions::criterion_weights)
+    .add_property("upset_roots", &lincs::SufficientCoalitions::get_upset_roots)
   ;
   sufficient_coalitions_class.attr("Kind") = auto_enum<lincs::SufficientCoalitions::Kind>("Kind");
   sufficient_coalitions_class.attr("weights") = lincs::SufficientCoalitions::weights;

lincs/liblincs_module_tests.py

@@ -130,15 +130,18 @@ def test_init_three_criteria_two_categories_weights_boundary(self):
             [
                 Model.Boundary(
                     [5., 5., 5],
-                    SufficientCoalitions(SufficientCoalitions.weights, [0.7, 0.7, 1])
+                    SufficientCoalitions(SufficientCoalitions.weights, [0.5, 0.25, 1])
                 ),
             ],
         )
         self.assertEqual(len(model.boundaries), 1)
         self.assertEqual(len(model.boundaries[0].profile), 3)
         self.assertEqual(model.boundaries[0].sufficient_coalitions.kind, SufficientCoalitions.Kind.weights)
         self.assertEqual(len(model.boundaries[0].sufficient_coalitions.criterion_weights), 3)
-        # @todo(Feature, when upset_roots are exposed, v1) self.assertEqual(len(model.boundaries[0].sufficient_coalitions.upset_roots), 0)
+        self.assertEqual(model.boundaries[0].sufficient_coalitions.criterion_weights[0], 0.5)
+        self.assertEqual(model.boundaries[0].sufficient_coalitions.criterion_weights[1], 0.25)
+        self.assertEqual(model.boundaries[0].sufficient_coalitions.criterion_weights[2], 1)
+        self.assertEqual(len(model.boundaries[0].sufficient_coalitions.upset_roots), 0)
 
     def test_init_three_criteria_two_categories_roots_boundary(self):
         problem = Problem(
@@ -164,7 +167,12 @@ def test_init_three_criteria_two_categories_roots_boundary(self):
         self.assertEqual(len(model.boundaries[0].profile), 3)
         self.assertEqual(model.boundaries[0].sufficient_coalitions.kind, SufficientCoalitions.Kind.roots)
         self.assertEqual(len(model.boundaries[0].sufficient_coalitions.criterion_weights), 0)
-        # @todo(Feature, when upset_roots are exposed, v1) self.assertEqual(len(model.boundaries[0].sufficient_coalitions.upset_roots), 2)
+        self.assertEqual(len(model.boundaries[0].sufficient_coalitions.upset_roots), 2)
+        self.assertEqual(len(model.boundaries[0].sufficient_coalitions.upset_roots[0]), 2)
+        self.assertEqual(model.boundaries[0].sufficient_coalitions.upset_roots[0][0], 0)
+        self.assertEqual(model.boundaries[0].sufficient_coalitions.upset_roots[0][1], 1)
+        self.assertEqual(model.boundaries[0].sufficient_coalitions.upset_roots[1][0], 0)
+        self.assertEqual(model.boundaries[0].sufficient_coalitions.upset_roots[1][1], 2)
 
     def test_assign_model_attributes(self):
         problem = Problem([], [])