Anil's review for ACC exporter, fixing create_acc tests

Author: lukasgd

bluepyopt/ephys/create_acc.py

@@ -31,7 +31,7 @@ def __init__(self, name, conv=None):
         self.conv = conv
 
 
-class Nrn2ArbParamFormatter:
+class Nrn2ArbParamAdapter:
     """Converts a Neuron parameter to Arbor format (name and value)"""
 
     _mapping = dict(
@@ -202,7 +202,7 @@ def _format_params_and_group_by_mech(params, channels):
             Mapping of Arbor mechanism name to list of parameters in Arbor
             format
         """
-        mech_params = [Nrn2ArbParamFormatter.format(
+        mech_params = [Nrn2ArbParamAdapter.format(
                        param, channels) for param in params]
         mechs = {mech: [] for mech, _ in mech_params}
         for mech in channels:
@@ -214,7 +214,8 @@ def _format_params_and_group_by_mech(params, channels):
 
     @classmethod
     def process_global(cls, params):
-        """Group global params by mechanism, convert them to Arbor format
+        """Group global BluePyOpt params by mech, convert them to Arbor format
+
         Args:
             params (): List of global parameters in Neuron format
 
@@ -231,7 +232,7 @@ def process_global(cls, params):
 
     @classmethod
     def process_local(cls, params, channels):
-        """Group local params by mechanism, convert them to Arbor format
+        """Group local BluePyOpt params by mech, convert them to Arbor format
 
         Args:
             params (): List of Arbor label/local parameters pairs in Neuron

bluepyopt/ephys/create_hoc.py

@@ -112,12 +112,12 @@ def _loc_desc(location, param_or_mech):
         elif isinstance(param_or_mech, NrnMODPointProcessMechanism):
             raise CreateHocException("%s is currently not supported." %
                                      type(param_or_mech).__name__)
-    elif not (isinstance(location, NrnSeclistCompLocation) or
-              isinstance(location, NrnSectionCompLocation) or
-              isinstance(location, NrnSomaDistanceCompLocation) or
-              isinstance(location, NrnSecSomaDistanceCompLocation) or
-              isinstance(location, NrnTrunkSomaDistanceCompLocation)) and \
-            not isinstance(location, ArbLocation) and \
+    elif not isinstance(location, (NrnSeclistCompLocation,
+                                   NrnSectionCompLocation,
+                                   NrnSomaDistanceCompLocation,
+                                   NrnSecSomaDistanceCompLocation,
+                                   NrnTrunkSomaDistanceCompLocation,
+                                   ArbLocation)) and \
             not isinstance(param_or_mech, NrnPointProcessParameter):
         return location.seclist_name
     else:

bluepyopt/ephys/locations.py

@@ -541,7 +541,14 @@ def acc_label(self):
 class ArbLocation(Location):
     """Arbor Location"""
 
-    pass
+    def instantiate(self, sim=None, icell=None):  # pylint: disable=W0613
+        """Find the instantiate compartment (default implementation)"""
+        raise EPhysLocInstantiateException(
+            '%s not supported in NEURON.' % type(self).__name__)
+
+    def __str__(self):
+        """String representation"""
+        return '%s \'%s\'' % (type(self).__name__, self.acc_label().defn)
 
 
 class ArbSegmentLocation(ArbLocation):
@@ -552,19 +559,10 @@ def __init__(self, name, segment, comment=''):
         super().__init__(name, comment)
         self.segment = segment
 
-    def instantiate(self, sim=None, icell=None):  # pylint: disable=W0613
-        """Find the instantiate compartment"""
-        raise EPhysLocInstantiateException(
-            '%s not supported in NEURON.' % type(self).__name__)
-
     def acc_label(self):
         """Arbor label"""
         return ArbLabel('region', self.name, '(segment %s)' % (self.segment))
 
-    def __str__(self):
-        """String representation"""
-        return '%s \'%s\'' % (type(self).__name__, self.acc_label().defn)
-
 
 class ArbBranchLocation(ArbLocation):
     """Branch in an Arbor morphology.
@@ -576,19 +574,10 @@ def __init__(self, name, branch, comment=''):
         super().__init__(name, comment)
         self.branch = branch
 
-    def instantiate(self, sim=None, icell=None):  # pylint: disable=W0613
-        """Find the instantiate compartment"""
-        raise EPhysLocInstantiateException(
-            '%s not supported in NEURON.' % type(self).__name__)
-
     def acc_label(self):
         """Arbor label"""
         return ArbLabel('region', self.name, '(branch %s)' % (self.branch))
 
-    def __str__(self):
-        """String representation"""
-        return '%s \'%s\'' % (type(self).__name__, self.acc_label().defn)
-
 
 class ArbSegmentRelLocation(ArbLocation):
     """Relative position on a segment in an Arbor morphology.
@@ -599,21 +588,12 @@ def __init__(self, name, segment, pos, comment=''):
         self.segment = segment
         self.pos = pos
 
-    def instantiate(self, sim=None, icell=None):  # pylint: disable=W0613
-        """Find the instantiate compartment"""
-        raise EPhysLocInstantiateException(
-            '%s not supported in NEURON.' % type(self).__name__)
-
     def acc_label(self):
         """Arbor label"""
         return ArbLabel('locset', self.name,
                         '(on-components %s (segment %s))' %
                         (format_float(self.pos), self.segment))
 
-    def __str__(self):
-        """String representation"""
-        return '%s \'%s\'' % (type(self).__name__, self.acc_label().defn)
-
 
 class ArbBranchRelLocation(ArbLocation):
     """Relative position on a branch in an Arbor morphology.
@@ -626,21 +606,12 @@ def __init__(self, name, branch, pos, comment=''):
         self.branch = branch
         self.pos = pos
 
-    def instantiate(self, sim=None, icell=None):  # pylint: disable=W0613
-        """Find the instantiate compartment"""
-        raise EPhysLocInstantiateException(
-            '%s not supported in NEURON.' % type(self).__name__)
-
     def acc_label(self):
         """Arbor label"""
         return ArbLabel('locset', self.name,
                         '(location %s %s)' %
                         (self.branch, format_float(self.pos)))
 
-    def __str__(self):
-        """String representation"""
-        return '%s \'%s\'' % (type(self).__name__, self.acc_label().defn)
-
 
 class ArbLocsetLocation(ArbLocation):
     """Arbor location set defined by a user-supplied string (S-expression).
@@ -650,19 +621,10 @@ def __init__(self, name, locset, comment=''):
         super().__init__(name, comment)
         self.locset = locset
 
-    def instantiate(self, sim=None, icell=None):  # pylint: disable=W0613
-        """Find the instantiate compartment"""
-        raise EPhysLocInstantiateException(
-            '%s not supported in NEURON.' % type(self).__name__)
-
     def acc_label(self):
         """Arbor label"""
         return ArbLabel('locset', self.name, self.locset)
 
-    def __str__(self):
-        """String representation"""
-        return '%s %s' % (type(self).__name__, self.acc_label().defn)
-
 
 class ArbRegionLocation(ArbLocation):
     """Arbor region defined by a user-supplied string (S-expression).
@@ -672,19 +634,10 @@ def __init__(self, name, region, comment=''):
         super().__init__(name, comment)
         self.region = region
 
-    def instantiate(self, sim=None, icell=None):  # pylint: disable=W0613
-        """Find the instantiate compartment"""
-        raise EPhysLocInstantiateException(
-            '%s not supported in NEURON.' % type(self).__name__)
-
     def acc_label(self):
         """Arbor label"""
         return ArbLabel('region', self.name, self.region)
 
-    def __str__(self):
-        """String representation"""
-        return '%s %s' % (type(self).__name__, self.acc_label().defn)
-
 
 class EPhysLocInstantiateException(Exception):
 

bluepyopt/tests/test_ephys/test_create_acc.py

@@ -17,7 +17,7 @@
 
 from bluepyopt import ephys
 from bluepyopt.ephys import create_acc
-from bluepyopt.ephys.create_acc import (Nrn2ArbParamFormatter,
+from bluepyopt.ephys.create_acc import (Nrn2ArbParamAdapter,
                                         Nrn2ArbMechGrouper,
                                         ArbNmodlMechFormatter)
 
@@ -48,52 +48,52 @@ def test_read_templates():
 
 
 @pytest.mark.unit
-def test_Nrn2ArbParamFormatter_param_name():
+def test_Nrn2ArbParamAdapter_param_name():
     """Test Neuron to Arbor parameter mapping."""
     # Identity
     mech_param_name = "gSKv3_1bar_SKv3_1"
-    assert Nrn2ArbParamFormatter._param_name(mech_param_name) \
+    assert Nrn2ArbParamAdapter._param_name(mech_param_name) \
         == mech_param_name
 
     # Non-trivial transformation
     global_property_name = "v_init"
-    assert Nrn2ArbParamFormatter._param_name(global_property_name) \
+    assert Nrn2ArbParamAdapter._param_name(global_property_name) \
         == "membrane-potential"
 
 
 @pytest.mark.unit
-def test_Nrn2ArbParamFormatter_param_value():
+def test_Nrn2ArbParamAdapter_param_value():
     """Test Neuron to Arbor parameter units conversion."""
     # Identity for region parameter
     mech_param = create_acc.Location(name="gSKv3_1bar_SKv3_1", value="1.025")
-    assert Nrn2ArbParamFormatter._param_value(mech_param) == "1.025"
+    assert Nrn2ArbParamAdapter._param_value(mech_param) == "1.025"
 
     # Non-trivial name transformation, but identical value/units
     global_property = create_acc.Location(name="v_init", value=-65)
-    assert Nrn2ArbParamFormatter._param_value(global_property) == "-65"
+    assert Nrn2ArbParamAdapter._param_value(global_property) == "-65"
 
     # Non-trivial name and value/units transformation
     global_property = create_acc.Location(name="celsius", value=34)
-    assert Nrn2ArbParamFormatter._param_value(global_property) == (
+    assert Nrn2ArbParamAdapter._param_value(global_property) == (
         "307.14999999999998")
 
 
 @pytest.mark.unit
-def test_Nrn2ArbParamFormatter_format():
+def test_Nrn2ArbParamAdapter_format():
     """Test Neuron to Arbor parameter reformatting."""
     # Constant mechanism parameter
     mech_param = create_acc.Location(name="gSKv3_1bar_SKv3_1", value="1.025")
     mech = "SKv3_1"
     arb_mech_param = create_acc.Location(name="gSKv3_1bar", value="1.025")
     assert (
-        Nrn2ArbParamFormatter.format(
+        Nrn2ArbParamAdapter.format(
             mech_param, mechs=[mech])
         == (mech, arb_mech_param)
     )
 
     # Non-unique mapping to mechanisms
     with pytest.raises(create_acc.CreateAccException):
-        Nrn2ArbParamFormatter.format(
+        Nrn2ArbParamAdapter.format(
             mech_param, mechs=["SKv3_1", "1"])
 
     # Global property with non-trivial transformation
@@ -102,7 +102,7 @@ def test_Nrn2ArbParamFormatter_format():
     arb_global_property = create_acc.Location(
         name="temperature-kelvin", value="273.14999999999998")
     # Non-trivial name and value/units transformation
-    assert Nrn2ArbParamFormatter.format(global_property, []) == \
+    assert Nrn2ArbParamAdapter.format(global_property, []) == \
         (mech, arb_global_property)
 
     # Inhomogeneuos mechanism parameter
@@ -126,7 +126,7 @@ def test_Nrn2ArbParamFormatter_format():
         value_scaler=param_scaler,
     )
     assert (
-        Nrn2ArbParamFormatter.format(
+        Nrn2ArbParamAdapter.format(
             iexpr_param, mechs=[mech])
         == (mech, arb_iexpr_param)
     )
@@ -151,7 +151,7 @@ def test_Nrn2ArbParamFormatter_format():
     arb_point_expr_param = create_acc.PointExpr(
         name="tau", value="10", point_loc=[mech_loc])
     assert (
-        Nrn2ArbParamFormatter.format(
+        Nrn2ArbParamAdapter.format(
             point_expr_param, mechs=[mech])
         == (mech, arb_point_expr_param)
     )
@@ -540,8 +540,6 @@ def test_cell_model_write_and_read_acc_replace_axon():
             assert len(e.args) == 1 and e.args[0] == \
                 "Need a newer version of Arbor for axon replacement."
             return
-        finally:
-            cell.destroy(nrn_sim)
 
         # Axon replacement implemented in installed Arbor version
         cell_json, arb_morph, arb_decor, arb_labels = \