Clean-up catalogue extension. (#2409)

1. rename C++ `catalogue::import` to `catalogue::extend` as `import`
will likely be a reserved word due to modules
2. Use an empty default for the prefix. 99% real world examples use an
empty prefix.
3. Adjust tests and examples accordingly.

arbor/include/arbor/mechcat.hpp

@@ -96,7 +96,7 @@ class ARB_ARBOR_API mechanism_catalogue {
     }
 
     // Copy over another catalogue's mechanism and attach a -- possibly empty -- prefix
-    void import(const mechanism_catalogue& other, const std::string& prefix);
+    void extend(const mechanism_catalogue& other, const std::string& prefix = "");
 
     ~mechanism_catalogue();
 

arbor/mechcat.cpp

@@ -1,5 +1,4 @@
 #include <cstdlib>
-#include <map>
 #include <memory>
 #include <string>
 #include <vector>
@@ -114,19 +113,19 @@ struct catalogue_state {
     catalogue_state() = default;
 
     catalogue_state(const catalogue_state& other) {
-        import(other, "");
+        extend(other, "");
     }
 
     catalogue_state& operator=(const catalogue_state& other) {
         *this = {};
-        import(other, "");
+        extend(other, "");
         return *this;
     }
 
     catalogue_state& operator=(catalogue_state&&) = default;
     catalogue_state(catalogue_state&& other) = default;
 
-    void import(const catalogue_state& other, const std::string& prefix) {
+    void extend(const catalogue_state& other, const std::string& prefix) {
         // Do all checks before adding anything, otherwise we might get inconsistent state.
         auto assert_undefined = [&](const std::string& key) {
             auto pkey = prefix+key;
@@ -579,8 +578,8 @@ void mechanism_catalogue::derive(const std::string& name, const std::string& par
     state_->bind(name, value(state_->derive(parent)));
 }
 
-void mechanism_catalogue::import(const mechanism_catalogue& other, const std::string& prefix) {
-    state_->import(*other.state_, prefix);
+void mechanism_catalogue::extend(const mechanism_catalogue& other, const std::string& prefix) {
+    state_->extend(*other.state_, prefix);
 }
 
 void mechanism_catalogue::remove(const std::string& name) {

doc/python/mechanisms.rst

@@ -469,7 +469,7 @@ Mechanism catalogues
         :return: :class:`py_mech_cat_iterator`
 
 
-    .. py:method:: extend(other, prefix)
+    .. py:method:: extend(other, prefix="")
 
         Import another catalogue, possibly with a prefix. Will raise an exception
         in case of name collisions.
@@ -479,7 +479,7 @@ Mechanism catalogues
             import arbor
 
             cat = arbor.default_catalogue()
-            cat.extend(arbor.allen_catalogue(), "")
+            cat.extend(arbor.allen_catalogue())
 
         :param other: reference to other catalogue.
         :type other: :class:`mechanism_catalogue`

example/busyring/ring.cpp

@@ -56,7 +56,7 @@ class ring_recipe: public arb::recipe {
         params_(params)
     {
         gprop.default_parameters = arb::neuron_parameter_defaults;
-        gprop.catalogue.import(arb::global_allen_catalogue(), "");
+        gprop.catalogue.extend(arb::global_allen_catalogue());
 
         if (params.cell.complex_cell) {
             gprop.default_parameters.reversal_potential_method["ca"] = "nernst/ca";

example/ornstein_uhlenbeck/ou.cpp

@@ -21,7 +21,7 @@ class recipe: public arb::recipe {
 
         // build catalogue with stochastic mechanism
         cell_gprop_.catalogue = global_default_catalogue();
-        cell_gprop_.catalogue.import(arb::global_ornstein_uhlenbeck_catalogue(), "");
+        cell_gprop_.catalogue.extend(arb::global_ornstein_uhlenbeck_catalogue());
         cell_gprop_.default_parameters = neuron_parameter_defaults;
 
         // paint the process on the whole cell

python/example/calcium_stdp.py

@@ -74,7 +74,7 @@ def __init__(self, cell, time_lags):
         self.the_cell = cell
         # create extended catalogue including stochastic mechanisms
         self.the_props = A.neuron_cable_properties()
-        self.the_props.catalogue.extend(A.stochastic_catalogue(), "")
+        self.the_props.catalogue.extend(A.stochastic_catalogue())
         self.time_lags = time_lags
         self.num = len(time_lags)
 

python/example/ou_lif/ou_lif.py

@@ -16,7 +16,7 @@ def make_catalogue():
     print(out)
     # load the new catalogue and extend it with builtin stochastic catalogue
     cat = A.load_catalogue("./ou_lif-catalogue.so")
-    cat.extend(A.stochastic_catalogue(), "")
+    cat.extend(A.stochastic_catalogue())
     return cat
 
 

python/example/single_cell_allen.py

@@ -145,7 +145,7 @@ def make_cell(base, swc, fit):
 model.probe("voltage", '"midpoint"', "Um", frequency=1 / (5 * U.us))
 
 # (14) Install the Allen mechanism catalogue.
-model.properties.catalogue.extend(A.allen_catalogue(), "")
+model.properties.catalogue.extend(A.allen_catalogue())
 
 # (15) Run simulation
 model.run(tfinal=1.4 * U.s, dt=5 * U.us)

python/example/single_cell_detailed.py

@@ -96,7 +96,7 @@
 # second string parameter that can prefix the name of the mechanisms to avoid
 # collisions between catalogues in this case we have no collisions so we use an
 # empty prefix string.
-model.properties.catalogue.extend(A.allen_catalogue(), "")
+model.properties.catalogue.extend(A.allen_catalogue())
 
 # (7) Add probes.
 # Add a voltage probe on "custom_terminal"

python/example/single_cell_detailed_recipe.py

@@ -104,7 +104,7 @@ def __init__(self):
         self.the_props.set_ion(
             ion="ca", int_con=5e-5 * U.mM, ext_con=2 * U.mM, rev_pot=132.5 * U.mV
         )
-        self.the_props.catalogue.extend(A.allen_catalogue(), "")
+        self.the_props.catalogue.extend(A.allen_catalogue())
 
     # (5.2) Override the num_cells method
     def num_cells(self):

python/mechanism.cpp

@@ -180,9 +180,9 @@ void register_mechanisms(pybind11::module& m) {
                     throw pybind11::key_error(name);
                 }
             })
-        .def("extend", &arb::mechanism_catalogue::import,
+        .def("extend", &arb::mechanism_catalogue::extend,
              "other"_a, "Catalogue to import into self",
-             "prefix"_a, "Prefix for names in other",
+             "prefix"_a="", "Prefix for names in other",
              "Import another catalogue, possibly with a prefix. Will overwrite in case of name collisions.")
         .def("derive", &apply_derive,
                 "name"_a, "parent"_a,

python/test/unit/test_catalogues.py

@@ -77,7 +77,7 @@ def hash_(cat):
         # Test empty constructor
         self.assertEqual(0, len(cat), "Expected no mechanisms in `arbor.catalogue()`.")
         # Test empty extend
-        other.extend(cat, "")
+        other.extend(cat)
         self.assertEqual(
             hash_(ref), hash_(other), "Extending cat with empty should not change cat."
         )
@@ -93,7 +93,7 @@ def hash_(cat):
         self.assertEqual(
             0, len(cat), "Extending cat with prefixed empty should not change empty."
         )
-        cat.extend(other, "")
+        cat.extend(other)
         self.assertEqual(
             hash_(other),
             hash_(cat),

test/unit/test_fvm_layout.cpp

@@ -995,7 +995,7 @@ TEST(fvm_layout, gj_example_2) {
     // Check the GJ CV map
     cable_cell_global_properties gprop;
     gprop.catalogue = make_unit_test_catalogue();
-    gprop.catalogue.import(arb::global_default_catalogue(), "");
+    gprop.catalogue.extend(arb::global_default_catalogue());
     gprop.default_parameters = neuron_parameter_defaults;
 
     auto cells = system.cells();

test/unit/test_fvm_lowered.cpp

@@ -845,7 +845,7 @@ TEST(fvm_lowered, post_events_shared_state) {
             arb::cable_cell_global_properties gprop;
             gprop.default_parameters = arb::neuron_parameter_defaults;
             gprop.catalogue = make_unit_test_catalogue();
-            gprop.catalogue.import(arb::global_default_catalogue(), "");
+            gprop.catalogue.extend(arb::global_default_catalogue());
             return gprop;
         }
 

test/unit/test_mechcat.cpp

@@ -540,7 +540,7 @@ TEST(mechcat, copy) {
 TEST(mechcat, import) {
     auto cat = build_fake_catalogue();
     mechanism_catalogue cat2;
-    cat2.import(cat, "fake_");
+    cat2.extend(cat, "fake_");
 
     EXPECT_TRUE(cat.has("fleeb2"));
     EXPECT_FALSE(cat.has("fake_fleeb2"));
@@ -561,8 +561,8 @@ TEST(mechcat, import_collisions) {
         auto cat = build_fake_catalogue();
 
         mechanism_catalogue cat2;
-        EXPECT_NO_THROW(cat2.import(cat, "prefix:")); // Should have no collisions.
-        EXPECT_NO_THROW(cat.import(cat2, "prefix:")); // Should have no collisions here either.
+        EXPECT_NO_THROW(cat2.extend(cat, "prefix:")); // Should have no collisions.
+        EXPECT_NO_THROW(cat.extend(cat2, "prefix:")); // Should have no collisions here either.
 
         // cat should have both original entries and copies with 'prefix:prefix:' prefixed.
         ASSERT_TRUE(cat.has("fleeb2"));
@@ -580,7 +580,7 @@ TEST(mechcat, import_collisions) {
             mechanism_catalogue other;
             other.add("fleeb", mk_burble_info()); // Note different mechanism info!
 
-            EXPECT_THROW(cat.import(other, ""), arb::duplicate_mechanism);
+            EXPECT_THROW(cat.extend(other), arb::duplicate_mechanism);
             ASSERT_EQ(cat["fleeb"], mk_fleeb_info());
         }
 
@@ -592,7 +592,7 @@ TEST(mechcat, import_collisions) {
             other.add("fleeb2", mk_burble_info());
 
             auto fleeb2_info = cat["fleeb2"];
-            EXPECT_THROW(cat.import(other, ""), arb::duplicate_mechanism);
+            EXPECT_THROW(cat.extend(other), arb::duplicate_mechanism);
             EXPECT_EQ(cat["fleeb2"], fleeb2_info);
         }
 
@@ -606,7 +606,7 @@ TEST(mechcat, import_collisions) {
             ASSERT_FALSE(other["fleeb"]==mk_fleeb_info());
 
             ASSERT_FALSE(cat.has("zonkers"));
-            EXPECT_THROW(cat.import(other, ""), arb::duplicate_mechanism);
+            EXPECT_THROW(cat.extend(other), arb::duplicate_mechanism);
             EXPECT_EQ(cat["fleeb"], mk_fleeb_info());
             EXPECT_FALSE(cat.has("zonkers"));
         }
@@ -623,7 +623,7 @@ TEST(mechcat, import_collisions) {
             ASSERT_FALSE(other["fleeb2"]==fleeb2_info);
 
             ASSERT_FALSE(cat.has("zonkers"));
-            EXPECT_THROW(cat.import(other, ""), arb::duplicate_mechanism);
+            EXPECT_THROW(cat.extend(other), arb::duplicate_mechanism);
             EXPECT_EQ(cat["fleeb2"], fleeb2_info);
             EXPECT_FALSE(cat.has("zonkers"));
         }

test/unit/test_sde.cpp

@@ -244,7 +244,7 @@ class simple_sde_recipe: public simple_recipe_base {
     : simple_recipe_base()
     , ncell_(ncell) {
         // add unit test catalogue
-        cell_gprop_.catalogue.import(make_unit_test_catalogue(), "");
+        cell_gprop_.catalogue.extend(make_unit_test_catalogue());
 
         simple_rec_ptr = this;
         auto inst = cell_gprop_.catalogue.instance(arb_backend_kind_cpu, "mean_reverting_stochastic_density_process");
@@ -322,7 +322,7 @@ class sde_recipe: public simple_recipe_base {
     : simple_recipe_base()
     , ncell_(ncell) {
         // add unit test catalogue
-        cell_gprop_.catalogue.import(make_unit_test_catalogue(), "");
+        cell_gprop_.catalogue.extend(make_unit_test_catalogue());
 
         // replace mechanisms' advance methods
         if (replace_implementation) {
@@ -952,7 +952,7 @@ class sde_recipe_gpu: public simple_recipe_base {
     : simple_recipe_base()
     , ncell_(ncell) {
         // add unit test catalogue
-        cell_gprop_.catalogue.import(make_unit_test_catalogue(), "");
+        cell_gprop_.catalogue.extend(make_unit_test_catalogue());
 
         rec_gpu_ptr = this;
 

test/unit/unit_test_catalogue.cpp

@@ -23,7 +23,7 @@
 
 arb::mechanism_catalogue make_unit_test_catalogue(const arb::mechanism_catalogue& from) {
     auto result = from;
-    result.import(arb::global_testing_catalogue(), "");
+    result.extend(arb::global_testing_catalogue());
     return result;
 }