From dbb2340692fd420b5736e46d421fb4ce5839952a Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Tue, 5 Jul 2016 16:13:16 +0200
Subject: [PATCH] Fixed has_contiguous_segments implementation.

The previous implementation was failing when nodes were numbered in
breadth-first order. Also gave a more descriptive name to the
algorithm.
---
 src/algorithms.hpp        | 45 +++++++++++++++++----------------------
 src/swcio.cpp             |  2 +-
 tests/test_algorithms.cpp | 27 +++++++++++++++++------
 3 files changed, 40 insertions(+), 34 deletions(-)

diff --git a/src/algorithms.hpp b/src/algorithms.hpp
index a1f6bad6..1d55535f 100644
--- a/src/algorithms.hpp
+++ b/src/algorithms.hpp
@@ -121,49 +121,42 @@ bool is_positive(C const& c)
 }
 
 template<typename C>
-bool has_contiguous_segments(const C &parent_index)
+std::vector<typename C::value_type> child_count(const C& parent_index)
 {
     static_assert(
         std::is_integral<typename C::value_type>::value,
         "integral type required"
     );
 
-    if (!is_minimal_degree(parent_index)) {
-        return false;
-    }
-
-    int n = parent_index.size();
-    std::vector<bool> is_leaf(n, false);
-
-    for(auto i=1; i<n; ++i) {
-        auto p = parent_index[i];
-        if(is_leaf[p]) {
-            return false;
-        }
-
-        if(p != i-1) {
-            // we have a branch and i-1 is a leaf node
-            is_leaf[i-1] = true;
-        }
+    std::vector<typename C::value_type> count(parent_index.size(), 0);
+    for (auto i = 1u; i < parent_index.size(); ++i) {
+        ++count[parent_index[i]];
     }
 
-    return true;
+    return count;
 }
 
 template<typename C>
-std::vector<typename C::value_type> child_count(const C &parent_index)
+bool has_contiguous_compartments(const C& parent_index)
 {
     static_assert(
         std::is_integral<typename C::value_type>::value,
         "integral type required"
     );
 
-    std::vector<typename C::value_type> count(parent_index.size(), 0);
-    for (std::size_t i = 1; i < parent_index.size(); ++i) {
-        ++count[parent_index[i]];
+    if (!is_minimal_degree(parent_index)) {
+        return false;
     }
 
-    return count;
+    auto num_child = child_count(parent_index);
+    for (auto i = 1u; i < parent_index.size(); ++i) {
+        auto p = parent_index[i];
+        if (num_child[p] == 1 && p != i-1) {
+            return false;
+        }
+    }
+
+    return true;
 }
 
 template<typename C>
@@ -174,7 +167,7 @@ std::vector<typename C::value_type> branches(const C& parent_index)
         "integral type required"
     );
 
-    EXPECTS(has_contiguous_segments(parent_index));
+    EXPECTS(has_contiguous_compartments(parent_index));
 
     std::vector<typename C::value_type> branch_index;
     if (parent_index.empty()) {
@@ -250,7 +243,7 @@ std::vector<typename C::value_type> make_parent_index(
     }
 
     EXPECTS(parent_index.size() == branch_index.back());
-    EXPECTS(has_contiguous_segments(parent_index));
+    EXPECTS(has_contiguous_compartments(parent_index));
     EXPECTS(is_strictly_monotonic_increasing(branch_index));
 
     // expand the branch index
diff --git a/src/swcio.cpp b/src/swcio.cpp
index 3f38f8e7..db8001be 100644
--- a/src/swcio.cpp
+++ b/src/swcio.cpp
@@ -261,7 +261,7 @@ swc_record_range_clean::swc_record_range_clean(std::istream& is)
         parent_list.push_back(records_[i].parent());
     }
 
-    if (!nest::mc::algorithms::has_contiguous_segments(parent_list)) {
+    if (!nest::mc::algorithms::has_contiguous_compartments(parent_list)) {
         throw swc_parse_error("branches are not contiguously numbered", 0);
     }
 }
diff --git a/tests/test_algorithms.cpp b/tests/test_algorithms.cpp
index d1ffdf11..1a363be6 100644
--- a/tests/test_algorithms.cpp
+++ b/tests/test_algorithms.cpp
@@ -172,7 +172,7 @@ TEST(algorithms, is_positive)
     );
 }
 
-TEST(algorithms, has_contiguous_segments)
+TEST(algorithms, has_contiguous_compartments)
 {
     //
     //       0
@@ -186,7 +186,7 @@ TEST(algorithms, has_contiguous_segments)
     //   5       6
     //
     EXPECT_FALSE(
-        nest::mc::algorithms::has_contiguous_segments(
+        nest::mc::algorithms::has_contiguous_compartments(
             std::vector<int>{0, 0, 1, 2, 2, 3, 4, 2}
         )
     );
@@ -203,7 +203,7 @@ TEST(algorithms, has_contiguous_segments)
     //   4       7
     //
     EXPECT_FALSE(
-        nest::mc::algorithms::has_contiguous_segments(
+        nest::mc::algorithms::has_contiguous_compartments(
             std::vector<int>{0, 0, 1, 2, 3, 2, 2, 5}
         )
     );
@@ -220,7 +220,7 @@ TEST(algorithms, has_contiguous_segments)
     //   4       6
     //
     EXPECT_TRUE(
-        nest::mc::algorithms::has_contiguous_segments(
+        nest::mc::algorithms::has_contiguous_compartments(
             std::vector<int>{0, 0, 1, 2, 3, 2, 5, 2}
         )
     );
@@ -237,21 +237,34 @@ TEST(algorithms, has_contiguous_segments)
     //   4       6
     //
     EXPECT_TRUE(
-        nest::mc::algorithms::has_contiguous_segments(
+        nest::mc::algorithms::has_contiguous_compartments(
             std::vector<int>{0, 0, 1, 2, 3, 2, 5, 1}
         )
     );
 
+    //
+    //     0
+    //    / \
+    //   1   2
+    //  / \
+    // 3   4
+    //
+    EXPECT_TRUE(
+        nest::mc::algorithms::has_contiguous_compartments(
+            std::vector<int>{0, 0, 0, 1, 1}
+        )
+    );
+
     // Soma-only list
     EXPECT_TRUE(
-        nest::mc::algorithms::has_contiguous_segments(
+        nest::mc::algorithms::has_contiguous_compartments(
             std::vector<int>{0}
         )
     );
 
     // Empty list
     EXPECT_TRUE(
-        nest::mc::algorithms::has_contiguous_segments(
+        nest::mc::algorithms::has_contiguous_compartments(
             std::vector<int>{}
         )
     );
-- 
GitLab