From 5fdfb02e45cb4128dc5a19babd43b7ba347c1229 Mon Sep 17 00:00:00 2001 From: Nora Abi Akar <nora.abiakar@gmail.com> Date: Mon, 14 Dec 2020 18:16:07 +0100 Subject: [PATCH] Expose global_properties in Python recipes (#1273) Extend Ptyhon recipe wrapper to support setting of global cell properties. * Rename `pyrecipe::get_probes` to `py_recipe::probes` * Remove `global_properties_shim` : It's easy to make mistakes with the class, as it holds a `mechanism_catalogue` and a `cable_cell_global_properties` which holds a pointer to the catalogue. This would likely have caused issues with the users. * Expose `py_recipe::global_properties` --- doc/python/recipe.rst | 13 ++++- python/cells.cpp | 53 ++++++++++--------- python/cells.hpp | 17 ------ python/example/network_ring.py | 8 ++- python/example/single_cell_recipe.py | 8 ++- python/mechanism.cpp | 2 +- python/recipe.cpp | 28 ++++++++-- python/recipe.hpp | 25 +++++---- python/single_cell_model.cpp | 14 ++--- python/test/unit/test_cable_probes.py | 9 +++- python/test/unit/test_simulator.py | 10 +++- .../test/unit_distributed/test_simulator.py | 9 +++- 12 files changed, 121 insertions(+), 75 deletions(-) delete mode 100644 python/cells.hpp diff --git a/doc/python/recipe.rst b/doc/python/recipe.rst index eade1da2..5d74801e 100644 --- a/doc/python/recipe.rst +++ b/doc/python/recipe.rst @@ -90,7 +90,7 @@ Recipe By default returns 0. - .. function:: get_probes(gid) + .. function:: probes(gid) Returns a list specifying the probe addresses describing probes on the cell ``gid``. Each address in the list is an opaque object of type :class:`probe` produced by @@ -100,6 +100,17 @@ Recipe By default returns an empty list. + .. function:: global_properties(kind) + + The global properties of a model. + + This method needs to be implemented for :class:`arbor.cell_kind.cable`, where the + properties include ion concentrations and reversal potentials; initial membrane voltage; + temperature; axial resistivity; membrane capacitance; cv_policy; and a pointer + to the mechanism catalogue. + + By default returns an empty object. + Cells ------ diff --git a/python/cells.cpp b/python/cells.cpp index 9017e445..69b691f1 100644 --- a/python/cells.cpp +++ b/python/cells.cpp @@ -26,7 +26,6 @@ #include "arbor/cable_cell_param.hpp" #include "arbor/cv_policy.hpp" -#include "cells.hpp" #include "conversion.hpp" #include "error.hpp" #include "pybind11/cast.h" @@ -129,23 +128,16 @@ struct label_dict_proxy { } }; -global_props_shim::global_props_shim(): - cat(arb::global_default_catalogue()) -{ - props.catalogue = &cat; -} - // This isn't pretty. Partly because the information in the global parameters // is all over the place. -std::string to_string(const global_props_shim& G) { +std::string to_string(const arb::cable_cell_global_properties& props) { std::string s = "{arbor.cable_global_properties"; - const auto& P = G.props; - const auto& D = P.default_parameters; + const auto& D = props.default_parameters; const auto& I = D.ion_data; // name, valence, int_con, ext_con, rev_pot, rev_pot_method s += "\n ions: {"; - for (auto& ion: P.ion_species) { + for (auto& ion: props.ion_species) { if (!I.count(ion.first)) { s += util::pprintf("\n {name: '{}', valence: {}, int_con: None, ext_con: None, rev_pot: None, rev_pot_method: None}", ion.first, ion.second); @@ -437,23 +429,23 @@ void register_cells(pybind11::module& m) { // arb::cable_cell_global_properties - pybind11::class_<global_props_shim> gprop(m, "cable_global_properties"); + pybind11::class_<arb::cable_cell_global_properties> gprop(m, "cable_global_properties"); gprop .def(pybind11::init<>()) - .def(pybind11::init<const global_props_shim&>()) - .def("check", [](const global_props_shim& shim) { - arb::check_global_properties(shim.props);}, + .def(pybind11::init<const arb::cable_cell_global_properties&>()) + .def("check", [](const arb::cable_cell_global_properties& props) { + arb::check_global_properties(props);}, "Test whether all default parameters and ion species properties have been set.") // set cable properties .def("set_property", - [](global_props_shim& G, + [](arb::cable_cell_global_properties& props, optional<double> Vm, optional<double> cm, optional<double> rL, optional<double> tempK) { - if (Vm) G.props.default_parameters.init_membrane_potential = Vm; - if (cm) G.props.default_parameters.membrane_capacitance=cm; - if (rL) G.props.default_parameters.axial_resistivity=rL; - if (tempK) G.props.default_parameters.temperature_K=tempK; + if (Vm) props.default_parameters.init_membrane_potential = Vm; + if (cm) props.default_parameters.membrane_capacitance=cm; + if (rL) props.default_parameters.axial_resistivity=rL; + if (tempK) props.default_parameters.temperature_K=tempK; }, pybind11::arg_v("Vm", pybind11::none(), "initial membrane voltage [mV]."), pybind11::arg_v("cm", pybind11::none(), "membrane capacitance [F/m²]."), @@ -462,16 +454,16 @@ void register_cells(pybind11::module& m) { "Set global default values for cable and cell properties.") // add/modify ion species .def("set_ion", - [](global_props_shim& G, const char* ion, + [](arb::cable_cell_global_properties& props, const char* ion, optional<double> int_con, optional<double> ext_con, optional<double> rev_pot, pybind11::object method) { - auto& data = G.props.default_parameters.ion_data[ion]; + auto& data = props.default_parameters.ion_data[ion]; if (int_con) data.init_int_concentration = *int_con; if (ext_con) data.init_ext_concentration = *ext_con; if (rev_pot) data.init_reversal_potential = *rev_pot; if (auto m = maybe_method(method)) { - G.props.default_parameters.reversal_potential_method[ion] = *m; + props.default_parameters.reversal_potential_method[ion] = *m; } }, pybind11::arg_v("ion", "name of the ion species."), @@ -484,9 +476,18 @@ void register_cells(pybind11::module& m) { "specific regions using the paint interface, while the method for calculating\n" "reversal potential is global for all compartments in the cell, and can't be\n" "overriden locally.") - .def_readwrite("catalogue", &global_props_shim::cat, "The mechanism catalogue.") - .def("__str__", [](const global_props_shim& p){return to_string(p);}); - + .def("register", [](arb::cable_cell_global_properties& props, const arb::mechanism_catalogue& cat) { + props.catalogue = &cat; + }, + "Register the pointer to the mechanism catalogue in the global properties") + .def("__str__", [](const arb::cable_cell_global_properties& p){return to_string(p);}); + + m.def("neuron_cable_propetries", []() { + arb::cable_cell_global_properties prop; + prop.default_parameters = arb::neuron_parameter_defaults; + return prop; + }, + "default NEURON cable_global_properties"); pybind11::class_<arb::decor> decor(m, "decor", "Description of the decorations to be applied to a cable cell, that is the painted,\n" diff --git a/python/cells.hpp b/python/cells.hpp deleted file mode 100644 index 8560f16a..00000000 --- a/python/cells.hpp +++ /dev/null @@ -1,17 +0,0 @@ -#pragma once - -#include <pybind11/pybind11.h> - -#include <arbor/cable_cell_param.hpp> -#include <arbor/mechcat.hpp> -#include <arbor/util/unique_any.hpp> - -namespace pyarb { - -struct global_props_shim { - arb::mechanism_catalogue cat; - arb::cable_cell_global_properties props; - global_props_shim(); -}; - -} diff --git a/python/example/network_ring.py b/python/example/network_ring.py index b080ab02..1fb552ad 100755 --- a/python/example/network_ring.py +++ b/python/example/network_ring.py @@ -62,6 +62,9 @@ class ring_recipe (arbor.recipe): # all memory in the C++ class is initialized correctly. arbor.recipe.__init__(self) self.ncells = n + self.props = arbor.neuron_cable_propetries() + self.cat = arbor.default_catalogue() + self.props.register(self.cat) # The num_cells method that returns the total number of cells in the model # must be implemented. @@ -97,9 +100,12 @@ class ring_recipe (arbor.recipe): return [arbor.event_generator(arbor.cell_member(0,0), 0.1, sched)] return [] - def get_probes(self, gid): + def probes(self, gid): return [arbor.cable_probe_membrane_voltage('(location 0 0)')] + def global_properties(self, kind): + return self.props + context = arbor.context(threads=12, gpu_id=None) print(context) diff --git a/python/example/single_cell_recipe.py b/python/example/single_cell_recipe.py index a9b72880..76af62c5 100644 --- a/python/example/single_cell_recipe.py +++ b/python/example/single_cell_recipe.py @@ -12,6 +12,9 @@ class single_recipe (arbor.recipe): arbor.recipe.__init__(self) self.the_cell = cell self.the_probes = probes + self.the_props = arbor.neuron_cable_propetries() + self.the_cat = arbor.default_catalogue() + self.the_props.register(self.the_cat) def num_cells(self): return 1 @@ -25,9 +28,12 @@ class single_recipe (arbor.recipe): def cell_description(self, gid): return self.the_cell - def get_probes(self, gid): + def probes(self, gid): return self.the_probes + def global_properties(self, kind): + return self.the_props + # (2) Create a cell. # Morphology diff --git a/python/mechanism.cpp b/python/mechanism.cpp index e0fc9846..70e2c528 100644 --- a/python/mechanism.cpp +++ b/python/mechanism.cpp @@ -103,7 +103,7 @@ void register_mechanisms(pybind11::module& m) { [](const arb::mechanism_info& inf) { return util::pprintf("(arbor.mechanism_info)"); }); - pybind11::class_<arb::mechanism_catalogue> cat(m, "mechanism_catalogue"); + pybind11::class_<arb::mechanism_catalogue> cat(m, "catalogue"); cat .def(pybind11::init<const arb::mechanism_catalogue&>()) .def("has", &arb::mechanism_catalogue::has, diff --git a/python/recipe.cpp b/python/recipe.cpp index d543fe6d..21b9a7ec 100644 --- a/python/recipe.cpp +++ b/python/recipe.cpp @@ -14,7 +14,6 @@ #include <arbor/morph/primitives.hpp> #include <arbor/recipe.hpp> -#include "cells.hpp" #include "conversion.hpp" #include "error.hpp" #include "event_generator.hpp" @@ -64,6 +63,26 @@ static arb::util::unique_any convert_cell(pybind11::object o) { "Python error already thrown"); } +// Convert global properties inside a Python object to a +// std::any, as required by the recipe interface. +// This helper is only to called while holding the GIL, see above. +static std::any convert_gprop(pybind11::object o) { + if (o.is(pybind11::none())) { + return {}; + } + return pybind11::cast<arb::cable_cell_global_properties>(o); +} + +// The py::recipe::global_properties returns a pybind11::object, that is +// unwrapped and copied into an std::any. +std::any py_recipe_shim::get_global_properties(arb::cell_kind kind) const { + return try_catch_pyexception([&](){ + pybind11::gil_scoped_acquire guard; + return convert_gprop(impl_->global_properties(kind)); + }, + "Python error already thrown"); +} + // This helper is only to called while holding the GIL, see above. static std::vector<arb::event_generator> convert_gen(std::vector<pybind11::object> pygens, arb::cell_gid_type gid) { using namespace std::string_literals; @@ -95,7 +114,7 @@ std::vector<arb::event_generator> py_recipe_shim::event_generators(arb::cell_gid pybind11::gil_scoped_acquire guard; return convert_gen(impl_->event_generators(gid), gid); }, - "Python error already thrown"); + "Python error already thrown"); } std::string con_to_string(const arb::cell_connection& c) { @@ -186,9 +205,12 @@ void register_recipe(pybind11::module& m) { .def("gap_junctions_on", &py_recipe::gap_junctions_on, "gid"_a, "A list of the gap junctions connected to gid, [] by default.") - .def("get_probes", &py_recipe::get_probes, + .def("probes", &py_recipe::probes, "gid"_a, "The probes to allow monitoring.") + .def("global_properties", &py_recipe::global_properties, + "kind"_a, + "The default properties applied to all cells of type 'kind' in the model.") // TODO: py_recipe::global_properties .def("__str__", [](const py_recipe&){return "<arbor.recipe>";}) .def("__repr__", [](const py_recipe&){return "<arbor.recipe>";}); diff --git a/python/recipe.hpp b/python/recipe.hpp index a6196d0f..b00a608c 100644 --- a/python/recipe.hpp +++ b/python/recipe.hpp @@ -51,9 +51,12 @@ public: virtual std::vector<arb::gap_junction_connection> gap_junctions_on(arb::cell_gid_type) const { return {}; } - virtual std::vector<arb::probe_info> get_probes(arb::cell_gid_type gid) const { + virtual std::vector<arb::probe_info> probes(arb::cell_gid_type gid) const { return {}; } + virtual pybind11::object global_properties(arb::cell_kind kind) const { + return pybind11::none(); + }; //TODO: virtual pybind11::object global_properties(arb::cell_kind kind) const {return pybind11::none();}; }; @@ -95,8 +98,12 @@ public: PYBIND11_OVERLOAD(std::vector<arb::gap_junction_connection>, py_recipe, gap_junctions_on, gid); } - std::vector<arb::probe_info> get_probes(arb::cell_gid_type gid) const override { - PYBIND11_OVERLOAD(std::vector<arb::probe_info>, py_recipe, get_probes, gid); + std::vector<arb::probe_info> probes(arb::cell_gid_type gid) const override { + PYBIND11_OVERLOAD(std::vector<arb::probe_info>, py_recipe, probes, gid); + } + + pybind11::object global_properties(arb::cell_kind kind) const override { + PYBIND11_OVERLOAD(pybind11::object, py_recipe, global_properties, kind); } }; @@ -152,18 +159,10 @@ public: } std::vector<arb::probe_info> get_probes(arb::cell_gid_type gid) const override { - return try_catch_pyexception([&](){ return impl_->get_probes(gid); }, msg); + return try_catch_pyexception([&](){ return impl_->probes(gid); }, msg); } - // TODO: make thread safe - std::any get_global_properties(arb::cell_kind kind) const override { - if (kind==arb::cell_kind::cable) { - arb::cable_cell_global_properties gprop; - gprop.default_parameters = arb::neuron_parameter_defaults; - return gprop; - } - return std::any{}; - } + std::any get_global_properties(arb::cell_kind kind) const override; }; } // namespace pyarb diff --git a/python/single_cell_model.cpp b/python/single_cell_model.cpp index 43029969..fae9d364 100644 --- a/python/single_cell_model.cpp +++ b/python/single_cell_model.cpp @@ -13,7 +13,6 @@ #include <arbor/simulation.hpp> #include <arbor/util/any_cast.hpp> -#include "cells.hpp" #include "error.hpp" #include "strprintf.hpp" @@ -149,15 +148,14 @@ class single_cell_model { std::vector<trace> traces_; public: - // Make gprop public to make it possible to expose it as a field in Python: - // model.properties.catalogue = my_custom_cat - //arb::cable_cell_global_properties gprop; - global_props_shim gprop; + arb::cable_cell_global_properties gprop; + arb::mechanism_catalogue cat; single_cell_model(arb::cable_cell c): cell_(std::move(c)), ctx_(arb::make_context()) { - gprop.props.default_parameters = arb::neuron_parameter_defaults; + gprop.default_parameters = arb::neuron_parameter_defaults; + cat = arb::global_default_catalogue(); } // example use: @@ -180,7 +178,8 @@ public: } void run(double tfinal, double dt) { - single_cell_recipe rec(cell_, probes_, gprop.props); + gprop.catalogue = &cat; + single_cell_recipe rec(cell_, probes_, gprop); auto domdec = arb::partition_load_balance(rec, ctx_); @@ -270,6 +269,7 @@ void register_single_cell(pybind11::module& m) { return m.traces();}, "Holds sample traces after a call to run().") .def_readwrite("properties", &single_cell_model::gprop, "Global properties.") + .def_readwrite("catalogue", &single_cell_model::cat, "Mechanism catalogue.") .def("__repr__", [](const single_cell_model&){return "<arbor.single_cell_model>";}) .def("__str__", [](const single_cell_model&){return "<arbor.single_cell_model>";}); } diff --git a/python/test/unit/test_cable_probes.py b/python/test/unit/test_cable_probes.py index 791b2397..27524179 100644 --- a/python/test/unit/test_cable_probes.py +++ b/python/test/unit/test_cable_probes.py @@ -33,6 +33,10 @@ class cc_recipe(A.recipe): self.cell = A.cable_cell(st, A.label_dict(), dec) + self.cat = A.default_catalogue() + self.props = A.neuron_cable_propetries() + self.props.register(self.cat) + def num_cells(self): return 1 @@ -45,7 +49,10 @@ class cc_recipe(A.recipe): def cell_kind(self, gid): return A.cell_kind.cable - def get_probes(self, gid): + def global_properties(self, kind): + return self.props + + def probes(self, gid): # Use keyword arguments to check that the wrappers have actually declared keyword arguments correctly. # Place single-location probes at (location 0 0.01*j) where j is the index of the probe address in # the returned list. diff --git a/python/test/unit/test_simulator.py b/python/test/unit/test_simulator.py index 9dc26acd..d25c371d 100644 --- a/python/test/unit/test_simulator.py +++ b/python/test/unit/test_simulator.py @@ -29,6 +29,9 @@ class cc2_recipe(A.recipe): st.append(i, (1, 3, 0, 5), 1) st.append(i, (1, -4, 0, 3), 1) self.the_morphology = A.morphology(st) + self.the_cat = A.default_catalogue() + self.the_props = A.neuron_cable_propetries() + self.the_props.register(self.the_cat) def num_cells(self): return 2 @@ -48,7 +51,10 @@ class cc2_recipe(A.recipe): def event_generators(self, gid): return [] - def get_probes(self, gid): + def global_properties(self, kind): + return self.the_props + + def probes(self, gid): # Cell 0 has three voltage probes: # 0, 0: end of branch 1 # 0, 1: end of branch 2 @@ -105,7 +111,7 @@ class lif2_recipe(A.recipe): weight = 400 return [A.event_generator((gid,0), weight, A.regular_schedule(sched_dt)) for gid in range(0, self.num_cells())] - def get_probes(self, gid): + def probes(self, gid): return [] def cell_description(self, gid): diff --git a/python/test/unit_distributed/test_simulator.py b/python/test/unit_distributed/test_simulator.py index ee0da551..8ca179a2 100644 --- a/python/test/unit_distributed/test_simulator.py +++ b/python/test/unit_distributed/test_simulator.py @@ -25,7 +25,9 @@ class lifN_recipe(A.recipe): def __init__(self, n_cell): A.recipe.__init__(self) self.n_cell = n_cell - + self.cat = A.default_catalogue() + self.props = A.neuron_cable_propetries() + self.props.register(self.cat) def num_cells(self): return self.n_cell @@ -46,9 +48,12 @@ class lifN_recipe(A.recipe): weight = 400 return [A.event_generator((gid,0), weight, A.regular_schedule(sched_dt)) for gid in range(0, self.num_cells())] - def get_probes(self, gid): + def probes(self, gid): return [] + def global_properties(self,kind): + return self.props + def cell_description(self, gid): c = A.lif_cell() if gid%2==0: -- GitLab