From 1455706166efc071205f7470eba781d323055e0b Mon Sep 17 00:00:00 2001 From: abiakarn Date: Fri, 20 Aug 2021 16:34:15 +0200 Subject: [PATCH 1/4] check mechanism kind --- arbor/fvm_layout.cpp | 18 +++++++--- arbor/include/arbor/mechinfo.hpp | 3 ++ arbor/mechinfo.cpp | 1 + test/unit/test_fvm_layout.cpp | 59 ++++++++++++++++++++++++++++++++ 4 files changed, 77 insertions(+), 4 deletions(-) diff --git a/arbor/fvm_layout.cpp b/arbor/fvm_layout.cpp index 3261726fec..8153369fe4 100644 --- a/arbor/fvm_layout.cpp +++ b/arbor/fvm_layout.cpp @@ -858,14 +858,17 @@ fvm_mechanism_data fvm_build_mechanism_data(const cable_cell_global_properties& const std::string& name = entry.first; mechanism_info info = catalogue[name]; - std::vector param_dflt; fvm_mechanism_config config; - config.kind = arb_mechanism_kind_density; + if (info.kind != arb_mechanism_kind_density) { + throw cable_cell_error("Expected density mechanism, got "+ name); + } + config.kind = info.kind; std::vector param_names; assign(param_names, util::keys(info.parameters)); sort(param_names); + std::vector param_dflt; std::size_t n_param = param_names.size(); param_dflt.reserve(n_param); config.param_values.reserve(n_param); @@ -963,6 +966,10 @@ fvm_mechanism_data fvm_build_mechanism_data(const cable_cell_global_properties& const std::string& name = entry.first; mechanism_info info = catalogue[name]; + if (info.kind != arb_mechanism_kind_point) { + throw cable_cell_error("Expected point mechanism, got "+ name); + } + post_events |= info.post_events; std::size_t n_param = info.parameters.size(); std::size_t n_inst = entry.second.size(); @@ -1044,7 +1051,7 @@ fvm_mechanism_data fvm_build_mechanism_data(const cable_cell_global_properties& bool coalesce = catalogue[name].linear && gprop.coalesce_synapses; fvm_mechanism_config config; - config.kind = arb_mechanism_kind_point; + config.kind = info.kind; for (auto& kv: info.parameters) { config.param_values.emplace_back(kv.first, std::vector{}); if (!coalesce) { @@ -1248,7 +1255,10 @@ fvm_mechanism_data fvm_build_mechanism_data(const cable_cell_global_properties& } else { fvm_mechanism_config config; - config.kind = arb_mechanism_kind_reversal_potential; + if (info.kind != arb_mechanism_kind_reversal_potential) { + throw cable_cell_error("Expected reversal potential mechanism for ion " + ion + ", got "+ revpot.name()); + } + config.kind = info.kind; config.cv = M.ions[ion].cv; config.norm_area.assign(config.cv.size(), 1.); diff --git a/arbor/include/arbor/mechinfo.hpp b/arbor/include/arbor/mechinfo.hpp index 3e4ff24b81..904fa90d8c 100644 --- a/arbor/include/arbor/mechinfo.hpp +++ b/arbor/include/arbor/mechinfo.hpp @@ -58,6 +58,9 @@ struct mechanism_info { mechanism_info(const arb_mechanism_type&); mechanism_info() = default; + // Mechanism kind + arb_mechanism_kind kind; + // Global fields have one value common to an instance of a mechanism, are // constant in time and set at instantiation. std::unordered_map globals; diff --git a/arbor/mechinfo.cpp b/arbor/mechinfo.cpp index abfe6c978e..341568a78c 100644 --- a/arbor/mechinfo.cpp +++ b/arbor/mechinfo.cpp @@ -4,6 +4,7 @@ namespace arb { mechanism_info::mechanism_info(const arb_mechanism_type& m) { + kind = m.kind; post_events = m.has_post_events; linear = m.is_linear; fingerprint = m.fingerprint; diff --git a/test/unit/test_fvm_layout.cpp b/test/unit/test_fvm_layout.cpp index 6d4e3a5a98..8211f87792 100644 --- a/test/unit/test_fvm_layout.cpp +++ b/test/unit/test_fvm_layout.cpp @@ -129,6 +129,65 @@ namespace { } } // namespace +TEST(fvm_layout, compatible_mechanisms) { + cable_cell_global_properties gprop; + gprop.default_parameters = neuron_parameter_defaults; + + { + auto system = two_cell_system(); + + // place a density mechanism instead of a point mechanism + system.descriptions[0].decorations.place(system.builders[0].location({1, 0.4}), "hh", "syn0"); + + auto cells = system.cells(); + check_two_cell_system(cells); + fvm_cv_discretization D = fvm_cv_discretize(cells, gprop.default_parameters); + + EXPECT_THROW(fvm_build_mechanism_data(gprop, cells, D), arb::cable_cell_error); + } + { + auto system = two_cell_system(); + + // paint a point mechanism instead of a density mechanism + system.descriptions[1].decorations.paint(system.builders[1].cable(mcable{0}), "expsyn"); + + auto cells = system.cells(); + check_two_cell_system(cells); + fvm_cv_discretization D = fvm_cv_discretize(cells, gprop.default_parameters); + + EXPECT_THROW(fvm_build_mechanism_data(gprop, cells, D), arb::cable_cell_error); + } + { + auto system = two_cell_system(); + + // Set a density mechanism instead of a reversal potential mechanism + gprop.default_parameters.reversal_potential_method["na"] = "pas"; + + // Paint a mechanism that uses "na" + system.descriptions[1].decorations.paint(system.builders[1].cable(mcable{0}), "hh"); + + auto cells = system.cells(); + check_two_cell_system(cells); + fvm_cv_discretization D = fvm_cv_discretize(cells, gprop.default_parameters); + + EXPECT_THROW(fvm_build_mechanism_data(gprop, cells, D), arb::cable_cell_error); + } + { + auto system = two_cell_system(); + + // Set a density mechanism instead of a reversal potential mechanism + // Paint a mechanism that uses "na" + system.descriptions[1].decorations.set_default(ion_reversal_potential_method{"na", "nernst/na"}); + system.descriptions[1].decorations.paint(system.builders[1].cable(mcable{0}), "hh"); + + auto cells = system.cells(); + check_two_cell_system(cells); + fvm_cv_discretization D = fvm_cv_discretize(cells, gprop.default_parameters); + + EXPECT_THROW(fvm_build_mechanism_data(gprop, cells, D), arb::cable_cell_error); + } +} + TEST(fvm_layout, mech_index) { auto system = two_cell_system(); auto& descriptions = system.descriptions; From 304908465b6f2db10533265cf18c92908ec574a5 Mon Sep 17 00:00:00 2001 From: abiakarn Date: Fri, 20 Aug 2021 16:54:51 +0200 Subject: [PATCH 2/4] fix unit test --- arbor/fvm_layout.cpp | 7 ++++--- test/unit/test_fvm_layout.cpp | 9 ++------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/arbor/fvm_layout.cpp b/arbor/fvm_layout.cpp index 8153369fe4..ea277b4ebf 100644 --- a/arbor/fvm_layout.cpp +++ b/arbor/fvm_layout.cpp @@ -1217,6 +1217,10 @@ fvm_mechanism_data fvm_build_mechanism_data(const cable_cell_global_properties& { const mechanism_desc& revpot = *maybe_revpot; mechanism_info info = catalogue[revpot.name()]; + if (info.kind != arb_mechanism_kind_reversal_potential) { + throw cable_cell_error("Expected reversal potential mechanism for ion " + ion + ", got "+ revpot.name()); + } + verify_mechanism(info, revpot); revpot_specified.insert(ion); @@ -1255,9 +1259,6 @@ fvm_mechanism_data fvm_build_mechanism_data(const cable_cell_global_properties& } else { fvm_mechanism_config config; - if (info.kind != arb_mechanism_kind_reversal_potential) { - throw cable_cell_error("Expected reversal potential mechanism for ion " + ion + ", got "+ revpot.name()); - } config.kind = info.kind; config.cv = M.ions[ion].cv; config.norm_area.assign(config.cv.size(), 1.); diff --git a/test/unit/test_fvm_layout.cpp b/test/unit/test_fvm_layout.cpp index 8211f87792..f29d5ae14d 100644 --- a/test/unit/test_fvm_layout.cpp +++ b/test/unit/test_fvm_layout.cpp @@ -161,10 +161,7 @@ TEST(fvm_layout, compatible_mechanisms) { auto system = two_cell_system(); // Set a density mechanism instead of a reversal potential mechanism - gprop.default_parameters.reversal_potential_method["na"] = "pas"; - - // Paint a mechanism that uses "na" - system.descriptions[1].decorations.paint(system.builders[1].cable(mcable{0}), "hh"); + system.descriptions[1].decorations.set_default(ion_reversal_potential_method{"na", "expsyn"}); auto cells = system.cells(); check_two_cell_system(cells); @@ -176,9 +173,7 @@ TEST(fvm_layout, compatible_mechanisms) { auto system = two_cell_system(); // Set a density mechanism instead of a reversal potential mechanism - // Paint a mechanism that uses "na" - system.descriptions[1].decorations.set_default(ion_reversal_potential_method{"na", "nernst/na"}); - system.descriptions[1].decorations.paint(system.builders[1].cable(mcable{0}), "hh"); + gprop.default_parameters.reversal_potential_method["na"] = "pas"; auto cells = system.cells(); check_two_cell_system(cells); From c38c67182868ed5a406c4267515cb539cc175760 Mon Sep 17 00:00:00 2001 From: abiakarn Date: Mon, 23 Aug 2021 10:22:00 +0200 Subject: [PATCH 3/4] address PR comments --- arbor/fvm_layout.cpp | 16 +++++++++++++--- doc/python/mechanisms.rst | 10 ++++++++++ python/mechanism.cpp | 11 +++++++++++ 3 files changed, 34 insertions(+), 3 deletions(-) diff --git a/arbor/fvm_layout.cpp b/arbor/fvm_layout.cpp index ea277b4ebf..871bb6679e 100644 --- a/arbor/fvm_layout.cpp +++ b/arbor/fvm_layout.cpp @@ -838,6 +838,16 @@ fvm_mechanism_data fvm_build_mechanism_data(const cable_cell_global_properties& } }; + // Mechanism kind as string, for error messages. + auto mech_kind_to_string = [](arb_mechanism_kind t) { + switch (t) { + case arb_mechanism_kind_density: return "density mechanism kind"; + case arb_mechanism_kind_point: return "point mechanism kind"; + case arb_mechanism_kind_reversal_potential: return "reversal potential mechanism kind"; + default: return "unknown mechanism kind"; + } + }; + // Track ion usage of mechanisms so that ions are only instantiated where required. std::unordered_map> ion_support; auto update_ion_support = [&ion_support](const mechanism_info& info, const std::vector& cvs) { @@ -860,7 +870,7 @@ fvm_mechanism_data fvm_build_mechanism_data(const cable_cell_global_properties& fvm_mechanism_config config; if (info.kind != arb_mechanism_kind_density) { - throw cable_cell_error("Expected density mechanism, got "+ name); + throw cable_cell_error("expected density mechanism, got " +name +" which has " +mech_kind_to_string(info.kind)); } config.kind = info.kind; @@ -967,7 +977,7 @@ fvm_mechanism_data fvm_build_mechanism_data(const cable_cell_global_properties& mechanism_info info = catalogue[name]; if (info.kind != arb_mechanism_kind_point) { - throw cable_cell_error("Expected point mechanism, got "+ name); + throw cable_cell_error("expected point mechanism, got " +name +" which has " +mech_kind_to_string(info.kind)); } post_events |= info.post_events; @@ -1218,7 +1228,7 @@ fvm_mechanism_data fvm_build_mechanism_data(const cable_cell_global_properties& const mechanism_desc& revpot = *maybe_revpot; mechanism_info info = catalogue[revpot.name()]; if (info.kind != arb_mechanism_kind_reversal_potential) { - throw cable_cell_error("Expected reversal potential mechanism for ion " + ion + ", got "+ revpot.name()); + throw cable_cell_error("expected reversal potential mechanism for ion " +ion +", got "+ revpot.name() +" which has " +mech_kind_to_string(info.kind)); } verify_mechanism(info, revpot); diff --git a/doc/python/mechanisms.rst b/doc/python/mechanisms.rst index 54c6a32efa..cb19925fcc 100644 --- a/doc/python/mechanisms.rst +++ b/doc/python/mechanisms.rst @@ -138,6 +138,11 @@ mechanism that is to be painted or placed on the cable cell. print(mech.parameters['tau'].default) # 2.0 + .. py:attribute:: kind + :type: string + + String representation of the kind of the mechanism: density, point or reversal potential. + .. py:attribute:: globals :type: dict[str, mechanism_field] @@ -163,6 +168,11 @@ mechanism that is to be painted or placed on the cable cell. True if a synapse mechanism has linear current contributions so that multiple instances on the same :term:`control volume` can be coalesced. + .. py:attribute:: post_events + :type: bool + + True if a synapse mechanism has a `POST_EVENT` procedure defined. + .. py:class:: ion_dependency diff --git a/python/mechanism.cpp b/python/mechanism.cpp index 2a27eccbcd..d1867d194d 100644 --- a/python/mechanism.cpp +++ b/python/mechanism.cpp @@ -96,6 +96,17 @@ void register_mechanisms(pybind11::module& m) { "Ion dependencies.") .def_readonly("linear", &arb::mechanism_info::linear, "True if a synapse mechanism has linear current contributions so that multiple instances on the same compartment can be coalesced.") + .def_readonly("post_events", &arb::mechanism_info::post_events, + "True if a synapse mechanism has a `POST_EVENT` procedure defined.") + .def_property_readonly("kind", + [](const arb::mechanism_info& info) { + switch (info.kind) { + case arb_mechanism_kind_density: return "Density mechanism"; + case arb_mechanism_kind_point: return "Point mechanism"; + case arb_mechanism_kind_reversal_potential: return "Reversal potential mechanism"; + default: return "Unknown mechanism kind"; + }; + }, "String representation of the kind of the mechanism.") .def("__repr__", [](const arb::mechanism_info& inf) { return util::pprintf("(arbor.mechanism_info)"); }) From 39950dbf1fa1917240fc3831ec13c2f7c4f18d83 Mon Sep 17 00:00:00 2001 From: abiakarn Date: Mon, 23 Aug 2021 17:17:06 +0200 Subject: [PATCH 4/4] address PR comments --- arbor/fvm_layout.cpp | 16 ++------ arbor/include/arbor/mechanism_abi.h | 9 +++++ python/mechanism.cpp | 7 +--- test/unit/test_fvm_layout.cpp | 63 ++++++++--------------------- 4 files changed, 30 insertions(+), 65 deletions(-) diff --git a/arbor/fvm_layout.cpp b/arbor/fvm_layout.cpp index 871bb6679e..d2ee99ce30 100644 --- a/arbor/fvm_layout.cpp +++ b/arbor/fvm_layout.cpp @@ -838,16 +838,6 @@ fvm_mechanism_data fvm_build_mechanism_data(const cable_cell_global_properties& } }; - // Mechanism kind as string, for error messages. - auto mech_kind_to_string = [](arb_mechanism_kind t) { - switch (t) { - case arb_mechanism_kind_density: return "density mechanism kind"; - case arb_mechanism_kind_point: return "point mechanism kind"; - case arb_mechanism_kind_reversal_potential: return "reversal potential mechanism kind"; - default: return "unknown mechanism kind"; - } - }; - // Track ion usage of mechanisms so that ions are only instantiated where required. std::unordered_map> ion_support; auto update_ion_support = [&ion_support](const mechanism_info& info, const std::vector& cvs) { @@ -870,7 +860,7 @@ fvm_mechanism_data fvm_build_mechanism_data(const cable_cell_global_properties& fvm_mechanism_config config; if (info.kind != arb_mechanism_kind_density) { - throw cable_cell_error("expected density mechanism, got " +name +" which has " +mech_kind_to_string(info.kind)); + throw cable_cell_error("expected density mechanism, got " +name +" which has " +arb_mechsnism_kind_str(info.kind)); } config.kind = info.kind; @@ -977,7 +967,7 @@ fvm_mechanism_data fvm_build_mechanism_data(const cable_cell_global_properties& mechanism_info info = catalogue[name]; if (info.kind != arb_mechanism_kind_point) { - throw cable_cell_error("expected point mechanism, got " +name +" which has " +mech_kind_to_string(info.kind)); + throw cable_cell_error("expected point mechanism, got " +name +" which has " +arb_mechsnism_kind_str(info.kind)); } post_events |= info.post_events; @@ -1228,7 +1218,7 @@ fvm_mechanism_data fvm_build_mechanism_data(const cable_cell_global_properties& const mechanism_desc& revpot = *maybe_revpot; mechanism_info info = catalogue[revpot.name()]; if (info.kind != arb_mechanism_kind_reversal_potential) { - throw cable_cell_error("expected reversal potential mechanism for ion " +ion +", got "+ revpot.name() +" which has " +mech_kind_to_string(info.kind)); + throw cable_cell_error("expected reversal potential mechanism for ion " +ion +", got "+ revpot.name() +" which has " +arb_mechsnism_kind_str(info.kind)); } verify_mechanism(info, revpot); diff --git a/arbor/include/arbor/mechanism_abi.h b/arbor/include/arbor/mechanism_abi.h index ce85067fbb..83770e2e4b 100644 --- a/arbor/include/arbor/mechanism_abi.h +++ b/arbor/include/arbor/mechanism_abi.h @@ -27,6 +27,15 @@ typedef uint32_t arb_backend_kind; #define arb_backend_kind_cpu 1 #define arb_backend_kind_gpu 2 +inline const char* arb_mechsnism_kind_str(const arb_mechanism_kind& mech) { + switch (mech) { + case arb_mechanism_kind_density: return "density mechanism kind"; + case arb_mechanism_kind_point: return "point mechanism kind"; + case arb_mechanism_kind_reversal_potential: return "reversal potential mechanism kind"; + default: return "unknown mechanism kind"; + } +} + // Ion state variables; view into shared_state typedef struct arb_ion_state { arb_value_type* current_density; diff --git a/python/mechanism.cpp b/python/mechanism.cpp index d1867d194d..16f9f25e83 100644 --- a/python/mechanism.cpp +++ b/python/mechanism.cpp @@ -100,12 +100,7 @@ void register_mechanisms(pybind11::module& m) { "True if a synapse mechanism has a `POST_EVENT` procedure defined.") .def_property_readonly("kind", [](const arb::mechanism_info& info) { - switch (info.kind) { - case arb_mechanism_kind_density: return "Density mechanism"; - case arb_mechanism_kind_point: return "Point mechanism"; - case arb_mechanism_kind_reversal_potential: return "Reversal potential mechanism"; - default: return "Unknown mechanism kind"; - }; + return arb_mechsnism_kind_str(info.kind); }, "String representation of the kind of the mechanism.") .def("__repr__", [](const arb::mechanism_info& inf) { diff --git a/test/unit/test_fvm_layout.cpp b/test/unit/test_fvm_layout.cpp index f29d5ae14d..981b291a06 100644 --- a/test/unit/test_fvm_layout.cpp +++ b/test/unit/test_fvm_layout.cpp @@ -129,58 +129,29 @@ namespace { } } // namespace -TEST(fvm_layout, compatible_mechanisms) { - cable_cell_global_properties gprop; - gprop.default_parameters = neuron_parameter_defaults; - - { - auto system = two_cell_system(); - - // place a density mechanism instead of a point mechanism - system.descriptions[0].decorations.place(system.builders[0].location({1, 0.4}), "hh", "syn0"); - - auto cells = system.cells(); - check_two_cell_system(cells); - fvm_cv_discretization D = fvm_cv_discretize(cells, gprop.default_parameters); - - EXPECT_THROW(fvm_build_mechanism_data(gprop, cells, D), arb::cable_cell_error); - } - { - auto system = two_cell_system(); - - // paint a point mechanism instead of a density mechanism - system.descriptions[1].decorations.paint(system.builders[1].cable(mcable{0}), "expsyn"); - - auto cells = system.cells(); - check_two_cell_system(cells); - fvm_cv_discretization D = fvm_cv_discretize(cells, gprop.default_parameters); - - EXPECT_THROW(fvm_build_mechanism_data(gprop, cells, D), arb::cable_cell_error); - } - { - auto system = two_cell_system(); +template +void check_compatible_mechanism_failure(cable_cell_global_properties gprop, P painter) { + auto system = two_cell_system(); - // Set a density mechanism instead of a reversal potential mechanism - system.descriptions[1].decorations.set_default(ion_reversal_potential_method{"na", "expsyn"}); + painter(system); - auto cells = system.cells(); - check_two_cell_system(cells); - fvm_cv_discretization D = fvm_cv_discretize(cells, gprop.default_parameters); + auto cells = system.cells(); + check_two_cell_system(cells); + fvm_cv_discretization D = fvm_cv_discretize(cells, gprop.default_parameters); - EXPECT_THROW(fvm_build_mechanism_data(gprop, cells, D), arb::cable_cell_error); - } - { - auto system = two_cell_system(); + EXPECT_THROW(fvm_build_mechanism_data(gprop, cells, D), arb::cable_cell_error); +} - // Set a density mechanism instead of a reversal potential mechanism - gprop.default_parameters.reversal_potential_method["na"] = "pas"; +TEST(fvm_layout, compatible_mechanisms) { + cable_cell_global_properties gprop; + gprop.default_parameters = neuron_parameter_defaults; - auto cells = system.cells(); - check_two_cell_system(cells); - fvm_cv_discretization D = fvm_cv_discretize(cells, gprop.default_parameters); + check_compatible_mechanism_failure(gprop, [](auto& sys) {sys.descriptions[0].decorations.place(sys.builders[0].location({1, 0.4}), "hh", "syn0"); }); + check_compatible_mechanism_failure(gprop, [](auto& sys) {sys.descriptions[1].decorations.paint(sys.builders[1].cable(mcable{0}), "expsyn"); }); + check_compatible_mechanism_failure(gprop, [](auto& sys) {sys.descriptions[0].decorations.set_default(ion_reversal_potential_method{"na", "expsyn"}); }); - EXPECT_THROW(fvm_build_mechanism_data(gprop, cells, D), arb::cable_cell_error); - } + gprop.default_parameters.reversal_potential_method["na"] = "pas"; + check_compatible_mechanism_failure(gprop, [](auto& sys) {}); } TEST(fvm_layout, mech_index) {