[BBPBGLIB-1059] Integrate Oren's gap junction compensation procedure into neurodamus #195

WeinaJi · 2024-09-04T13:38:20Z

Context

Integrate Oren's compensation script for gap junction synapses into the neurodamus main flow. Uses are expected to use predefined parameters in the SONATA simulation config files to configure the correction steps and values. In this PR, those parameters are stored in the experimental "beta_features" section of the SONATA config files. The corresponding formal SONATA parameters are to be determined in the SONATA specifications, the libsonata parser, and the future PR in neurodamus.

Scope

At the finalize step for GapJunctionManager, if users set the right gapjunction_target_population, the user corrections defined in the new file gj_user_functions.py will be triggered. This file is implemented based on Oren's script which was used for Elisabetta's publication (/gpfs/bbp.cscs.ch/project/proj55/software/gap_junctions/pylibs/manager_python.py). It provides these corrections to be enabled by users via the SONATA simulation config file:

Set deterministic = 1 for StochKv mechanisms set in the simulation config file "determanisitc_stoch"
Update the conductance value of gap synapses to the value set in the simulation config file "gjc"
Remove cell channel mechanisms set by "remove_channels"
Update segment g_pas to fit the gjc with values set by the calibration file given in the simulation config file "load_g_pas_file"
Add current clamps to certain cells where the gids and the amplitudes are read from the file given in the simulation config "manual_MEComboInfo_file"
Add SEClamps to the certain cells where the gids and the amplitudes are read from the file given in the simulation config "vc_amp" and meanwhile save the SEClamp Data

Those SONATA parameters are chosen based on the setting in Elisabetta's publication /gpfs/bbp.cscs.ch/project/proj55/iavarone/releases/simulations/spontaneous_evoked/CorrectCTFiberAll_Backgroundwith_afferents_MLNoise25_CTNoise4/MLStim4/MLNoise0/CTNoise0/settings.p for which corrections 1,2,4,5 were used.

Testing

New test test_v5_gap_junction_corrections in test/scientific/test_simulation.py

Review

PR description is complete
Coding style (imports, function length, New functions, classes or files) are good
Unit/Scientific test added
Updated Readme, in-code, developer documentation

pramodk · 2024-09-09T10:40:54Z

tests/scientific/test_simulation.py

+        "load_g_pas_file": "/gpfs/bbp.cscs.ch/project/proj55/amsalem/gap_junctions/circ19_11_2019_gjs_19_20_20/rm_correction/Circ_mc2_Rt_Remove_ch_all_Det_stoch_True_Dis_holding_False_Correc_type_impedance_tool_Cm0p01_Num_iter_10/num_0/g_pas_passive.hdf5", # noqa
+        "manual_MEComboInfo_file": "/gpfs/bbp.cscs.ch/project/proj55/amsalem/gap_junctions/circ19_11_2019_gjs_19_20_20_new_holding/find_holding_current/vc_ampFile/Circ_mc2_Rt_Remove_ch_none_Det_stoch_True_Dis_holding_False_gjc0p2_Change_mecomb_False_manual_MEComboInfoFile_False_Load_g_pas_True_Correc_iter_loadlast/num_0/holding_per_gid.hdf5" # noqa
+    }


@WeinaJi : Just a comment: at a minimum, we should move this to proj12?

In general, as we want to move the CI out of gitlab / BB5 very soon, it would be good to have tests that are not dependent on BB5/GitLab. Because very soon, we need to work on removing gitlab dependency.

this was my comment also, would be nice to avoid depending on bb5 for the tests...

Yes sure, for this test I will create small, local and dummy calibration files to read as I am not test the final results.

jorblancoa · 2024-09-09T15:57:05Z

General comment:
Do we want to merge those in beta_features? or shall we wait to define them in the sonata-extension and then implement the necessary changes in libsonata first?

orena1 · 2024-09-10T01:48:37Z

Hi @WeinaJi I think it looks good, but is there a way to test it?

orena1 · 2024-09-10T01:50:40Z

Very cool, a bit off-topic, but there is also the code that generates the GJs and use them. I'll try to get it on github and we can refer to it.

WeinaJi · 2024-09-10T14:13:20Z

General comment: Do we want to merge those in beta_features? or shall we wait to define them in the sonata-extension and then implement the necessary changes in libsonata first?

Thinking about the time left, I am inclined to merge them in beta_features so @PolinaL can use in her thalamus simulation asap. This way we don't need the approval for the SONATA specs and the libsonata API.
For the SONATA specs, we need to define proper naming for all required parameters, and possibly store them in a proper section. Maybe we could brainstorm with scientists if time permits, @jamesgking ?

WeinaJi · 2024-09-10T14:22:31Z

Hi @WeinaJi I think it looks good, but is there a way to test it?

Hello @orena1 , can you access our bb5 cluster? If yes, you can clone this branch into your directory, load the thalamus module, and overwrite the neurodamus component with your local copy. Something like:

git clone [email protected]:BlueBrain/neurodamus.git -b weji/oren
module load unstable neurodamus-thalamus
export PYTHONPATH=$(pwd)/neurodamus:$PYTHONPATH
srun dplace special -mpi -python $NEURODAMUS_PYTHON/init.py --configFile=simulation_config.json

The config file simulation_config.json should contain the beta_features section like

{
"beta_features" :{
        "gapjunction_target_population": "thalamus_neurons",
        "determanisitc_stoch": true,
        "procedure_type": "validation_sim",
        "gjc": 0.2,
        "load_g_pas_file": "/gpfs/bbp.cscs.ch/project/proj55/amsalem/gap_junctions/circ19_11_2019_gjs_19_20_20/rm_correction/Circ_mc2_Rt_Remove_ch_all_Det_stoch_True_Dis_holding_False_Correc_type_impedance_tool_Cm0p01_Num_iter_10/num_0/g_pas_passive.hdf5",
        "manual_MEComboInfo_file": "/gpfs/bbp.cscs.ch/project/proj55/amsalem/gap_junctions/circ19_11_2019_gjs_19_20_20_new_holding/find_holding_current/vc_ampFile/Circ_mc2_Rt_Remove_ch_none_Det_stoch_True_Dis_holding_False_gjc0p2_Change_mecomb_False_manual_MEComboInfoFile_False_Load_g_pas_True_Correc_iter_loadlast/num_0/holding_per_gid.hdf5"
    },
}

An example from my test config file /gpfs/bbp.cscs.ch/home/weji/JIRA/thalamus_elsabetta/0/simulation_config.json

Also cc @PolinaL

WeinaJi · 2024-09-10T14:34:26Z

Very cool, a bit off-topic, but there is also the code that generates the GJs and use them. I'll try to get it on github and we can refer to it.

That will be great. Please let me know if you would like to push something in this repo. I can give you the write permission.

In fact, I have another version of your script which includes these commented code:

# if load_g_pas_correction_file:
#     logging.info("Loading g_pas_correction_file")
#     exec(open('/gpfs/bbp.cscs.ch/home/amsalem/Dropbox/Blue_Brain/Experiments_Reproduction/Coupling_Coefficient/pylibs/gap_junction_conductance_search_damus.py').read())

Is it still needed? I can't find the script gap_junction_conductance_search_damus.py any more.

neurodamus/gj_user_corrections.py

ferdonline

Some nitpicks, but it might be worth to a wider round of checks for naming and other small inconsistencies. vars starting in upcase remember class variables or constants

ferdonline · 2024-10-17T11:21:14Z

neurodamus/gap_junction.py

@@ -66,6 +69,8 @@ def __init__(self, gj_conf, target_manager, cell_manager, src_cell_manager=None,
        super().__init__(gj_conf, target_manager, cell_manager, src_cell_manager, **kw)
        self._src_target_filter = target_manager.get_target(cell_manager.circuit_target,
                                                            src_cell_manager.population_name)
+        self.holding_ic_per_gid = None
+        self.SEClamp_current_per_gid = None


totally nitpick, sorry, but would it be too bad to adhere to normal lowercase?

Suggested change

self.SEClamp_current_per_gid = None

self.seclamp_current_per_gid = None

ferdonline · 2024-10-17T11:22:37Z

neurodamus/gap_junction.py

+        if self.cell_manager.population_name == \
+                SimConfig.beta_features.get("gapjunction_target_population"):


Just another style nitpick 🙈

Suggested change

if self.cell_manager.population_name == \

SimConfig.beta_features.get("gapjunction_target_population"):

gj_target_pop = SimConfig.beta_features.get("gapjunction_target_population")

if self.cell_manager.population_name == gj_target_pop:

Thanks, done!

ferdonline · 2024-10-17T11:28:35Z

neurodamus/gj_user_corrections.py

+    Mechanisims = []
+    if remove_channels == 'all':
+        Mechanisims = non_stochastic_mechs + stochastic_mechs
+    if remove_channels == 'only_stoch':
+        Mechanisims = stochastic_mechs
+    if remove_channels == 'only_non_stoch':
+        Mechanisims = non_stochastic_mechs
+    if remove_channels:
+        logging.info("Removing channels type = " + remove_channels)
+        _perform_remove_channels(node_manager, Mechanisims)


Unnecessary checks, var naming typos, missing warning

Suggested change

Mechanisims = []

if remove_channels == 'all':

Mechanisims = non_stochastic_mechs + stochastic_mechs

if remove_channels == 'only_stoch':

Mechanisims = stochastic_mechs

if remove_channels == 'only_non_stoch':

Mechanisims = non_stochastic_mechs

if remove_channels:

logging.info("Removing channels type = " + remove_channels)

_perform_remove_channels(node_manager, Mechanisims)

if remove_channels:

if remove_channels == 'all':

rm_mechanisms = non_stochastic_mechs + stochastic_mechs

elif remove_channels == 'only_stoch':

rm_mechanisms = stochastic_mechs

elif remove_channels == 'only_non_stoch':

rm_mechanisms = non_stochastic_mechs

else:

logging.warning("Unknown GJ remove_channels setting: %s", remove_channels)

rm_mechanisms = []

logging.info("Removing channels type = " + remove_channels)

_perform_remove_channels(node_manager, rm_mechanisms)

bbpbuildbot · 2024-10-21T14:49:33Z

Logfiles from GitLab pipeline #236165 (:white_check_mark:) have been uploaded here!

Status and direct links:

WeinaJi · 2024-10-22T16:06:27Z

Very cool, a bit off-topic, but there is also the code that generates the GJs and use them. I'll try to get it on github and we can refer to it.

Hello @orena1, have you managed to test this branch on bb5? Or would you like to add your code in it?
As BBP closure is approaching, I would like to merge this feature into the main branch although it is still in "beta_futures". In this way, @PolinaL could check it out in another computing cluster after bb5 to continue her simulation.

This comment has been minimized.

Sign in to view

WeinaJi force-pushed the weji/oren branch from 992115e to 3b1d9e8 Compare September 6, 2024 10:20

This comment has been minimized.

Sign in to view

WeinaJi changed the title ~~Integrate gap junction compensation steps from Oren~~ [BBPBGLIB-1059] Integrate Oren's gap junction compensation procedure into neurodamus Sep 6, 2024

This comment has been minimized.

Sign in to view

WeinaJi marked this pull request as ready for review September 9, 2024 08:14

This comment has been minimized.

Sign in to view

pramodk reviewed Sep 9, 2024

View reviewed changes

ferdonline previously approved these changes Sep 17, 2024

View reviewed changes

neurodamus/gj_user_corrections.py Outdated Show resolved Hide resolved

neurodamus/gj_user_corrections.py Show resolved Hide resolved

jorblancoa reviewed Sep 19, 2024

View reviewed changes

neurodamus/gj_user_corrections.py Outdated Show resolved Hide resolved

WeinaJi dismissed ferdonline’s stale review via 980be30 September 23, 2024 10:25

WeinaJi force-pushed the weji/oren branch 2 times, most recently from a395192 to 94aad9e Compare October 14, 2024 12:25

This comment has been minimized.

Sign in to view

jorblancoa previously approved these changes Oct 16, 2024

View reviewed changes

ferdonline reviewed Oct 17, 2024

View reviewed changes

WeinaJi added 6 commits October 21, 2024 11:11

integrate oren's script

872c77f

register correction vector into gapjunctionconnectionmanager

715fe2d

move correct to finalize

fd6548a

read settings from beta_features

51e772f

Add unit test

5b2e26e

correct gjc var

7df4476

WeinaJi added 3 commits October 21, 2024 11:11

Add authors

ab4b9d9

use local dummpy calibration files

07bcbcf

review comments: error handling

c3ea6a7

WeinaJi force-pushed the weji/oren branch from 94aad9e to c3ea6a7 Compare October 21, 2024 09:11

This comment has been minimized.

Sign in to view

apply review comments

8535770

WeinaJi dismissed jorblancoa’s stale review via 8535770 October 21, 2024 14:07

WeinaJi linked an issue Oct 22, 2024 that may be closed by this pull request

[BBPBGLIB-1059] Implement conductance compensation for incorporating gap junctions into the thalamic SONATA circuit #203

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BBPBGLIB-1059] Integrate Oren's gap junction compensation procedure into neurodamus #195

[BBPBGLIB-1059] Integrate Oren's gap junction compensation procedure into neurodamus #195

WeinaJi commented Sep 4, 2024 •

edited

Loading

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

pramodk Sep 9, 2024

jorblancoa Sep 9, 2024

WeinaJi Sep 10, 2024

jorblancoa commented Sep 9, 2024

orena1 commented Sep 10, 2024

orena1 commented Sep 10, 2024

WeinaJi commented Sep 10, 2024

WeinaJi commented Sep 10, 2024 •

edited

Loading

WeinaJi commented Sep 10, 2024

This comment has been minimized.

This comment has been minimized.

ferdonline left a comment

ferdonline Oct 17, 2024

WeinaJi Oct 22, 2024

ferdonline Oct 17, 2024

WeinaJi Oct 22, 2024

ferdonline Oct 17, 2024

WeinaJi Oct 22, 2024

This comment has been minimized.

bbpbuildbot commented Oct 21, 2024

WeinaJi commented Oct 22, 2024

	self.SEClamp_current_per_gid = None
	self.seclamp_current_per_gid = None

		if self.cell_manager.population_name == \
		SimConfig.beta_features.get("gapjunction_target_population"):

[BBPBGLIB-1059] Integrate Oren's gap junction compensation procedure into neurodamus #195

Are you sure you want to change the base?

[BBPBGLIB-1059] Integrate Oren's gap junction compensation procedure into neurodamus #195

Conversation

WeinaJi commented Sep 4, 2024 • edited Loading

Context

Scope

Testing

Review

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jorblancoa commented Sep 9, 2024

orena1 commented Sep 10, 2024

orena1 commented Sep 10, 2024

WeinaJi commented Sep 10, 2024

WeinaJi commented Sep 10, 2024 • edited Loading

WeinaJi commented Sep 10, 2024

This comment has been minimized.

This comment has been minimized.

ferdonline left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

bbpbuildbot commented Oct 21, 2024

WeinaJi commented Oct 22, 2024

WeinaJi commented Sep 4, 2024 •

edited

Loading

WeinaJi commented Sep 10, 2024 •

edited

Loading