From 0d1f53fafcde4c748c2fa38d49057bfe456c4167 Mon Sep 17 00:00:00 2001 From: Nora Abi Akar <nora.abiakar@gmail.com> Date: Fri, 13 Nov 2020 09:18:09 +0100 Subject: [PATCH] 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 | 1 + arbor/communication/mpi_error.cpp | 15 +++++ arbor/cv_policy.cpp | 39 +++++++++++++ .../include/arbor/communication/mpi_error.hpp | 15 +---- arbor/include/arbor/cv_policy.hpp | 39 ++++--------- arbor/include/arbor/symmetric_recipe.hpp | 55 +++--------------- arbor/symmetric_recipe.cpp | 56 +++++++++++++++++++ 7 files changed, 134 insertions(+), 86 deletions(-) create mode 100644 arbor/symmetric_recipe.cpp diff --git a/arbor/CMakeLists.txt b/arbor/CMakeLists.txt index 5f135d92..3d3f25f1 100644 --- a/arbor/CMakeLists.txt +++ b/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 diff --git a/arbor/communication/mpi_error.cpp b/arbor/communication/mpi_error.cpp index 4f0b62c5..de511c21 100644 --- a/arbor/communication/mpi_error.cpp +++ b/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 diff --git a/arbor/cv_policy.cpp b/arbor/cv_policy.cpp index 468d6a70..65deddce 100644 --- a/arbor/cv_policy.cpp +++ b/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 diff --git a/arbor/include/arbor/communication/mpi_error.hpp b/arbor/include/arbor/communication/mpi_error.hpp index fcbfc50b..911447a1 100644 --- a/arbor/include/arbor/communication/mpi_error.hpp +++ b/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) { diff --git a/arbor/include/arbor/cv_policy.hpp b/arbor/include/arbor/cv_policy.hpp index b8869f02..4ddbdfcb 100644 --- a/arbor/include/arbor/cv_policy.hpp +++ b/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_; diff --git a/arbor/include/arbor/symmetric_recipe.hpp b/arbor/include/arbor/symmetric_recipe.hpp index 7addcf6f..81c8336e 100644 --- a/arbor/include/arbor/symmetric_recipe.hpp +++ b/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_; }; diff --git a/arbor/symmetric_recipe.cpp b/arbor/symmetric_recipe.cpp new file mode 100644 index 00000000..66b846f8 --- /dev/null +++ b/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 \ No newline at end of file -- GitLab