From d04de96a8d9592f4f9072299674d9a2d3d63e0cd Mon Sep 17 00:00:00 2001
From: Vasileios Karakasis <karakasis@cscs.ch>
Date: Mon, 18 Apr 2016 10:23:49 +0200
Subject: [PATCH] Algorithm for checking contiguous numbering in cell branches.
+ converted swc_record::kind to an ``enum class``
---
src/algorithms.hpp | 26 ++++++++++++
src/swcio.cpp | 9 ++--
src/swcio.hpp | 14 +++----
tests/test_algorithms.cpp | 86 +++++++++++++++++++++++++++++++++++++++
tests/test_swcio.cpp | 61 +++++++++++++++++++--------
vector | 2 +-
6 files changed, 166 insertions(+), 32 deletions(-)
diff --git a/src/algorithms.hpp b/src/algorithms.hpp
index 250cade2..d627b755 100644
--- a/src/algorithms.hpp
+++ b/src/algorithms.hpp
@@ -99,6 +99,32 @@ namespace algorithms{
return true;
}
+ template<typename C>
+ bool is_contiguously_numbered(const C &parent_list)
+ {
+ static_assert(
+ std::is_integral<typename C::value_type>::value,
+ "integral type required"
+ );
+
+ std::vector<bool> is_leaf(parent_list.size(), false);
+
+ auto ret = true;
+ for (std::size_t i = 1; i < parent_list.size(); ++i) {
+ if (is_leaf[parent_list[i]]) {
+ ret = false;
+ break;
+ }
+
+ if (parent_list[i] != i-1) {
+ // we have a branch and i-1 is a leaf node
+ is_leaf[i-1] = true;
+ }
+ }
+
+ return ret;
+ }
+
} // namespace algorithms
} // namespace mc
} // namespace nest
diff --git a/src/swcio.cpp b/src/swcio.cpp
index 98db848e..2c3fe4d2 100644
--- a/src/swcio.cpp
+++ b/src/swcio.cpp
@@ -32,7 +32,8 @@ void swc_record::check_consistency() const
{
// Check record type as well; enum's do not offer complete type safety,
// since you can cast anything that fits to its underlying type
- if (type_ < 0 || type_ > custom) {
+ if (static_cast<int>(type_) < 0 ||
+ static_cast<int>(type_) > static_cast<int>(kind::custom)) {
throw std::invalid_argument("unknown record type");
}
@@ -65,7 +66,7 @@ std::ostream &operator<<(std::ostream &os, const swc_record &record)
{
// output in one-based indexing
os << record.id_+1 << " "
- << record.type_ << " "
+ << static_cast<int>(record.type_) << " "
<< std::setprecision(7) << record.x_ << " "
<< std::setprecision(7) << record.y_ << " "
<< std::setprecision(7) << record.z_ << " "
@@ -177,9 +178,9 @@ swc_record_range_clean::swc_record_range_clean(std::istream &is)
{
std::unordered_set<swc_record::id_type> ids;
- std::size_t num_trees = 0;
+ std::size_t num_trees = 0;
swc_record::id_type last_id = -1;
- bool needsort = false;
+ bool needsort = false;
swc_record curr_record;
for (auto c : swc_get_records<swc_io_raw>(is)) {
diff --git a/src/swcio.hpp b/src/swcio.hpp
index 0486a2b5..6a4af5f8 100644
--- a/src/swcio.hpp
+++ b/src/swcio.hpp
@@ -15,11 +15,8 @@ class swc_record
public:
using id_type = int;
- // FIXME: enum's are not completely type-safe, since they can accept
- // anything that can be casted to their underlying type.
- //
// More on SWC files: http://research.mssm.edu/cnic/swc.html
- enum kind {
+ enum class kind {
undefined = 0,
soma,
axon,
@@ -31,9 +28,9 @@ public:
};
// swc records assume zero-based indexing; root's parent remains -1
- swc_record(kind type, int id,
- float x, float y, float z, float r,
- int parent_id)
+ swc_record(swc_record::kind type, int id,
+ float x, float y, float z, float r,
+ int parent_id)
: type_(type)
, id_(id)
, x_(x)
@@ -46,7 +43,7 @@ public:
}
swc_record()
- : type_(swc_record::undefined)
+ : type_(swc_record::kind::undefined)
, id_(0)
, x_(0)
, y_(0)
@@ -409,4 +406,3 @@ template<typename T = swc_io_clean>
} // namespace io
} // namespace mc
} // namespace nest
-
diff --git a/tests/test_algorithms.cpp b/tests/test_algorithms.cpp
index decf1316..3e59db7e 100644
--- a/tests/test_algorithms.cpp
+++ b/tests/test_algorithms.cpp
@@ -169,3 +169,89 @@ TEST(algorithms, is_positive)
)
);
}
+
+TEST(algorithms, is_contiguously_numbered)
+{
+ //
+ // 0
+ // |
+ // 1
+ // |
+ // 2
+ // /|\
+ // 3 7 4
+ // / \
+ // 5 6
+ //
+ EXPECT_FALSE(
+ nest::mc::algorithms::is_contiguously_numbered(
+ std::vector<int>{0, 0, 1, 2, 2, 3, 4, 2}
+ )
+ );
+
+ //
+ // 0
+ // |
+ // 1
+ // |
+ // 2
+ // /|\
+ // 3 6 5
+ // / \
+ // 4 7
+ //
+ EXPECT_FALSE(
+ nest::mc::algorithms::is_contiguously_numbered(
+ std::vector<int>{0, 0, 1, 2, 3, 2, 2, 5}
+ )
+ );
+
+ //
+ // 0
+ // |
+ // 1
+ // |
+ // 2
+ // /|\
+ // 3 7 5
+ // / \
+ // 4 6
+ //
+ EXPECT_TRUE(
+ nest::mc::algorithms::is_contiguously_numbered(
+ std::vector<int>{0, 0, 1, 2, 3, 2, 5, 2}
+ )
+ );
+
+ //
+ // 0
+ // |
+ // 1
+ // / \
+ // 2 7
+ // / \
+ // 3 5
+ // / \
+ // 4 6
+ //
+ EXPECT_TRUE(
+ nest::mc::algorithms::is_contiguously_numbered(
+ std::vector<int>{0, 0, 1, 2, 3, 2, 5, 1}
+ )
+ );
+
+ // Soma-only list
+ EXPECT_TRUE(
+ nest::mc::algorithms::is_contiguously_numbered(
+ std::vector<int>{0}
+ )
+ );
+
+ // Empty list
+ EXPECT_TRUE(
+ nest::mc::algorithms::is_contiguously_numbered(
+ std::vector<int>{}
+ )
+ );
+
+}
diff --git a/tests/test_swcio.cpp b/tests/test_swcio.cpp
index 3de060ed..635ccb44 100644
--- a/tests/test_swcio.cpp
+++ b/tests/test_swcio.cpp
@@ -36,43 +36,43 @@ TEST(swc_record, construction)
{
// invalid id
EXPECT_THROW(swc_record record(
- swc_record::custom, -3, 1., 1., 1., 1., 5),
+ swc_record::kind::custom, -3, 1., 1., 1., 1., 5),
std::invalid_argument);
}
{
// invalid parent id
EXPECT_THROW(swc_record record(
- swc_record::custom, 0, 1., 1., 1., 1., -5),
+ swc_record::kind::custom, 0, 1., 1., 1., 1., -5),
std::invalid_argument);
}
{
// invalid radius
EXPECT_THROW(swc_record record(
- swc_record::custom, 0, 1., 1., 1., -1., -1),
+ swc_record::kind::custom, 0, 1., 1., 1., -1., -1),
std::invalid_argument);
}
{
// parent_id > id
EXPECT_THROW(swc_record record(
- swc_record::custom, 0, 1., 1., 1., 1., 2),
+ swc_record::kind::custom, 0, 1., 1., 1., 1., 2),
std::invalid_argument);
}
{
// parent_id == id
EXPECT_THROW(swc_record record(
- swc_record::custom, 0, 1., 1., 1., 1., 0),
+ swc_record::kind::custom, 0, 1., 1., 1., 1., 0),
std::invalid_argument);
}
{
// check standard construction by value
- swc_record record(swc_record::custom, 0, 1., 1., 1., 1., -1);
+ swc_record record(swc_record::kind::custom, 0, 1., 1., 1., 1., -1);
EXPECT_EQ(record.id(), 0);
- EXPECT_EQ(record.type(), swc_record::custom);
+ EXPECT_EQ(record.type(), swc_record::kind::custom);
EXPECT_EQ(record.x(), 1.);
EXPECT_EQ(record.y(), 1.);
EXPECT_EQ(record.z(), 1.);
@@ -83,7 +83,7 @@ TEST(swc_record, construction)
{
// check copy constructor
- swc_record record_orig(swc_record::custom, 0, 1., 1., 1., 1., -1);
+ swc_record record_orig(swc_record::kind::custom, 0, 1., 1., 1., 1., -1);
swc_record record(record_orig);
expect_record_equals(record_orig, record);
}
@@ -95,9 +95,9 @@ TEST(swc_record, comparison)
{
// check comparison operators
- swc_record record0(swc_record::custom, 0, 1., 1., 1., 1., -1);
- swc_record record1(swc_record::custom, 0, 2., 3., 4., 5., -1);
- swc_record record2(swc_record::custom, 1, 2., 3., 4., 5., -1);
+ swc_record record0(swc_record::kind::custom, 0, 1., 1., 1., 1., -1);
+ swc_record record1(swc_record::kind::custom, 0, 2., 3., 4., 5., -1);
+ swc_record record2(swc_record::kind::custom, 1, 2., 3., 4., 5., -1);
EXPECT_EQ(record0, record1);
EXPECT_LT(record0, record2);
EXPECT_GT(record2, record1);
@@ -131,6 +131,31 @@ TEST(swc_parser, invalid_input)
swc_record record;
EXPECT_THROW(is >> record, swc_parse_error);
}
+
+ {
+ // Non-contiguous numbering in branches is considered invalid
+ // 1
+ // / \
+ // 2 3
+ // /
+ // 4
+ std::stringstream is;
+ is << "1 1 14.566132 34.873772 7.857000 0.717830 -1\n";
+ is << "2 1 14.566132 34.873772 7.857000 0.717830 1\n";
+ is << "3 1 14.566132 34.873772 7.857000 0.717830 1\n";
+ is << "4 1 14.566132 34.873772 7.857000 0.717830 2\n";
+
+ std::vector<swc_record> records;
+ try {
+ for (auto c : swc_get_records<swc_io_clean>(is)) {
+ records.push_back(std::move(c));
+ }
+
+ FAIL() << "expected swc_parse_error, none was thrown\n";
+ } catch (const swc_parse_error &e) {
+ SUCCEED();
+ }
+ }
}
@@ -162,7 +187,7 @@ TEST(swc_parser, valid_input)
swc_record record;
EXPECT_NO_THROW(is >> record);
EXPECT_EQ(0, record.id()); // zero-based indexing
- EXPECT_EQ(swc_record::soma, record.type());
+ EXPECT_EQ(swc_record::kind::soma, record.type());
EXPECT_FLOAT_EQ(14.566132, record.x());
EXPECT_FLOAT_EQ(34.873772, record.y());
EXPECT_FLOAT_EQ( 7.857000, record.z());
@@ -173,9 +198,9 @@ TEST(swc_parser, valid_input)
{
// check valid input with a series of records
std::vector<swc_record> records_orig = {
- swc_record(swc_record::soma, 0,
+ swc_record(swc_record::kind::soma, 0,
14.566132, 34.873772, 7.857000, 0.717830, -1),
- swc_record(swc_record::dendrite, 1,
+ swc_record(swc_record::kind::dendrite, 1,
14.566132+1, 34.873772+1, 7.857000+1, 0.717830+1, -1)
};
@@ -229,7 +254,7 @@ TEST(swc_parser, input_cleaning)
is << "2 1 14.566132 34.873772 7.857000 0.717830 1\n";
is << "2 1 14.566132 34.873772 7.857000 0.717830 1\n";
- EXPECT_EQ(2u, swc_get_records(is).size());
+ EXPECT_EQ(2u, swc_get_records<swc_io_clean>(is).size());
}
{
@@ -240,7 +265,7 @@ TEST(swc_parser, input_cleaning)
is << "3 1 14.566132 34.873772 7.857000 0.717830 -1\n";
is << "4 1 14.566132 34.873772 7.857000 0.717830 1\n";
- auto records = swc_get_records(is);
+ auto records = swc_get_records<swc_io_clean>(is);
EXPECT_EQ(2u, records.size());
}
@@ -255,7 +280,7 @@ TEST(swc_parser, input_cleaning)
std::array<swc_record::id_type, 4> expected_id_list = {{ 0, 1, 2, 3 }};
auto expected_id = expected_id_list.cbegin();
- for (auto c : swc_get_records(is)) {
+ for (auto c : swc_get_records<swc_io_clean>(is)) {
EXPECT_EQ(*expected_id, c.id());
++expected_id;
}
@@ -281,7 +306,7 @@ TEST(swc_parser, input_cleaning)
auto expected_id = expected_id_list.cbegin();
auto expected_parent = expected_parent_list.cbegin();
- for (auto c : swc_get_records(is)) {
+ for (auto c : swc_get_records<swc_io_clean>(is)) {
EXPECT_EQ(*expected_id, c.id());
EXPECT_EQ(*expected_parent, c.parent());
++expected_id;
diff --git a/vector b/vector
index 7fc001ef..c2fc6318 160000
--- a/vector
+++ b/vector
@@ -1 +1 @@
-Subproject commit 7fc001ef98c01383d384541de08b861328c29405
+Subproject commit c2fc6318f6d65ae9d7aad60309f3b53d590a00ec
--
GitLab