Merge pull request #150 from higra/fix_simplify_tree

Fix simplify_tree with process_leaves (again)

higra/hierarchy/py_hierarchy_core.cpp

@@ -70,23 +70,6 @@ struct def_quasi_flat_zone_hierarchy {
     }
 };
 
-template<typename graph_t>
-struct def_simplify_tree {
-    template<typename value_t, typename C>
-    static
-    void def(C &m, const char *doc) {
-        m.def("_simplify_tree",
-              [](const hg::tree &t, pyarray<value_t> &criterion, bool process_leaves) {
-                  return hg::simplify_tree(t, criterion, process_leaves);
-              },
-              doc,
-              py::arg("tree"),
-              py::arg("deleted_nodes"),
-              py::arg("process_leaves")
-        );
-    }
-};
-
 template<typename M>
 void add_simplified_tree(M &m) {
     using class_t = hg::remapped_tree<hg::tree, hg::array_1d<hg::index_t>>;
@@ -107,15 +90,17 @@ void py_init_hierarchy_core(pybind11::module &m) {
             (m,
              "Compute the canonical binary partition tree (binary tree by altitude ordering) of the given weighted graph."
             );
+
     add_simplified_tree(m);
-    add_type_overloads<def_simplify_tree<hg::ugraph>, HG_TEMPLATE_SNUMERIC_TYPES>
-            (m,
-             "Creates a copy of the current Tree and deletes the nodes such that the criterion function is true.\n"
-             "Also returns an array that maps any node index i of the new tree, to the index of this node in the original tree\n"
-             "\n"
-             "The criterion is an array that associates true (this node must be deleted) or\n"
-             "false (do not delete this node) to a node index."
-            );
+    m.def("_simplify_tree",
+          [](const hg::tree &t, pyarray<bool> &criterion, bool process_leaves) {
+              return hg::simplify_tree(t, criterion, process_leaves);
+          },
+          "",
+          py::arg("tree"),
+          py::arg("deleted_nodes"),
+          py::arg("process_leaves"));
+
     add_type_overloads<def_quasi_flat_zone_hierarchy<hg::ugraph>, HG_TEMPLATE_SNUMERIC_TYPES>
             (m,
              "Compute the quasi flat zones hierarchy of the given weighted graph."

include/higra/hierarchy/hierarchy_core.hpp

@@ -150,16 +150,16 @@ namespace hg {
 
             // ********************************
             // identification of deleted sub-trees
-            // true if all nodes below a given node are deleted
-            // a non leaf node i such that removed_branch(i) && !removed_branch(parent(i)) is thus new leaf
+            // true if all nodes below a given  node are deleted
+            // a non deleted non leaf node i such that removed_branch(i) && !removed_branch(parent(i)) is thus a new leaf
             array_1d<bool> removed_branch = xt::zeros<bool>({num_vertices(t)});
-            for (index_t i = 0; i < (index_t)num_leaves(t); i++) {
+            for (index_t i = 0; i < (index_t) num_leaves(t); i++) {
                 removed_branch(i) = criterion(i);
             }
 
-            for (index_t i = num_leaves(t); i < (index_t)num_vertices(t); i++) {
+            for (index_t i = num_leaves(t); i < (index_t) num_vertices(t); i++) {
                 bool flag = true;
-                for (index_t c = 0; flag && c < (index_t)num_children(i, t); c++) {
+                for (index_t c = 0; flag && c < (index_t) num_children(i, t); c++) {
                     auto cc = child(c, i, t);
                     flag = flag && removed_branch(cc) && criterion(cc);
                 }
@@ -172,19 +172,20 @@ namespace hg {
             std::vector<index_t> new_leaves;
             index_t removed = 0;
 
-            for (index_t i : leaves_iterator(t)){
-                if(!criterion(i)){
+            for (index_t i : leaves_iterator(t)) {
+                if (!removed_branch(i)) {
                     new_leaves.push_back(i);
-                }else{
+                } else {
                     removed++;
                 }
             }
 
             for (index_t i : leaves_to_root_iterator(t, leaves_it::exclude, root_it::exclude)) {
-                if (removed_branch(i) && !removed_branch(parent(i, t))) {
-                    new_leaves.push_back(i);
-                }
-                if(criterion(i)){
+                if (!criterion(i)) {
+                    if (removed_branch(i) && !removed_branch(parent(i, t))) {
+                        new_leaves.push_back(i);
+                    }
+                } else {
                     removed++;
                 }
             }
@@ -206,7 +207,7 @@ namespace hg {
 
             // new index of each node
             array_1d<index_t> new_order({num_vertices(t)}, invalid_index);
-            for (index_t i = 0; i < (index_t)new_leaves.size(); i++) {
+            for (index_t i = 0; i < (index_t) new_leaves.size(); i++) {
                 new_order(new_leaves[i]) = i;
             }
 
@@ -215,12 +216,12 @@ namespace hg {
             while (!queue.empty()) {
                 auto e = queue.front();
                 queue.pop();
-                if(!criterion(e) || e == root(t)){
+                if (!criterion(e) || e == root(t)) {
                     new_order(e) = node_number;
                     new_parent(node_number) = new_order(parent(e, t));
                     node_map(node_number) = e;
                     node_number--;
-                }else{
+                } else {
                     new_order(e) = new_order(parent(e, t));
                 }
 

test/cpp/hierarchy/test_hierarchy_core.cpp

@@ -138,6 +138,42 @@ namespace hierarchy_core {
         REQUIRE(xt::amax(xt::index_view(criterion, nm))() == false);
     }
 
+    TEST_CASE("simplify tree remove leaves3", "[hierarchy_core]") {
+
+        tree t(xt::xarray<index_t>{7, 7, 8, 8, 8, 9, 9, 11, 10, 10, 11, 11});
+
+        array_1d<bool> criterion{true, true, true, true, true, true, true, true, false, false, false, false};
+
+        auto res = hg::simplify_tree(t, criterion, true);
+        auto nt = res.tree;
+        auto nm = res.node_map;
+
+        array_1d<index_t> refp{2, 2, 3, 3};
+        tree ref_tree(refp);
+        REQUIRE(test_tree_isomorphism(nt, ref_tree));
+
+
+        REQUIRE(xt::amax(xt::index_view(criterion, nm))() == false);
+    }
+
+    TEST_CASE("simplify tree remove leaves4", "[hierarchy_core]") {
+
+        tree t(xt::xarray<index_t>{7, 7, 8, 8, 8, 9, 9, 11, 10, 10, 11, 11});
+
+        array_1d<bool> criterion{true, true, true, true, true, true, true, true, true, false, false, false};
+
+        auto res = hg::simplify_tree(t, criterion, true);
+        auto nt = res.tree;
+        auto nm = res.node_map;
+
+        array_1d<index_t> refp{1, 2, 2};
+        tree ref_tree(refp);
+        REQUIRE(test_tree_isomorphism(nt, ref_tree));
+
+        criterion(root(t)) = false;
+        REQUIRE(xt::amax(xt::index_view(criterion, nm))() == false);
+    }
+
     TEST_CASE("simplify tree remove leaves trivial", "[hierarchy_core]") {
 
         tree t(xt::xarray<index_t>{2, 2, 2});

test/python/test_hierarchy/test_hierarchy_core.py

@@ -109,6 +109,30 @@ def test_simplify_tree_with_leaves(self):
 
         self.assertFalse(np.max(criterion[node_map]))
 
+    def test_simplify_tree_with_leaves2(self):
+        t = hg.Tree((7, 7, 8, 8, 8, 9, 9, 11, 10, 10, 11, 11))
+
+        criterion = np.zeros(t.num_vertices(), dtype=np.bool)
+        criterion[:8] = True
+        new_tree, node_map = hg.simplify_tree(t, criterion, process_leaves=True)
+
+        ref_tree = hg.Tree((2, 2, 3, 3))
+        self.assertTrue(hg.test_tree_isomorphism(new_tree, ref_tree))
+
+        self.assertFalse(np.max(criterion[node_map]))
+
+    def test_simplify_tree_with_leaves3(self):
+        t = hg.Tree((7, 7, 8, 8, 8, 9, 9, 11, 10, 10, 11, 11))
+
+        criterion = np.zeros(t.num_vertices(), dtype=np.bool)
+        criterion[:9] = True
+        new_tree, node_map = hg.simplify_tree(t, criterion, process_leaves=True)
+
+        ref_tree = hg.Tree((1, 2, 2))
+        self.assertTrue(hg.test_tree_isomorphism(new_tree, ref_tree))
+
+        self.assertFalse(np.max(criterion[node_map]))
+
     def test_simplify_tree_propagate_category(self):
         g = hg.get_4_adjacency_implicit_graph((1, 6))
         vertex_values = np.asarray((1, 5, 4, 3, 3, 6), dtype=np.int32)