From 39b15bd7639f448193b930dcbc8248d98faabf80 Mon Sep 17 00:00:00 2001
From: Ben Cumming <bcumming@cscs.ch>
Date: Wed, 22 Jan 2020 18:56:32 +0100
Subject: [PATCH] C++ library changes for python-cells (#938)

* Make small changes to region/locset interfaces for ease of python interface implemenation.
---
 arbor/include/arbor/morph/locset.hpp   |  7 ++++-
 arbor/include/arbor/morph/region.hpp   |  9 ++++--
 arbor/include/arbor/recipe.hpp         |  4 +--
 arbor/include/arbor/util/typed_map.hpp |  4 +--
 arbor/morph/locset.cpp                 |  6 ++--
 arbor/morph/region.cpp                 |  9 +++---
 arbor/morph/sample_tree.cpp            | 12 ++++----
 arbor/util/sentinel.hpp                |  1 +
 test/unit/test_fvm_layout.cpp          |  1 +
 test/unit/test_morph_expr.cpp          | 38 +++++++++++++-------------
 10 files changed, 52 insertions(+), 39 deletions(-)

diff --git a/arbor/include/arbor/morph/locset.hpp b/arbor/include/arbor/morph/locset.hpp
index 2e39b39f..1ca7e066 100644
--- a/arbor/include/arbor/morph/locset.hpp
+++ b/arbor/include/arbor/morph/locset.hpp
@@ -118,7 +118,7 @@ private:
 
 namespace ls {
 
-// Location of a sample.
+// Explicit location on morphology.
 locset location(mlocation);
 
 // Location of a sample.
@@ -138,4 +138,9 @@ locset nil();
 
 } // namespace ls
 
+// union of two locsets
+locset join(locset, locset);
+// multiset sum of two locsets
+locset sum(locset, locset);
+
 } // namespace arb
diff --git a/arbor/include/arbor/morph/region.hpp b/arbor/include/arbor/morph/region.hpp
index 6f0fd63e..9173bd27 100644
--- a/arbor/include/arbor/morph/region.hpp
+++ b/arbor/include/arbor/morph/region.hpp
@@ -117,9 +117,7 @@ namespace reg {
 region nil();
 
 // An explicit cable section.
-region cable(mcable);
-
-region interval(mlocation, mlocation);
+region cable(msize_t, double, double);
 
 // An explicit branch.
 region branch(msize_t);
@@ -135,4 +133,9 @@ region named(std::string);
 
 } // namespace reg
 
+// union of two regions
+region join(region, region);
+// intersection of two regions
+region intersect(region, region);
+
 } // namespace arb
diff --git a/arbor/include/arbor/recipe.hpp b/arbor/include/arbor/recipe.hpp
index f5910c90..fea78935 100644
--- a/arbor/include/arbor/recipe.hpp
+++ b/arbor/include/arbor/recipe.hpp
@@ -21,8 +21,8 @@ struct probe_info {
 };
 
 /* Recipe descriptions are cell-oriented: in order that the building
- * phase can be done distributedly and in order that the recipe
- * description can be built indepdently of any runtime execution environment.
+ * phase can be distributed, and in order that the recipe description
+ * can be built indepedently of any runtime execution environment.
  */
 
 // Note: `cell_connection` and `connection` have essentially the same data
diff --git a/arbor/include/arbor/util/typed_map.hpp b/arbor/include/arbor/util/typed_map.hpp
index 785602e1..22b59433 100644
--- a/arbor/include/arbor/util/typed_map.hpp
+++ b/arbor/include/arbor/util/typed_map.hpp
@@ -21,7 +21,7 @@
 
 namespace arb {
 
-template <template <class> typename E>
+template <template <class> class E>
 struct dynamic_typed_map {
     // Retrieve value by reference associated with type T; create entry with
     // default value if no entry in map for T.
@@ -50,7 +50,7 @@ private:
     std::unordered_map<std::type_index, arb::util::any> tmap_;
 };
 
-template <template <class> typename E, typename... Keys>
+template <template <class> class E, typename... Keys>
 struct static_typed_map {
     // Retrieve value by reference associated with type T.
     template <typename T>
diff --git a/arbor/morph/locset.cpp b/arbor/morph/locset.cpp
index dd81e56e..3fa6ea89 100644
--- a/arbor/morph/locset.cpp
+++ b/arbor/morph/locset.cpp
@@ -96,7 +96,7 @@ mlocation_list thingify_(const terminal_&, const mprovider& p) {
 }
 
 std::ostream& operator<<(std::ostream& o, const terminal_& x) {
-    return o << "terminal";
+    return o << "(terminal)";
 }
 
 // Root location (most proximal point).
@@ -112,7 +112,7 @@ mlocation_list thingify_(const root_&, const mprovider& p) {
 }
 
 std::ostream& operator<<(std::ostream& o, const root_& x) {
-    return o << "root";
+    return o << "(root)";
 }
 
 // Named locset.
@@ -130,7 +130,7 @@ mlocation_list thingify_(const named_& n, const mprovider& p) {
 }
 
 std::ostream& operator<<(std::ostream& o, const named_& x) {
-    return o << "(named \"" << x.name << "\")";
+    return o << "(locset \"" << x.name << "\")";
 }
 
 
diff --git a/arbor/morph/region.cpp b/arbor/morph/region.cpp
index abe93427..cb68726f 100644
--- a/arbor/morph/region.cpp
+++ b/arbor/morph/region.cpp
@@ -194,7 +194,8 @@ struct cable_ {
     mcable cable;
 };
 
-region cable(mcable c) {
+region cable(msize_t id, double prox, double dist) {
+    mcable c{id, prox, dist};
     if (!test_invariants(c)) {
         throw invalid_mcable(c);
     }
@@ -202,7 +203,7 @@ region cable(mcable c) {
 }
 
 region branch(msize_t bid) {
-    return cable({bid, 0, 1});
+    return cable(bid, 0, 1);
 }
 
 mcable_list thingify_(const cable_& reg, const mprovider& p) {
@@ -300,7 +301,7 @@ mcable_list thingify_(const all_&, const mprovider& p) {
 }
 
 std::ostream& operator<<(std::ostream& o, const all_& t) {
-    return o << "all";
+    return o << "(all)";
 }
 
 
@@ -319,7 +320,7 @@ mcable_list thingify_(const named_& n, const mprovider& p) {
 }
 
 std::ostream& operator<<(std::ostream& o, const named_& x) {
-    return o << "(named \"" << x.name << "\")";
+    return o << "(region \"" << x.name << "\")";
 }
 
 
diff --git a/arbor/morph/sample_tree.cpp b/arbor/morph/sample_tree.cpp
index 15ec4d8c..4ba6d256 100644
--- a/arbor/morph/sample_tree.cpp
+++ b/arbor/morph/sample_tree.cpp
@@ -7,6 +7,7 @@
 
 #include "io/sepval.hpp"
 #include "util/span.hpp"
+#include "util/transform.hpp"
 
 namespace arb {
 
@@ -102,11 +103,12 @@ const std::vector<point_prop>& sample_tree::properties() const {
 }
 
 std::ostream& operator<<(std::ostream& o, const sample_tree& m) {
-    o << "sample_tree:"
-      << "\n  " << m.size() << " samples"
-      << "\n  samples [" << io::csv(m.samples_) <<  "]"
-      << "\n  parents [" << io::csv(m.parents_) <<  "]";
-    return o;
+    auto tstr = util::transform_view(m.parents_,
+            [](msize_t i) -> std::string {
+                return i==mnpos? "npos": std::to_string(i);
+            });
+    return o << "(sample_tree (\n  " << io::sepval(m.samples_, "\n  ") <<  ")\n"
+             << "  (" << io::sepval(tstr, ' ') <<  "))";
 }
 
 sample_tree swc_as_sample_tree(const std::vector<swc_record>& swc_records) {
diff --git a/arbor/util/sentinel.hpp b/arbor/util/sentinel.hpp
index 7a47227e..9b1bf1a2 100644
--- a/arbor/util/sentinel.hpp
+++ b/arbor/util/sentinel.hpp
@@ -3,6 +3,7 @@
 #include <type_traits>
 
 #include <arbor/util/either.hpp>
+
 #include "util/meta.hpp"
 
 /*
diff --git a/test/unit/test_fvm_layout.cpp b/test/unit/test_fvm_layout.cpp
index b27a7b6d..a17d2bb1 100644
--- a/test/unit/test_fvm_layout.cpp
+++ b/test/unit/test_fvm_layout.cpp
@@ -112,6 +112,7 @@ namespace {
         cell.default_parameters.axial_resistivity = 90;
 
         cells.push_back(std::move(cell));
+
         return cells;
     }
 
diff --git a/test/unit/test_morph_expr.cpp b/test/unit/test_morph_expr.cpp
index bee279ab..dee25298 100644
--- a/test/unit/test_morph_expr.cpp
+++ b/test/unit/test_morph_expr.cpp
@@ -57,9 +57,9 @@ namespace arb {
 TEST(region, expr_repn) {
     using util::to_string;
 
-    auto c1 = reg::cable({1, 0, 1});
-    auto c2 = reg::cable({4, 0.125, 0.5});
-    auto c3 = join(reg::cable({4, 0.125, 0.5}), reg::cable({3, 0, 1}));
+    auto c1 = reg::cable(1, 0, 1);
+    auto c2 = reg::cable(4, 0.125, 0.5);
+    auto c3 = join(reg::cable(4, 0.125, 0.5), reg::cable(3, 0, 1));
     auto b1 = reg::branch(1);
     auto t1 = reg::tagged(1);
     auto t2 = reg::tagged(2);
@@ -77,12 +77,12 @@ TEST(region, expr_repn) {
     EXPECT_EQ(to_string(join(t1, t2, t3)), "(join (join (tag 1) (tag 2)) (tag 3))");
     EXPECT_EQ(to_string(intersect(t1, t2, t3)), "(intersect (intersect (tag 1) (tag 2)) (tag 3))");
     EXPECT_EQ(to_string(intersect(join(c1, t2), c2)),  "(intersect (join (cable 1 0 1) (tag 2)) (cable 4 0.125 0.5))");
-    EXPECT_EQ(to_string(all), "all");
+    EXPECT_EQ(to_string(all), "(all)");
 }
 
 TEST(region, invalid_mcable) {
-    EXPECT_NO_THROW(reg::cable({123, 0.5, 0.8}));
-    EXPECT_THROW(reg::cable({1, 0, 1.1}), invalid_mcable);
+    EXPECT_NO_THROW(reg::cable(123, 0.5, 0.8));
+    EXPECT_THROW(reg::cable(1, 0, 1.1), invalid_mcable);
     EXPECT_THROW(reg::branch(-1), invalid_mcable);
 }
 
@@ -94,11 +94,11 @@ TEST(locset, expr_repn) {
     auto samp = ls::sample(42);
     auto loc = ls::location({2, 0.5});
 
-    EXPECT_EQ(to_string(root), "root");
-    EXPECT_EQ(to_string(term), "terminal");
-    EXPECT_EQ(to_string(sum(root, term)), "(sum root terminal)");
-    EXPECT_EQ(to_string(sum(root, term, samp)), "(sum (sum root terminal) (sample 42))");
-    EXPECT_EQ(to_string(sum(root, term, samp, loc)), "(sum (sum (sum root terminal) (sample 42)) (location 2 0.5))");
+    EXPECT_EQ(to_string(root), "(root)");
+    EXPECT_EQ(to_string(term), "(terminal)");
+    EXPECT_EQ(to_string(sum(root, term)), "(sum (root) (terminal))");
+    EXPECT_EQ(to_string(sum(root, term, samp)), "(sum (sum (root) (terminal)) (sample 42))");
+    EXPECT_EQ(to_string(sum(root, term, samp, loc)), "(sum (sum (sum (root) (terminal)) (sample 42)) (location 2 0.5))");
     EXPECT_EQ(to_string(samp), "(sample 42)");
     EXPECT_EQ(to_string(loc), "(location 2 0.5)");
 }
@@ -159,7 +159,7 @@ TEST(region, thingify_named) {
     using svec = std::vector<msample>;
 
     region banana = reg::branch(0);
-    region cake = reg::cable(mcable{0, 0.2, 0.3});
+    region cake = reg::cable(0, 0.2, 0.3);
 
     // copy-paste ftw
 
@@ -284,8 +284,8 @@ TEST(region, thingify) {
         sample_tree sm(samples, parents);
         mprovider mp(morphology(sm, false));
 
-        auto h1  = reg::cable({0, 0, 0.5});
-        auto h2  = reg::cable({0, 0.5, 1});
+        auto h1  = reg::cable(0, 0, 0.5);
+        auto h2  = reg::cable(0, 0.5, 1);
         auto t1  = reg::tagged(1);
         auto t2  = reg::tagged(2);
         auto all = reg::all();
@@ -434,7 +434,7 @@ TEST(region, thingify) {
         //   |  xxxxx  |   xxx   | rand
         //   |xxxxxxxxx|xxxxxxxxx| ror
         auto lhs  = b13;
-        auto rhs  = join(cable({1,.2,.7}), cable({3,.3,.6}));
+        auto rhs  = join(cable(1,.2,.7), cable(3,.3,.6));
         auto rand = cl{         {1,.2,.7}, {3,.3,.6}};
         auto ror  = cl{         {1,.0,1.}, {3,.0,1.}};
         EXPECT_EQ(thingify(intersect(lhs, rhs), mp), rand);
@@ -450,8 +450,8 @@ TEST(region, thingify) {
         //   |  -----  |   ---   | rhs
         //   |   xxxx  |   xx    | rand
         //   |  xxxxxx | xxxxx   | ror
-        lhs  = join(cable({1,.3,.8}), cable({3,.1,.5}));
-        rhs  = join(cable({1,.2,.7}), cable({3,.3,.6}));
+        lhs  = join(cable(1,.3,.8), cable(3,.1,.5));
+        rhs  = join(cable(1,.2,.7), cable(3,.3,.6));
         rand = cl{         {1,.3,.7}, {3,.3,.5}};
         ror  = cl{         {1,.2,.8}, {3,.1,.6}};
         EXPECT_EQ(thingify(intersect(lhs, rhs), mp), rand);
@@ -467,8 +467,8 @@ TEST(region, thingify) {
         //   |  -----  |   ---   | rhs
         //   |  x x    |   x x   | rand
         //   | xxxxxx  | xxxxxxx | ror
-        lhs  = join(cable({1,.1,.3}), cable({1,.4,.5}), cable({3,.1,.4}), cable({3,.5,.9}));
-        rhs  = join(cable({1,.2,.7}), cable({3,.3,.6}));
+        lhs  = join(cable(1,.1,.3), cable(1,.4,.5), cable(3,.1,.4), cable(3,.5,.9));
+        rhs  = join(cable(1,.2,.7), cable(3,.3,.6));
         rand = cl{         {1,.2,.3}, {1,.4,.5}, {3,.3,.4}, {3,.5,.6}};
         ror  = cl{         {1,.1,.7},            {3,.1,.9}};
         EXPECT_EQ(thingify(intersect(lhs, rhs), mp), rand);
-- 
GitLab