Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[MRG] MAINT: refactor distal/proximal stuff #146

Merged
merged 18 commits into from Aug 28, 2020
Merged

[MRG] MAINT: refactor distal/proximal stuff #146

Changes from 4 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
6f71bb6
MAINT: refactor distal/proximal stuff
jasmainak Aug 23, 2020
4169927
FIX pos_src
jasmainak Aug 23, 2020
63530d6
More simplification
jasmainak Aug 23, 2020
c996f6a
MAINT: take method to base _Pyramidal class
jasmainak Aug 23, 2020
8d23a02
Forgotten refactor
jasmainak Aug 23, 2020
b5cd23d
Update hnn_core/pyramidal.py
jasmainak Aug 24, 2020
b2f2f5e
MAINT: _connect_at_loc -> _connect_feed_at_loc
jasmainak Aug 24, 2020
6cd00a8
MAINT: Seeing double?
jasmainak Aug 24, 2020
886cbfe
Flake8
jasmainak Aug 24, 2020
0029d80
MAINT: refactor parreceive and parreceive_ext in pyramidal
jasmainak Aug 25, 2020
5845d17
MAINT: refactor also basket cell
jasmainak Aug 25, 2020
4ca7f42
Addressed ryan's comment but doesn't work
jasmainak Aug 25, 2020
f5c8911
MAINT: monkeypatch Exp2Syn
jasmainak Aug 26, 2020
ece0ecb
Flake8
jasmainak Aug 26, 2020
96c266a
TST for _connect_feed_at_loc
jasmainak Aug 27, 2020
466504b
DOC update whats new
jasmainak Aug 27, 2020
52258f3
DOC: remove comment, values are in accordance to Jones, 2009
jasmainak Aug 27, 2020
1c71219
Define proximal and distal separately for the two classes
jasmainak Aug 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
282 changes: 102 additions & 180 deletions hnn_core/pyramidal.py
View file
Open in desktop
Expand Up @@ -219,6 +219,43 @@ def _synapse_create(self, p_syn):
self.dends['apical_tuft'](0.5), **p_syn['gabaa'])


def _connect_at_loc(self, loc, receptor, gid_src, nc_dict, nc_list):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this function only be used for connecting feeds (via artificial cells) to the given Pyr instance? If so, we may want to reflect the specificity of this method in the name and description.

Also, am I correct in assuming that the reason for having different methods that connect real cells -> real cells vs. artificial cells -> real cells is that the latter connects a point neuron with no physical morphology to a specific synapse on a dendrite?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me get back to you on that in a day or two. I need to mess around with the code to figure out what's possible and what's not. I think we can definitely extend to Basket cells. But I'd leave the refactoring with _connect method to another PR.

So something like _connect_feed_at_loc might do ...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_connect_feed_at_loc sounds good to me! We can always rename it in a later PR while refactoring the cell classes.

"""Get list of proximal/distal postsynaptic junctions.

Parameters
----------
loc : str
Either 'proximal' or 'distal'
receptor : str
'nmda', 'ampa' etc.
gid_src : int
The Cell ID of the source.
jasmainak marked this conversation as resolved.
Show resolved Hide resolved
nc_dict : dict.
The connection parameters. Usually contains the keys
pos_src, A_weight, A_delay, lamtha.
nc_list : list of NetCon
Different intput types are appended to different lists
of NetCon objects. The created network connections will
be appended to this list.
"""
if loc == 'proximal':
dends = ['apicaloblique', 'basal2', 'basal3']
elif loc == 'distal':
dends = ['apicaltuft']
else:
raise ValueError('loc must be one of proximal or distal')

postsyns = list()
for dend in dends:
postsyns.append(getattr(self, f'{dend}_{receptor}'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
postsyns = list()
for dend in dends:
postsyns.append(getattr(self, f'{dend}_{receptor}'))
[self.synapse[syn_key] for syn_key in self.synapses.keys() if receptor in syn_key]


for postsyn in postsyns:
nc = self.parconnect_from_src(gid_src, nc_dict, postsyn)
nc_list.append(nc)

return postsyns
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So the main point here is to simplify the parreceive() and parreceive_ext() methods correct? If so, I definitely like how this PR focuses on consolidating redundant code.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, but also to make the code a bit more readable. Not just concise but more readable. We'll probably need to do a couple more iterations / PRs to get it absolutely on point.



class L2Pyr(Pyr):
"""Layer 2 pyramidal cell class.

Expand Down Expand Up @@ -441,18 +478,10 @@ def parreceive(self, gid, gid_dict, pos_dict, p_ext):
'type_src': 'ext'
}

# Proximal feed AMPA synapses
if p_src['loc'] == 'proximal':
self.ncfrom_common.append(self.parconnect_from_src(
gid_src, nc_dict_ampa, self.basal2_ampa))
self.ncfrom_common.append(self.parconnect_from_src(
gid_src, nc_dict_ampa, self.basal3_ampa))
self.ncfrom_common.append(self.parconnect_from_src(
gid_src, nc_dict_ampa, self.apicaloblique_ampa))
# Distal feed AMPA synapses
elif p_src['loc'] == 'distal':
self.ncfrom_common.append(self.parconnect_from_src(
gid_src, nc_dict_ampa, self.apicaltuft_ampa))
self._connect_at_loc(
loc=p_src['loc'], receptor='ampa',
gid_src=gid_src, nc_dict=nc_dict_ampa,
nc_list=self.ncfrom_common)

# Check is NMDA params defined in p_src
if 'L2Pyr_nmda' in p_src.keys():
Expand All @@ -465,18 +494,10 @@ def parreceive(self, gid, gid_dict, pos_dict, p_ext):
'type_src': 'ext'
}

# Proximal feed NMDA synapses
if p_src['loc'] == 'proximal':
self.ncfrom_common.append(self.parconnect_from_src(
gid_src, nc_dict_nmda, self.basal2_nmda))
self.ncfrom_common.append(self.parconnect_from_src(
gid_src, nc_dict_nmda, self.basal3_nmda))
self.ncfrom_common.append(self.parconnect_from_src(
gid_src, nc_dict_nmda, self.apicaloblique_nmda))
# Distal feed NMDA synapses
elif p_src['loc'] == 'distal':
self.ncfrom_common.append(self.parconnect_from_src(
gid_src, nc_dict_nmda, self.apicaltuft_nmda))
self._connect_at_loc(
loc=p_src['loc'], receptor='nmda',
gid_src=gid_src, nc_dict=nc_dict_nmda,
nc_list=self.ncfrom_common)

# one parreceive function to handle all types of external parreceives
# types must be defined explicitly here
Expand All @@ -487,8 +508,9 @@ def parreceive_ext(self, type, gid, gid_dict, pos_dict, p_ext):
if self.celltype in p_ext.keys():
gid_ev = gid + gid_dict[type][0]

nc_dict = dict()
# separate dictionaries for ampa and nmda evoked inputs
nc_dict_ampa = {
nc_dict['ampa'] = {
'pos_src': pos_dict[type][gid],
# index 0 for ampa weight
'A_weight': p_ext[self.celltype][0],
Expand All @@ -498,7 +520,7 @@ def parreceive_ext(self, type, gid, gid_dict, pos_dict, p_ext):
'type_src': type
}

nc_dict_nmda = {
nc_dict['nmda'] = {
'pos_src': pos_dict[type][gid],
# index 1 for nmda weight
'A_weight': p_ext[self.celltype][1],
Expand All @@ -508,28 +530,13 @@ def parreceive_ext(self, type, gid, gid_dict, pos_dict, p_ext):
'type_src': type
}

if p_ext['loc'] == 'proximal':
self.ncfrom_ev.append(self.parconnect_from_src(
gid_ev, nc_dict_ampa, self.basal2_ampa))
self.ncfrom_ev.append(self.parconnect_from_src(
gid_ev, nc_dict_ampa, self.basal3_ampa))
self.ncfrom_ev.append(self.parconnect_from_src(
gid_ev, nc_dict_ampa, self.apicaloblique_ampa))

# NEW: note that default/original is 0 nmda weight
# for these proximal dends
self.ncfrom_ev.append(self.parconnect_from_src(
gid_ev, nc_dict_nmda, self.basal2_nmda))
self.ncfrom_ev.append(self.parconnect_from_src(
gid_ev, nc_dict_nmda, self.basal3_nmda))
self.ncfrom_ev.append(self.parconnect_from_src(
gid_ev, nc_dict_nmda, self.apicaloblique_nmda))

elif p_ext['loc'] == 'distal':
self.ncfrom_ev.append(self.parconnect_from_src(
gid_ev, nc_dict_ampa, self.apicaltuft_ampa))
self.ncfrom_ev.append(self.parconnect_from_src(
gid_ev, nc_dict_nmda, self.apicaltuft_nmda))
# NEW: note that default/original is 0 nmda weight
# for the proximal dends
for receptor in ['ampa', 'nmda']:
self._connect_at_loc(
loc=p_ext['loc'], receptor=receptor,
gid_src=gid_ev, nc_dict=nc_dict[receptor],
nc_list=self.ncfrom_common)

elif type == 'extgauss':
# gid is this cell's gid
Expand All @@ -554,12 +561,10 @@ def parreceive_ext(self, type, gid, gid_dict, pos_dict, p_ext):
'type_src': type
}

self.ncfrom_extgauss.append(self.parconnect_from_src(
gid_extgauss, nc_dict, self.basal2_ampa))
self.ncfrom_extgauss.append(self.parconnect_from_src(
gid_extgauss, nc_dict, self.basal3_ampa))
self.ncfrom_extgauss.append(self.parconnect_from_src(
gid_extgauss, nc_dict, self.apicaloblique_ampa))
self._connect_at_loc(
loc='proximal', receptor='ampa',
gid_src=gid_extgauss, nc_dict=nc_dict,
nc_list=self.ncfrom_extgauss)

elif type == 'extpois':
if self.celltype in p_ext.keys():
Expand All @@ -575,12 +580,10 @@ def parreceive_ext(self, type, gid, gid_dict, pos_dict, p_ext):
'type_src': type
}

self.ncfrom_extpois.append(self.parconnect_from_src(
gid_extpois, nc_dict, self.basal2_ampa))
self.ncfrom_extpois.append(self.parconnect_from_src(
gid_extpois, nc_dict, self.basal3_ampa))
self.ncfrom_extpois.append(self.parconnect_from_src(
gid_extpois, nc_dict, self.apicaloblique_ampa))
self._connect_at_loc(
loc='proximal', receptor='ampa',
gid_src=gid_extpois, nc_dict=nc_dict,
nc_list=self.ncfrom_extpois)

if p_ext[self.celltype][1] > 0.0:
# index 1 for nmda weight
Expand Down Expand Up @@ -877,65 +880,21 @@ def parconnect(self, gid, gid_dict, pos_dict, p):
def parreceive(self, gid, gid_dict, pos_dict, p_ext):
for gid_src, p_src, pos in zip(gid_dict['common'],
p_ext, pos_dict['common']):
# Check if AMPA params defined in p_src
if 'L5Pyr_ampa' in p_src.keys():
nc_dict_ampa = {
'pos_src': pos,
'A_weight': p_src['L5Pyr_ampa'][0],
'A_delay': p_src['L5Pyr_ampa'][1],
'lamtha': p_src['lamtha'],
'threshold': p_src['threshold'],
'type_src': 'ext'
}

# Proximal feed AMPA synapses
if p_src['loc'] == 'proximal':
# basal2_ampa, basal3_ampa, apicaloblique_ampa
self.ncfrom_common.append(
self.parconnect_from_src(gid_src, nc_dict_ampa,
self.basal2_ampa))
self.ncfrom_common.append(
self.parconnect_from_src(gid_src, nc_dict_ampa,
self.basal3_ampa))
self.ncfrom_common.append(
self.parconnect_from_src(gid_src, nc_dict_ampa,
self.apicaloblique_ampa))
# Distal feed AMPA synsapes
elif p_src['loc'] == 'distal':
# apical tuft
self.ncfrom_common.append(
self.parconnect_from_src(gid_src, nc_dict_ampa,
self.apicaltuft_ampa))

# Check if NMDA params defined in p_src
if 'L5Pyr_nmda' in p_src.keys():
nc_dict_nmda = {
'pos_src': pos,
'A_weight': p_src['L5Pyr_nmda'][0],
'A_delay': p_src['L5Pyr_nmda'][1],
'lamtha': p_src['lamtha'],
'threshold': p_src['threshold'],
'type_src': 'ext'
}

# Proximal feed NMDA synapses
if p_src['loc'] == 'proximal':
# basal2_nmda, basal3_nmda, apicaloblique_nmda
self.ncfrom_common.append(
self.parconnect_from_src(
gid_src, nc_dict_nmda, self.basal2_nmda))
self.ncfrom_common.append(
self.parconnect_from_src(
gid_src, nc_dict_nmda, self.basal3_nmda))
self.ncfrom_common.append(
self.parconnect_from_src(
gid_src, nc_dict_nmda, self.apicaloblique_nmda))
# Distal feed NMDA synsapes
elif p_src['loc'] == 'distal':
# apical tuft
self.ncfrom_common.append(
self.parconnect_from_src(
gid_src, nc_dict_nmda, self.apicaltuft_nmda))
for receptor in ['ampa', 'nmda']:
# Check if AMPA params defined in p_src
if f'L5Pyr_{receptor}' in p_src.keys():
nc_dict = {
'pos_src': pos,
'A_weight': p_src[f'L5Pyr_{receptor}'][0],
'A_delay': p_src[f'L5Pyr_{receptor}'][1],
'lamtha': p_src['lamtha'],
'threshold': p_src['threshold'],
'type_src': 'ext'
}

self._connect_at_loc(loc=p_src['loc'], receptor=receptor,
gid_src=gid_src, nc_dict=nc_dict,
nc_list=self.ncfrom_common)

# one parreceive function to handle all types of external parreceives
# types must be defined explicitly here
Expand All @@ -944,7 +903,8 @@ def parreceive_ext(self, type, gid, gid_dict, pos_dict, p_ext):
if self.celltype in p_ext.keys():
gid_ev = gid + gid_dict[type][0]

nc_dict_ampa = {
nc_dict = dict()
nc_dict['ampa'] = {
'pos_src': pos_dict[type][gid],
# index 0 for ampa weight
'A_weight': p_ext[self.celltype][0],
Expand All @@ -954,47 +914,20 @@ def parreceive_ext(self, type, gid, gid_dict, pos_dict, p_ext):
'type_src': type
}

nc_dict_nmda = {
nc_dict['nmda'] = {
'pos_src': pos_dict[type][gid],
# index 1 for nmda weight
'A_weight': p_ext[self.celltype][1],
'A_delay': p_ext[self.celltype][2], # index 2 for delay
'lamtha': p_ext['lamtha_space'],
'threshold': p_ext['threshold'],
'type_src': type
}

if p_ext['loc'] == 'proximal':
self.ncfrom_ev.append(
self.parconnect_from_src(
gid_ev, nc_dict_ampa, self.basal2_ampa))
self.ncfrom_ev.append(
self.parconnect_from_src(
gid_ev, nc_dict_ampa, self.basal3_ampa))
self.ncfrom_ev.append(
self.parconnect_from_src(
gid_ev, nc_dict_ampa, self.apicaloblique_ampa))

# NEW: note that default/original is 0 nmda weight
# for these proximal dends
self.ncfrom_ev.append(
self.parconnect_from_src(
gid_ev, nc_dict_nmda, self.basal2_nmda))
self.ncfrom_ev.append(
self.parconnect_from_src(
gid_ev, nc_dict_nmda, self.basal3_nmda))
self.ncfrom_ev.append(
self.parconnect_from_src(
gid_ev, nc_dict_nmda, self.apicaloblique_nmda))

elif p_ext['loc'] == 'distal':
# apical tuft
self.ncfrom_ev.append(
self.parconnect_from_src(
gid_ev, nc_dict_ampa, self.apicaltuft_ampa))
self.ncfrom_ev.append(
self.parconnect_from_src(
gid_ev, nc_dict_nmda, self.apicaltuft_nmda))
for receptor in ['ampa', 'nmda']:
self._connect_at_loc(loc=p_ext['loc'], receptor=receptor,
gid_src=gid_ev,
nc_dict=nc_dict[receptor],
nc_list=self.ncfrom_ev)

elif type == 'extgauss':
# gid is this cell's gid
Expand All @@ -1021,15 +954,11 @@ def parreceive_ext(self, type, gid, gid_dict, pos_dict, p_ext):
'type_src': type
}

self.ncfrom_extgauss.append(
self.parconnect_from_src(
gid_extgauss, nc_dict, self.basal2_ampa))
self.ncfrom_extgauss.append(
self.parconnect_from_src(
gid_extgauss, nc_dict, self.basal3_ampa))
self.ncfrom_extgauss.append(
self.parconnect_from_src(
gid_extgauss, nc_dict, self.apicaloblique_ampa))
self._connect_at_loc(
loc='proximal', receptor='ampa',
gid_src=gid_extgauss,
nc_dict=nc_dict,
nc_list=self.ncfrom_extgauss)

elif type == 'extpois':
if self.celltype in p_ext.keys():
Expand All @@ -1046,25 +975,18 @@ def parreceive_ext(self, type, gid, gid_dict, pos_dict, p_ext):
'type_src': type
}

self.ncfrom_extpois.append(
self.parconnect_from_src(
gid_extpois, nc_dict, self.basal2_ampa))
self.ncfrom_extpois.append(
self.parconnect_from_src(
gid_extpois, nc_dict, self.basal3_ampa))
self.ncfrom_extpois.append(
self.parconnect_from_src(
gid_extpois, nc_dict, self.apicaloblique_ampa))
self._connect_at_loc(
loc='proximal', receptor='ampa',
gid_src=gid_extpois,
nc_dict=nc_dict,
nc_list=self.ncfrom_extpois)

if p_ext[self.celltype][1] > 0.0:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this if condition is awkward but I didn't want to break things, so left it there

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed. We'll need to fix this when we remove the loops that iterate through the different feed types from the cell classes. I'm not sure why this was put here in the first place (maybe network build time?) since an nmda receptor conductance of of zero shouldn't have any effect on the simulation.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this related to your PRs about turning off inputs based on the weights? or was that different?

maybe network build time?

I hope not. "Premature optimization is the root of all evil." (Knuth) Another related aphorism is make it work, make it nice, make it fast

Network build times are really small compared to simulation time and developer time (most important in my opinion). Maybe for GUI application, some of these times matter but I'd rather refactor first, then profile and worry about timing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

isn't this related to your PRs about turning off inputs based on the weights? or was that different?

It's related in principle, but I was unaware that there was a check for synaptic weight >0 within a cell class method. There is probably more to the story that we are unaware of, but we'll need to handle this more consistently in the future.

# index 1 for nmda weight
nc_dict['A_weight'] = p_ext[self.celltype][1]
self.ncfrom_extpois.append(
self.parconnect_from_src(
gid_extpois, nc_dict, self.basal2_nmda))
self.ncfrom_extpois.append(
self.parconnect_from_src(
gid_extpois, nc_dict, self.basal3_nmda))
self.ncfrom_extpois.append(
self.parconnect_from_src(
gid_extpois, nc_dict, self.apicaloblique_nmda))

self._connect_at_loc(
loc='proximal', receptor='nmda',
gid_src=gid_extpois,
nc_dict=nc_dict,
nc_list=self.ncfrom_extpois)