From 32782a3a004403146d2518e2433bb1b3c20e63b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaquier=20Aur=C3=A9lien=20Tristan?= Date: Thu, 14 Sep 2023 16:00:43 +0200 Subject: [PATCH 1/9] replace stimulus by recording in _set_morphology_dependent_locations for clarity Change-Id: Ied07c52b6f3646b5287b11535eb6619a6f85f3a9 --- .../fitness_calculator_configuration.py | 50 ++++++++++--------- 1 file changed, 26 insertions(+), 24 deletions(-) diff --git a/bluepyemodel/evaluation/fitness_calculator_configuration.py b/bluepyemodel/evaluation/fitness_calculator_configuration.py index 3d34b66f..28bae5d4 100644 --- a/bluepyemodel/evaluation/fitness_calculator_configuration.py +++ b/bluepyemodel/evaluation/fitness_calculator_configuration.py @@ -29,45 +29,45 @@ logger = logging.getLogger(__name__) -def _set_morphology_dependent_locations(stimulus, cell): +def _set_morphology_dependent_locations(recording, cell): """Here we deal with morphology dependent locations""" - def _get_stim(stimulus, sec_id): - new_stim = deepcopy(stimulus) - stim_split = stimulus["name"].split(".") - new_stim["type"] = "nrnseclistcomp" - new_stim["name"] = f"{'.'.join(stim_split[:-1])}_{sec_id}.{stim_split[-1]}" - new_stim["sec_index"] = sec_id - return new_stim - - new_stims = [] - if stimulus["type"] == "somadistanceapic": - new_stims = [deepcopy(stimulus)] - new_stims[0]["sec_name"] = seclist_to_sec.get( - stimulus["seclist_name"], stimulus["seclist_name"] + def _get_rec(recording, sec_id): + new_rec = deepcopy(recording) + rec_split = recording["name"].split(".") + new_rec["type"] = "nrnseclistcomp" + new_rec["name"] = f"{'.'.join(rec_split[:-1])}_{sec_id}.{rec_split[-1]}" + new_rec["sec_index"] = sec_id + return new_rec + + new_recs = [] + if recording["type"] == "somadistanceapic": + new_recs = [deepcopy(recording)] + new_recs[0]["sec_name"] = seclist_to_sec.get( + recording["seclist_name"], recording["seclist_name"] ) - elif stimulus["type"] == "terminal_sections": + elif recording["type"] == "terminal_sections": # all terminal sections - for sec_id, section in enumerate(getattr(cell.icell, stimulus["seclist_name"])): + for sec_id, section in enumerate(getattr(cell.icell, recording["seclist_name"])): if len(section.subtree()) == 1: - new_stims.append(_get_stim(stimulus, sec_id)) + new_recs.append(_get_rec(recording, sec_id)) - elif stimulus["type"] == "all_sections": + elif recording["type"] == "all_sections": # all section of given type - for sec_id, section in enumerate(getattr(cell.icell, stimulus["seclist_name"])): - new_stims.append(_get_stim(stimulus, sec_id)) + for sec_id, section in enumerate(getattr(cell.icell, recording["seclist_name"])): + new_recs.append(_get_rec(recording, sec_id)) else: - new_stims = [deepcopy(stimulus)] + new_recs = [deepcopy(recording)] - if len(new_stims) == 0 and stimulus["type"] in [ + if len(new_recs) == 0 and recording["type"] in [ "somadistanceapic", "terminal_sections", "all_sections", ]: - logger.warning("We could not add a location for %s", stimulus) - return new_stims + logger.warning("We could not add a location for %s", recording) + return new_recs class FitnessCalculatorConfiguration: @@ -506,6 +506,8 @@ def configure_morphology_dependent_locations(self, _cell, simulator): for j, rec in enumerate(protocol.recordings): if rec["type"] != "CompRecording": for _rec in _set_morphology_dependent_locations(rec, cell): + try: + tmp_stim = recordings.append(_rec) else: recordings.append(self.protocols[i].recordings[j]) From fcf39d22617578664ba642c400e0891cb33774a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaquier=20Aur=C3=A9lien=20Tristan?= Date: Thu, 14 Sep 2023 16:19:33 +0200 Subject: [PATCH 2/9] filter recording and their efeatures using location outside of cell Change-Id: I9c49a3a75f3971bfa5f0c745c84d97fba8cf20a6 --- .../fitness_calculator_configuration.py | 22 +++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/bluepyemodel/evaluation/fitness_calculator_configuration.py b/bluepyemodel/evaluation/fitness_calculator_configuration.py index 28bae5d4..67cd286f 100644 --- a/bluepyemodel/evaluation/fitness_calculator_configuration.py +++ b/bluepyemodel/evaluation/fitness_calculator_configuration.py @@ -25,6 +25,7 @@ from bluepyemodel.evaluation.evaluator import seclist_to_sec from bluepyemodel.evaluation.protocol_configuration import ProtocolConfiguration from bluepyemodel.tools.utils import are_same_protocol +from bluepyopt.ephys.locations import EPhysLocInstantiateException logger = logging.getLogger(__name__) @@ -501,24 +502,37 @@ def configure_morphology_dependent_locations(self, _cell, simulator): # TODO: THE SAME FOR STIMULI + skipped_recordings = [] for i, protocol in enumerate(self.protocols): recordings = [] for j, rec in enumerate(protocol.recordings): if rec["type"] != "CompRecording": for _rec in _set_morphology_dependent_locations(rec, cell): try: - tmp_stim = - recordings.append(_rec) + tmp_rec = deepcopy(_rec) + tmp_rec.location.instantiate() + recordings.append(_rec) + except EPhysLocInstantiateException: + logger.warning( + f"Could not find {_rec.location.name}, " + "ignoring recording at this location" + ) + skipped_recordings.append((protocol.name, _rec.name)) else: recordings.append(self.protocols[i].recordings[j]) self.protocols[i].recordings = recordings - # if the loc of the recording is of the form axon*.v, we replace * by - # all the corresponding int from the created recordings to_remove = [] efeatures = [] for i, efeature in enumerate(self.efeatures): if isinstance(efeature.recording_name, str): + # remove efeature associated to skipped recording + for skiprec in skipped_recordings: + if efeature.protocol_name == skiprec[0] and efeature.recording_name == skiprec[1]: + to_remove.append(i) + continue + # if the loc of the recording is of the form axon*.v, we replace * by + # all the corresponding int from the created recordings loc_name, rec_name = efeature.recording_name.split(".") if loc_name[-1] == "*": to_remove.append(i) From ddb23d82435c171f97e25d68f925afde50115487 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaquier=20Aur=C3=A9lien=20Tristan?= Date: Fri, 15 Sep 2023 09:05:41 +0200 Subject: [PATCH 3/9] bug fix Change-Id: Ib6a9868e4347bd03a4c0b2e15f9dca5f9480e6af --- bluepyemodel/emodel_pipeline/emodel_pipeline.py | 1 + bluepyemodel/evaluation/fitness_calculator_configuration.py | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/bluepyemodel/emodel_pipeline/emodel_pipeline.py b/bluepyemodel/emodel_pipeline/emodel_pipeline.py index 4aa6f8f4..af5f35eb 100644 --- a/bluepyemodel/emodel_pipeline/emodel_pipeline.py +++ b/bluepyemodel/emodel_pipeline/emodel_pipeline.py @@ -60,6 +60,7 @@ def __init__( recipes_path=None, use_ipyparallel=None, use_multiprocessing=None, + data_access_point=None, ): """Initializes the EModel_pipeline. diff --git a/bluepyemodel/evaluation/fitness_calculator_configuration.py b/bluepyemodel/evaluation/fitness_calculator_configuration.py index 67cd286f..64d9a828 100644 --- a/bluepyemodel/evaluation/fitness_calculator_configuration.py +++ b/bluepyemodel/evaluation/fitness_calculator_configuration.py @@ -22,6 +22,7 @@ from bluepyemodel.evaluation.efeature_configuration import EFeatureConfiguration from bluepyemodel.evaluation.evaluator import LEGACY_PRE_PROTOCOLS from bluepyemodel.evaluation.evaluator import PRE_PROTOCOLS +from bluepyemodel.evaluation.evaluator import define_location from bluepyemodel.evaluation.evaluator import seclist_to_sec from bluepyemodel.evaluation.protocol_configuration import ProtocolConfiguration from bluepyemodel.tools.utils import are_same_protocol @@ -510,11 +511,12 @@ def configure_morphology_dependent_locations(self, _cell, simulator): for _rec in _set_morphology_dependent_locations(rec, cell): try: tmp_rec = deepcopy(_rec) - tmp_rec.location.instantiate() + location = define_location(tmp_rec) + location.instantiate(sim=simulator, icell=cell.icell) recordings.append(_rec) except EPhysLocInstantiateException: logger.warning( - f"Could not find {_rec.location.name}, " + f"Could not find {location.name}, " "ignoring recording at this location" ) skipped_recordings.append((protocol.name, _rec.name)) From 691d2e6623067dcb7be5b56f3eeb7abc263f08ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaquier=20Aur=C3=A9lien=20Tristan?= Date: Fri, 15 Sep 2023 11:09:54 +0200 Subject: [PATCH 4/9] Only save recording name in skipped recording Change-Id: I7c54d3336cae013ab3c01cc1f404930696c87dac --- bluepyemodel/evaluation/fitness_calculator_configuration.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/bluepyemodel/evaluation/fitness_calculator_configuration.py b/bluepyemodel/evaluation/fitness_calculator_configuration.py index 64d9a828..a2cbf956 100644 --- a/bluepyemodel/evaluation/fitness_calculator_configuration.py +++ b/bluepyemodel/evaluation/fitness_calculator_configuration.py @@ -519,7 +519,7 @@ def configure_morphology_dependent_locations(self, _cell, simulator): f"Could not find {location.name}, " "ignoring recording at this location" ) - skipped_recordings.append((protocol.name, _rec.name)) + skipped_recordings.append(_rec["name"]) else: recordings.append(self.protocols[i].recordings[j]) self.protocols[i].recordings = recordings @@ -530,8 +530,9 @@ def configure_morphology_dependent_locations(self, _cell, simulator): if isinstance(efeature.recording_name, str): # remove efeature associated to skipped recording for skiprec in skipped_recordings: - if efeature.protocol_name == skiprec[0] and efeature.recording_name == skiprec[1]: + if f"{efeature.protocol_name}.{efeature.recording_name}" == skiprec: to_remove.append(i) + logger.warning(f"removing {efeature.name}") continue # if the loc of the recording is of the form axon*.v, we replace * by # all the corresponding int from the created recordings From d7dbc4ca99fc501476128b95ba23a08081b48ab8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaquier=20Aur=C3=A9lien=20Tristan?= Date: Fri, 15 Sep 2023 11:22:18 +0200 Subject: [PATCH 5/9] Use lazy formatting in warnings Change-Id: I6e9ac8966a8f52dd572a2fbf6f79ece4c8b8e09d --- bluepyemodel/emodel_pipeline/emodel_pipeline.py | 2 +- .../evaluation/fitness_calculator_configuration.py | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/bluepyemodel/emodel_pipeline/emodel_pipeline.py b/bluepyemodel/emodel_pipeline/emodel_pipeline.py index af5f35eb..7ff2b8ed 100644 --- a/bluepyemodel/emodel_pipeline/emodel_pipeline.py +++ b/bluepyemodel/emodel_pipeline/emodel_pipeline.py @@ -109,7 +109,7 @@ def __init__( of the e-model building pipeline be based on multiprocessing. """ - # pylint: disable=too-many-arguments + # pylint: disable=too-many-arguments, unused-argument if use_ipyparallel and use_multiprocessing: raise ValueError( diff --git a/bluepyemodel/evaluation/fitness_calculator_configuration.py b/bluepyemodel/evaluation/fitness_calculator_configuration.py index a2cbf956..516bb731 100644 --- a/bluepyemodel/evaluation/fitness_calculator_configuration.py +++ b/bluepyemodel/evaluation/fitness_calculator_configuration.py @@ -19,6 +19,8 @@ import logging from copy import deepcopy +from bluepyopt.ephys.locations import EPhysLocInstantiateException + from bluepyemodel.evaluation.efeature_configuration import EFeatureConfiguration from bluepyemodel.evaluation.evaluator import LEGACY_PRE_PROTOCOLS from bluepyemodel.evaluation.evaluator import PRE_PROTOCOLS @@ -26,7 +28,6 @@ from bluepyemodel.evaluation.evaluator import seclist_to_sec from bluepyemodel.evaluation.protocol_configuration import ProtocolConfiguration from bluepyemodel.tools.utils import are_same_protocol -from bluepyopt.ephys.locations import EPhysLocInstantiateException logger = logging.getLogger(__name__) @@ -516,8 +517,8 @@ def configure_morphology_dependent_locations(self, _cell, simulator): recordings.append(_rec) except EPhysLocInstantiateException: logger.warning( - f"Could not find {location.name}, " - "ignoring recording at this location" + "Could not find %s, ignoring recording at this location", + location.name ) skipped_recordings.append(_rec["name"]) else: @@ -532,7 +533,7 @@ def configure_morphology_dependent_locations(self, _cell, simulator): for skiprec in skipped_recordings: if f"{efeature.protocol_name}.{efeature.recording_name}" == skiprec: to_remove.append(i) - logger.warning(f"removing {efeature.name}") + logger.warning("Removing %s", efeature.name) continue # if the loc of the recording is of the form axon*.v, we replace * by # all the corresponding int from the created recordings From 50777a0141082a8eed591ed6cb83d31791ac2703 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaquier=20Aur=C3=A9lien=20Tristan?= Date: Fri, 15 Sep 2023 11:31:54 +0200 Subject: [PATCH 6/9] black fix Change-Id: I2cc37a2c7951fc16518e25eb05dbda111f4fccfe --- bluepyemodel/evaluation/fitness_calculator_configuration.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bluepyemodel/evaluation/fitness_calculator_configuration.py b/bluepyemodel/evaluation/fitness_calculator_configuration.py index 516bb731..1e3f592d 100644 --- a/bluepyemodel/evaluation/fitness_calculator_configuration.py +++ b/bluepyemodel/evaluation/fitness_calculator_configuration.py @@ -518,7 +518,7 @@ def configure_morphology_dependent_locations(self, _cell, simulator): except EPhysLocInstantiateException: logger.warning( "Could not find %s, ignoring recording at this location", - location.name + location.name, ) skipped_recordings.append(_rec["name"]) else: From 813a056b8df661821144ac8f61f60d0eb4f6b0d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaquier=20Aur=C3=A9lien=20Tristan?= Date: Fri, 15 Sep 2023 12:50:03 +0200 Subject: [PATCH 7/9] no need to use temporary copy of recordings Change-Id: Ic40520e37a4d1632ca0057fa852a9ac91e71cafb --- bluepyemodel/evaluation/fitness_calculator_configuration.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bluepyemodel/evaluation/fitness_calculator_configuration.py b/bluepyemodel/evaluation/fitness_calculator_configuration.py index 1e3f592d..989fca5c 100644 --- a/bluepyemodel/evaluation/fitness_calculator_configuration.py +++ b/bluepyemodel/evaluation/fitness_calculator_configuration.py @@ -511,8 +511,7 @@ def configure_morphology_dependent_locations(self, _cell, simulator): if rec["type"] != "CompRecording": for _rec in _set_morphology_dependent_locations(rec, cell): try: - tmp_rec = deepcopy(_rec) - location = define_location(tmp_rec) + location = define_location(_rec) location.instantiate(sim=simulator, icell=cell.icell) recordings.append(_rec) except EPhysLocInstantiateException: From 86723f059fdfa6ed5385b14affe394bebbc217ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaquier=20Aur=C3=A9lien=20Tristan?= Date: Fri, 15 Sep 2023 13:48:53 +0200 Subject: [PATCH 8/9] add test for configure_morphology_dependent_locations Change-Id: I8595a3671aeaaadd464557cc0e5ea6c6d9175e0d --- .../test_fitness_calculator_configuration.py | 122 ++++++++++++++++++ 1 file changed, 122 insertions(+) diff --git a/tests/unit_tests/test_fitness_calculator_configuration.py b/tests/unit_tests/test_fitness_calculator_configuration.py index 369d255f..1f69a629 100644 --- a/tests/unit_tests/test_fitness_calculator_configuration.py +++ b/tests/unit_tests/test_fitness_calculator_configuration.py @@ -16,8 +16,15 @@ import pytest +from bluepyemodel.model import model + +from bluepyemodel.evaluation.evaluator import get_simulator from bluepyemodel.evaluation.fitness_calculator_configuration import FitnessCalculatorConfiguration +from tests.unit_tests.test_local_access_point import api_config +from tests.unit_tests.test_local_access_point import db +from tests.unit_tests.test_local_access_point import DATA + @pytest.fixture def config_dict(): @@ -94,6 +101,86 @@ def config_dict(): return config_dict +@pytest.fixture +def config_dict_bad_recordings(): + + efeatures = [ + { + "efel_feature_name": "maximum_voltage_from_voltagebase", + "protocol_name": "Step_150", + "recording_name": "soma.v", + "efel_settings": {"stim_start": 700, "stim_end": 2700}, + "mean": -83.1596, + "std": 1.0102, + "threshold_efeature_std": 0.05 + }, + { + "efel_feature_name": "maximum_voltage_from_voltagebase", + "protocol_name": "Step_150", + "recording_name": "dend100.v", + "efel_settings": {"stim_start": 700, "stim_end": 2700}, + "mean": -83.1596, + "std": 1.0102, + "threshold_efeature_std": 0.05 + }, + { + "efel_feature_name": "maximum_voltage_from_voltagebase", + "protocol_name": "Step_150", + "recording_name": "dend10000.v", + "efel_settings": {"stim_start": 700, "stim_end": 2700}, + "mean": -83.1596, + "std": 1.0102, + "threshold_efeature_std": 0.05 + }, + ] + + protocols = [ + { + "name": "Step_150", + "stimuli": [{ + "location": "soma", + "delay": 700.0, + "amp": None, + "thresh_perc": 148.7434, + "duration": 2000.0, + "totduration": 3000.0, + "holding_current": None + }], + "recordings": [ + { + "type": "CompRecording", + "name": "Step_150.soma.v", + "location": "soma", + "variable": "v" + }, + { + "type": "somadistanceapic", + "somadistance": 100, + "name": "Step_150.dend100.v", + "seclist_name": "apical", + "variable": "v" + }, + { + "type": "somadistanceapic", + "somadistance": 10000, + "name": "Step_150.dend10000.v", + "seclist_name": "apical", + "variable": "v" + } + ], + "validation": False + } + ] + + config_dict = { + "efeatures": efeatures, + "protocols": protocols, + "name_rmp_protocol": "IV_-40", + "name_rin_protocol": "IV_0", + } + + return config_dict + @pytest.fixture def config_dict_from_bpe2(): @@ -198,3 +285,38 @@ def test_init_from_bpe2(config_dict, config_dict_from_bpe2): "ohmic_input_resistance_vb_ssse" ]: assert next((f for f in config.efeatures if f.efel_feature_name == fn), False) + + +def test_configure_morphology_dependent_locations(config_dict_bad_recordings, db): + """Test configure_morphology_dependent_locations with recordings outside of cell.""" + config = FitnessCalculatorConfiguration(**config_dict_bad_recordings) + + model_configuration = db.get_model_configuration() + cell_model = model.create_cell_model( + name=db.emodel_metadata.emodel, + model_configuration=model_configuration, + morph_modifiers=None, + ) + + # don't know if I have to specify the mechanism directory here + simulator = get_simulator( + stochasticity=False, + cell_model=cell_model, + ) + + config.configure_morphology_dependent_locations(cell_model, simulator) + + assert len(config.protocols) == 1 + # one recording should have been removed + assert len(config.protocols[0].recordings) == 2 + for rec in config.protocols[0].recordings: + if rec["type"] == "somadistanceapic": + # check _set_morphology_dependent_locations section name replacement + assert rec["sec_name"] == "apic" + # check that this recording has been removed + assert rec["somadistance"] != 10000 + # one efeature should have been removed + assert len(config.efeatures) == 2 + for feat in config.efeatures: + # this efeature should have been removed + assert feat.recording_name != "dend10000.v" From b39338ebb6eb2cc6081d30d4400a575e3b4dc4ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaquier=20Aur=C3=A9lien=20Tristan?= Date: Fri, 15 Sep 2023 13:49:53 +0200 Subject: [PATCH 9/9] remove data_access_point from emodel_pipeline Change-Id: I0f8866f1de58b534194b95d7c34c294631eb424a --- bluepyemodel/emodel_pipeline/emodel_pipeline.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bluepyemodel/emodel_pipeline/emodel_pipeline.py b/bluepyemodel/emodel_pipeline/emodel_pipeline.py index 7ff2b8ed..4aa6f8f4 100644 --- a/bluepyemodel/emodel_pipeline/emodel_pipeline.py +++ b/bluepyemodel/emodel_pipeline/emodel_pipeline.py @@ -60,7 +60,6 @@ def __init__( recipes_path=None, use_ipyparallel=None, use_multiprocessing=None, - data_access_point=None, ): """Initializes the EModel_pipeline. @@ -109,7 +108,7 @@ def __init__( of the e-model building pipeline be based on multiprocessing. """ - # pylint: disable=too-many-arguments, unused-argument + # pylint: disable=too-many-arguments if use_ipyparallel and use_multiprocessing: raise ValueError(