Move inline overrides of public methods out of header files (#1231)

Fix Python linking errors on some platforms.

This appears to be related to the GCC visibility of methods overridden in header files, and is fixed by moving the implementation to the corresponding cpp files. 

Addresses the Travis CI failure of #1225

arbor/CMakeLists.txt

@@ -54,6 +54,7 @@ set(arbor_sources
     spike_event_io.cpp
     spike_source_cell_group.cpp
     s_expr.cpp
+    symmetric_recipe.cpp
     threading/threading.cpp
     thread_private_spike_store.cpp
     tree.cpp

arbor/communication/mpi_error.cpp

@@ -9,4 +9,19 @@ const mpi_error_category_impl& mpi_error_category() {
     return the_category;
 }
 
+const char* mpi_error_category_impl::name() const noexcept { return "MPI"; }
+
+std::string mpi_error_category_impl::message(int ev) const {
+    char err[MPI_MAX_ERROR_STRING];
+    int r;
+    MPI_Error_string(ev, err, &r);
+    return err;
+}
+
+std::error_condition mpi_error_category_impl::default_error_condition(int ev) const noexcept {
+    int eclass;
+    MPI_Error_class(ev, &eclass);
+    return std::error_condition(eclass, mpi_error_category());
 }
+
+} // namespace arb

arbor/cv_policy.cpp

@@ -65,6 +65,7 @@ cv_policy operator|(const cv_policy& lhs, const cv_policy& rhs) {
 
 // Public policy implementations:
 
+// cv_policy_explicit
 locset cv_policy_explicit::cv_boundary_points(const cable_cell& cell) const {
     return
         ls::support(
@@ -76,6 +77,24 @@ locset cv_policy_explicit::cv_boundary_points(const cable_cell& cell) const {
                 components(cell.morphology(), thingify(domain_, cell.provider()))));
 }
 
+cv_policy_base_ptr cv_policy_explicit::clone() const {
+    return cv_policy_base_ptr(new cv_policy_explicit(*this));
+}
+
+region cv_policy_explicit::domain() const { return domain_; }
+
+// cv_policy_single
+locset cv_policy_single::cv_boundary_points(const cable_cell&) const {
+    return ls::cboundary(domain_);
+}
+
+cv_policy_base_ptr cv_policy_single::clone() const {
+    return cv_policy_base_ptr(new cv_policy_single(*this));
+}
+
+region cv_policy_single::domain() const { return domain_; }
+
+// cv_policy_max_extent
 locset cv_policy_max_extent::cv_boundary_points(const cable_cell& cell) const {
     const unsigned nbranch = cell.morphology().num_branches();
     const auto& embed = cell.embedding();
@@ -109,6 +128,13 @@ locset cv_policy_max_extent::cv_boundary_points(const cable_cell& cell) const {
     return unique_sum(locset(std::move(points)), ls::cboundary(domain_));
 }
 
+cv_policy_base_ptr cv_policy_max_extent::clone() const {
+    return cv_policy_base_ptr(new cv_policy_max_extent(*this));
+}
+
+region cv_policy_max_extent::domain() const { return domain_; }
+
+// cv_policy_fixed_per_branch
 locset cv_policy_fixed_per_branch::cv_boundary_points(const cable_cell& cell) const {
     const unsigned nbranch = cell.morphology().num_branches();
     if (!nbranch) return ls::nil();
@@ -139,6 +165,14 @@ locset cv_policy_fixed_per_branch::cv_boundary_points(const cable_cell& cell) co
     return unique_sum(locset(std::move(points)), ls::cboundary(domain_));
 }
 
+cv_policy_base_ptr cv_policy_fixed_per_branch::clone() const {
+    return cv_policy_base_ptr(new cv_policy_fixed_per_branch(*this));
+}
+
+region cv_policy_fixed_per_branch::domain() const { return domain_; }
+
+
+// cv_policy_every_segment
 locset cv_policy_every_segment::cv_boundary_points(const cable_cell& cell) const {
     const unsigned nbranch = cell.morphology().num_branches();
     if (!nbranch) return ls::nil();
@@ -147,5 +181,10 @@ locset cv_policy_every_segment::cv_boundary_points(const cable_cell& cell) const
                 ls::cboundary(domain_),
                 ls::restrict(ls::segment_boundaries(), domain_));
 }
+cv_policy_base_ptr cv_policy_every_segment::clone() const {
+    return cv_policy_base_ptr(new cv_policy_every_segment(*this));
+}
+
+region cv_policy_every_segment::domain() const { return domain_; }
 
 } // namespace arb

arbor/include/arbor/communication/mpi_error.hpp

@@ -82,18 +82,9 @@ class mpi_error_category_impl;
 const mpi_error_category_impl& mpi_error_category();
 
 class mpi_error_category_impl: public std::error_category {
-    const char* name() const noexcept override { return "MPI"; }
-    std::string message(int ev) const override {
-        char err[MPI_MAX_ERROR_STRING];
-        int r;
-        MPI_Error_string(ev, err, &r);
-        return err;
-    }
-    std::error_condition default_error_condition(int ev) const noexcept override {
-        int eclass;
-        MPI_Error_class(ev, &eclass);
-        return std::error_condition(eclass, mpi_error_category());
-    }
+    const char* name() const noexcept override;
+    std::string message(int) const override;
+    std::error_condition default_error_condition(int) const noexcept override;
 };
 
 inline std::error_condition make_error_condition(mpi_errc ec) {

arbor/include/arbor/cv_policy.hpp

@@ -114,12 +114,9 @@ struct cv_policy_explicit: cv_policy_base {
     explicit cv_policy_explicit(locset locs, region domain = reg::all()):
         locs_(std::move(locs)), domain_(std::move(domain)) {}
 
-    cv_policy_base_ptr clone() const override {
-        return cv_policy_base_ptr(new cv_policy_explicit(*this));
-    }
-
+    cv_policy_base_ptr clone() const override;
     locset cv_boundary_points(const cable_cell&) const override;
-    region domain() const override { return domain_; }
+    region domain() const override;
 
 private:
     locset locs_;
@@ -130,14 +127,9 @@ struct cv_policy_single: cv_policy_base {
     explicit cv_policy_single(region domain = reg::all()):
         domain_(domain) {}
 
-    cv_policy_base_ptr clone() const override {
-        return cv_policy_base_ptr(new cv_policy_single(*this));
-    }
-
-    locset cv_boundary_points(const cable_cell&) const override {
-        return ls::cboundary(domain_);
-    }
-    region domain() const override { return domain_; }
+    cv_policy_base_ptr clone() const override;
+    locset cv_boundary_points(const cable_cell&) const override;
+    region domain() const override;
 
 private:
     region domain_;
@@ -150,12 +142,9 @@ struct cv_policy_max_extent: cv_policy_base {
     explicit cv_policy_max_extent(double max_extent, cv_policy_flag::value flags = cv_policy_flag::none):
          max_extent_(max_extent), domain_(reg::all()), flags_(flags) {}
 
-    cv_policy_base_ptr clone() const override {
-        return cv_policy_base_ptr(new cv_policy_max_extent(*this));
-    }
-
+    cv_policy_base_ptr clone() const override;
     locset cv_boundary_points(const cable_cell&) const override;
-    region domain() const override { return domain_; }
+    region domain() const override;
 
 private:
     double max_extent_;
@@ -170,12 +159,9 @@ struct cv_policy_fixed_per_branch: cv_policy_base {
     explicit cv_policy_fixed_per_branch(unsigned cv_per_branch, cv_policy_flag::value flags = cv_policy_flag::none):
          cv_per_branch_(cv_per_branch), domain_(reg::all()), flags_(flags) {}
 
-    cv_policy_base_ptr clone() const override {
-        return cv_policy_base_ptr(new cv_policy_fixed_per_branch(*this));
-    }
-
+    cv_policy_base_ptr clone() const override;
     locset cv_boundary_points(const cable_cell&) const override;
-    region domain() const override { return domain_; }
+    region domain() const override;
 
 private:
     unsigned cv_per_branch_;
@@ -187,12 +173,9 @@ struct cv_policy_every_segment: cv_policy_base {
     explicit cv_policy_every_segment(region domain = reg::all()):
          domain_(std::move(domain)) {}
 
-    cv_policy_base_ptr clone() const override {
-        return cv_policy_base_ptr(new cv_policy_every_segment(*this));
-    }
-
+    cv_policy_base_ptr clone() const override;
     locset cv_boundary_points(const cable_cell&) const override;
-    region domain() const override { return domain_; }
+    region domain() const override;
 
 private:
     region domain_;

arbor/include/arbor/symmetric_recipe.hpp

@@ -1,10 +1,6 @@
 #pragma once
 
 #include <any>
-#include <cstddef>
-#include <memory>
-#include <unordered_map>
-#include <stdexcept>
 
 #include <arbor/recipe.hpp>
 #include <arbor/util/unique_any.hpp>
@@ -29,56 +25,23 @@ class symmetric_recipe: public recipe {
 public:
     symmetric_recipe(std::unique_ptr<tile> rec): tiled_recipe_(std::move(rec)) {}
 
-    cell_size_type num_cells() const override {
-        return tiled_recipe_->num_cells() * tiled_recipe_->num_tiles();
-    }
+    cell_size_type num_cells() const override;
 
-    util::unique_any get_cell_description(cell_gid_type i) const override {
-        return tiled_recipe_->get_cell_description(i % tiled_recipe_->num_cells());
-    }
+    util::unique_any get_cell_description(cell_gid_type i) const override;
 
-    cell_kind get_cell_kind(cell_gid_type i) const override {
-        return tiled_recipe_->get_cell_kind(i % tiled_recipe_->num_cells());
-    }
+    cell_kind get_cell_kind(cell_gid_type i) const override;
 
-    cell_size_type num_sources(cell_gid_type i) const override {
-        return tiled_recipe_->num_sources(i % tiled_recipe_->num_cells());
-    }
+    cell_size_type num_sources(cell_gid_type i) const override;
 
-    cell_size_type num_targets(cell_gid_type i) const override {
-        return tiled_recipe_->num_targets(i % tiled_recipe_->num_cells());
-    }
+    cell_size_type num_targets(cell_gid_type i) const override;
 
-    // Only function that calls the underlying tile's function on the same gid.
-    // This is because applying transformations to event generators is not straightforward.
-    std::vector<event_generator> event_generators(cell_gid_type i) const override {
-        return tiled_recipe_->event_generators(i);
-    }
+    std::vector<event_generator> event_generators(cell_gid_type i) const override;
 
-    // Take connections_on from the original tile recipe for the cell we are duplicating.
-    // Transate the source and destination gids
-    std::vector<cell_connection> connections_on(cell_gid_type i) const override {
-        int n_local = tiled_recipe_->num_cells();
-        int n_global = num_cells();
-        int offset = (i / n_local) * n_local;
+    std::vector<cell_connection> connections_on(cell_gid_type i) const override;
 
-        std::vector<cell_connection> conns = tiled_recipe_->connections_on(i % n_local);
+    std::vector<probe_info> get_probes(cell_gid_type i) const override;
 
-        for (unsigned j = 0; j < conns.size(); j++) {
-            conns[j].source.gid = (conns[j].source.gid + offset) % n_global;
-            conns[j].dest.gid = (conns[j].dest.gid + offset) % n_global;
-        }
-        return conns;
-    }
-
-    std::vector<probe_info> get_probes(cell_gid_type i) const override {
-        i %= tiled_recipe_->num_cells();
-        return tiled_recipe_->get_probes(i);
-    }
-
-    std::any get_global_properties(cell_kind ck) const override {
-        return tiled_recipe_->get_global_properties(ck);
-    };
+    std::any get_global_properties(cell_kind ck) const override;
 
     std::unique_ptr<tile> tiled_recipe_;
 };

arbor/symmetric_recipe.cpp

@@ -0,0 +1,56 @@
+#include <arbor/symmetric_recipe.hpp>
+
+namespace arb {
+
+cell_size_type symmetric_recipe::num_cells() const {
+    return tiled_recipe_->num_cells() * tiled_recipe_->num_tiles();
+}
+
+util::unique_any symmetric_recipe::get_cell_description(cell_gid_type i) const {
+    return tiled_recipe_->get_cell_description(i % tiled_recipe_->num_cells());
+}
+
+cell_kind symmetric_recipe::get_cell_kind(cell_gid_type i) const {
+    return tiled_recipe_->get_cell_kind(i % tiled_recipe_->num_cells());
+}
+
+cell_size_type symmetric_recipe::num_sources(cell_gid_type i) const {
+    return tiled_recipe_->num_sources(i % tiled_recipe_->num_cells());
+}
+
+cell_size_type symmetric_recipe::num_targets(cell_gid_type i) const {
+    return tiled_recipe_->num_targets(i % tiled_recipe_->num_cells());
+}
+
+// Only function that calls the underlying tile's function on the same gid.
+// This is because applying transformations to event generators is not straightforward.
+std::vector<event_generator> symmetric_recipe::event_generators(cell_gid_type i) const {
+    return tiled_recipe_->event_generators(i);
+}
+
+// Take connections_on from the original tile recipe for the cell we are duplicating.
+// Transate the source and destination gids
+std::vector<cell_connection> symmetric_recipe::connections_on(cell_gid_type i) const {
+    int n_local = tiled_recipe_->num_cells();
+    int n_global = num_cells();
+    int offset = (i / n_local) * n_local;
+
+    std::vector<cell_connection> conns = tiled_recipe_->connections_on(i % n_local);
+
+    for (unsigned j = 0; j < conns.size(); j++) {
+        conns[j].source.gid = (conns[j].source.gid + offset) % n_global;
+        conns[j].dest.gid = (conns[j].dest.gid + offset) % n_global;
+    }
+    return conns;
+}
+
+std::vector<probe_info> symmetric_recipe::get_probes(cell_gid_type i) const {
+    i %= tiled_recipe_->num_cells();
+    return tiled_recipe_->get_probes(i);
+}
+
+std::any symmetric_recipe::get_global_properties(cell_kind ck) const {
+    return tiled_recipe_->get_global_properties(ck);
+};
+
+} //namespace arb