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.

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

[MRG+1] unify cell connectivity and drive connectivity #369

Merged
merged 22 commits into from
Aug 18, 2021
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
2790911
WIP: unify cell connectivity and drive connectivity
jasmainak Jun 18, 2021
829d144
fix src and target gids lists that get entered into add_connection
rythorpe Jul 28, 2021
3c68bcf
instantiate drives according to drive drive cell from gid_ranges
rythorpe Jul 29, 2021
8cf01d5
remove refs to drive['conn'] in network_builder
rythorpe Jul 29, 2021
48a1c07
update drive and network tests
rythorpe Jul 29, 2021
87e0512
legacy mode: set previously unspecified weight and delay parameters
rythorpe Jul 30, 2021
1c1b118
restructure weight and delay dicts in _attach_drive()
rythorpe Aug 12, 2021
50ea4bb
raise error when drives are added without specifying any synaptic wei…
rythorpe Aug 12, 2021
fc98b8c
update test_drives.py
rythorpe Aug 12, 2021
ce3e83e
fix drive weight_by_type dict
rythorpe Aug 13, 2021
e134695
use gid_pairs when finding target gids in _gid_assign
rythorpe Aug 13, 2021
7a2b11f
rename _get_target_population_properties() to _get_target_properties()
rythorpe Aug 13, 2021
7364f9d
add drive tests for _get_target_properties()
rythorpe Aug 13, 2021
997ff45
Better pick_connection tests
ntolley Aug 13, 2021
7d83505
clean up pick_connection test
rythorpe Aug 16, 2021
0810052
address comments
rythorpe Aug 16, 2021
959a181
add test for when pick_connection returns empty set
rythorpe Aug 16, 2021
2b12102
flake8
rythorpe Aug 16, 2021
e5e2a9e
allow passing drive name to add_connection() src_gids arg
rythorpe Aug 16, 2021
bc48a8b
whats_new.rst
rythorpe Aug 16, 2021
3663826
whats_new comments
rythorpe Aug 17, 2021
22f5906
Update hnn_core/tests/test_network.py
rythorpe Aug 17, 2021
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
4 changes: 4 additions & 0 deletions doc/whats_new.rst
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ Changelog

- Add :func:`~hnn_core.Network.pick_connection` to query the indices of specific connections in :attr:`~hnn_core.Network.connectivity`, by `Nick Tolley`_ in `#367 <https://github.com/jonescompneurolab/hnn-core/pull/367>`_

- Unify the codepath for instantiating drive and local network connectivity, by `Ryan Thorpe`_, `Mainak Jas`_, and `Nick Tolley`_ in `#369 <https://github.com/jonescompneurolab/hnn-core/pull/369>`_
Copy link
Collaborator Author

@jasmainak jasmainak Aug 17, 2021

Choose a reason for hiding this comment

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

You should think of "what affects the user" when updating this. User does not care about code path. They care about:

  • change in behavior (e.g., with 0 weights in param files, you now have empty connections?). That's a bug fix.
  • change in net.connectivity. It has more elements now and probably not in the same order. If someone hard coded net.connectivity[32] it would now break. So that's an API change.
  • change in drive['conns']. It no longer exists. If someone relied on it in their code, it would break now

Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think of 3663826 @jasmainak?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Looks good!


Bug
~~~

Expand Down Expand Up @@ -81,6 +83,8 @@ API

- `~hnn_core.CellResponse.reset` method is not supported any more, by `Mainak Jas`_ in `#397 <https://github.com/jonescompneurolab/hnn-core/pull/397>`_

- Target cell types and their connections are created for each drive according to the synaptic weight and delay dictionaries assigned in `Network.add_xxx_drive()`, by `Ryan Thorpe`_ in `#369 <https://github.com/jonescompneurolab/hnn-core/pull/369>`_

.. _0.1:

0.1
Expand Down
43 changes: 40 additions & 3 deletions hnn_core/drives.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,51 @@
_extract_drive_specs_from_hnn_params)


def _get_target_populations(weights_ampa, weights_nmda):
def _get_target_properties(weights_ampa, weights_nmda, synaptic_delays,
location):
"""Retrieve drive properties associated with each target cell type

Note that target cell types of a drive are inferred from the synaptic
weight and delay parameters keys defined by the user.
"""

# allow passing weights as None, but make iterable here
if weights_ampa is None:
weights_ampa = dict()
if weights_nmda is None:
weights_nmda = dict()
target_populations = (set(weights_ampa.keys()) | set(weights_nmda.keys()))
return target_populations, weights_ampa, weights_nmda

weights_by_type = {cell_type: dict() for cell_type in
(set(weights_ampa.keys()) | set(weights_ampa.keys()))}
for cell_type in weights_ampa:
weights_by_type[cell_type].update({'ampa': weights_ampa[cell_type]})
for cell_type in weights_nmda:
weights_by_type[cell_type].update({'nmda': weights_nmda[cell_type]})

target_populations = set(weights_by_type)
if not target_populations:
raise ValueError('No target cell types have been given a synaptic '
'weight for this drive.')
# Distal drives should not target L5 basket cells according to the
# canonical Jones model
if location == 'distal' and 'L5_basket' in target_populations:
raise ValueError('When adding a distal drive, synaptic weight cannot '
'be defined for the L5_basket cell type as this '
'connection does not exist.')
ntolley marked this conversation as resolved.
Show resolved Hide resolved

if isinstance(synaptic_delays, float):
delays_by_type = {cell_type: synaptic_delays for cell_type in
target_populations}
else:
delays_by_type = synaptic_delays.copy()

if set(delays_by_type.keys()) != target_populations:
raise ValueError('synaptic_delays is either a common float or needs '
'to be specified as a dict for each of the cell '
'types defined in weights_ampa and weights_nmda '
f'({target_populations})')

return target_populations, weights_by_type, delays_by_type


def _check_drive_parameter_values(drive_type, **kwargs):
Expand Down
250 changes: 67 additions & 183 deletions hnn_core/network.py

Large diffs are not rendered by default.

21 changes: 13 additions & 8 deletions hnn_core/network_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from .cell import _ArtificialCell
from .params import _long_name, _short_name
from .extracellular import _ExtracellularArrayBuilder
from .network import pick_connection

# a few globals
_PC = None
Expand Down Expand Up @@ -363,23 +364,27 @@ def _gid_assign(self, rank=None, n_hosts=None):

self.net._update_cells() # updates net.n_cells

# round robin assignment of gids
# round robin assignment of cell gids
for gid in range(self._rank, self.net.n_cells, n_hosts):
# set the cell gid
self._gid_list.append(gid)

# loop over all drives, then all cell types, then all artificial cells
# only assign a "source" artificial cell to this rank if its target
# exists in _gid_list. "Global" drives get placed on different ranks
for drive in self.net.external_drives.values():
if drive['cell_specific']:
for conn in drive['conn'].values(): # all cell types
for src_gid, target_gid in zip(conn['src_gids'],
conn['target_gids']):
# only assign drive gids that have a target cell gid already
# assigned to this rank
for src_gid in self.net.gid_ranges[drive['name']]:
conn_idxs = pick_connection(self.net, src_gids=src_gid)
target_gids = list()
for conn_idx in conn_idxs:
target_gids += (self.net.connectivity[conn_idx]
['gid_pairs'][src_gid])

for target_gid in set(target_gids):
if (target_gid in self._gid_list and
src_gid not in self._gid_list):
self._gid_list.append(src_gid)
else:
# round robin assignment of drive gids
src_gids = list(self.net.gid_ranges[drive['name']])
for gid_idx in range(self._rank, len(src_gids), n_hosts):
self._gid_list.append(src_gids[gid_idx])
Expand Down
14 changes: 7 additions & 7 deletions hnn_core/params.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,15 +177,15 @@ def _extract_drive_specs_from_hnn_params(params, cellname_list):
if cname_ampa in par:
ampa_weight = par[cname_ampa][0]
ampa_delay = par[cname_ampa][1]
if ampa_weight > 0.:
if ampa_weight >= 0.:
Copy link
Contributor

Choose a reason for hiding this comment

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

Since most param files contain zero-weighted drive connection parameters, this allows these param files to still be instantiated in hnn-core without throwing an error in drives.py L33-35.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Could you update whats_new.rst so we know what changed from a user perspective?

drive['weights_ampa'][cellname] = ampa_weight

# NB synaptic delay same for NMDA, read only for AMPA
drive['synaptic_delays'][cellname] = ampa_delay

if cname_nmda in par:
nmda_weight = par[cname_nmda][0]
if nmda_weight > 0.:
if nmda_weight >= 0.:
drive['weights_nmda'][cellname] = nmda_weight

drive_specs[feed_name] = drive
Expand Down Expand Up @@ -228,9 +228,9 @@ def _extract_drive_specs_from_hnn_params(params, cellname_list):
ampa_weight = par[cellname][0]
nmda_weight = par[cellname][1]
synaptic_delays = par[cellname][2]
if ampa_weight > 0.:
if ampa_weight >= 0.:
drive['weights_ampa'][cellname] = ampa_weight
if nmda_weight > 0.:
if nmda_weight >= 0.:
drive['weights_nmda'][cellname] = nmda_weight
drive['synaptic_delays'][cellname] = synaptic_delays

Expand All @@ -249,7 +249,7 @@ def _extract_drive_specs_from_hnn_params(params, cellname_list):
if cellname in par:
ampa_weight = par[cellname][0]
synaptic_delays = par[cellname][3]
if ampa_weight > 0.:
if ampa_weight >= 0.:
drive['weights_ampa'][cellname] = ampa_weight
drive['synaptic_delays'][cellname] = synaptic_delays

Expand All @@ -272,9 +272,9 @@ def _extract_drive_specs_from_hnn_params(params, cellname_list):
ampa_weight = par[cellname][0]
nmda_weight = par[cellname][1]
synaptic_delays = par[cellname][2]
if ampa_weight > 0.:
if ampa_weight >= 0.:
drive['weights_ampa'][cellname] = ampa_weight
if nmda_weight > 0.:
if nmda_weight >= 0.:
drive['weights_nmda'][cellname] = nmda_weight
drive['synaptic_delays'][cellname] = synaptic_delays

Expand Down
74 changes: 52 additions & 22 deletions hnn_core/tests/test_drives.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from hnn_core.drives import (drive_event_times, _get_prng, _create_extpois,
_create_bursty_input)
from hnn_core.params import create_pext
from hnn_core.network import pick_connection


def test_external_drive_times():
Expand Down Expand Up @@ -115,10 +116,8 @@ def test_add_drives():
net = Network(params, legacy_mode=False)

# Ensure weights and delays are updated
weights_ampa = {'L2_basket': 1.0, 'L2_pyramidal': 3.0,
'L5_basket': 2.0, 'L5_pyramidal': 4.0}
syn_delays = {'L2_basket': 1.0, 'L2_pyramidal': 2.0,
'L5_basket': 3.0, 'L5_pyramidal': 4.0}
weights_ampa = {'L2_basket': 1.0, 'L2_pyramidal': 3.0, 'L5_pyramidal': 4.0}
Copy link
Contributor

Choose a reason for hiding this comment

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

Removed to check weight=None conditon?

Copy link
Contributor

Choose a reason for hiding this comment

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

Are you referring to the L5_basket weight? I removed this because most of the tests in this function create distal drives.

syn_delays = {'L2_basket': 1.0, 'L2_pyramidal': 2.0, 'L5_pyramidal': 4.0}

n_drive_cells = 10
cell_specific = False # default for bursty drive
Expand All @@ -129,9 +128,12 @@ def test_add_drives():

assert net.external_drives['bursty']['n_drive_cells'] == n_drive_cells
assert net.external_drives['bursty']['cell_specific'] == cell_specific
for type_name, drive in net.external_drives['bursty']['conn'].items():
assert drive['ampa']['A_weight'] == weights_ampa[type_name]
assert drive['ampa']['A_delay'] == syn_delays[type_name]
conn_idxs = pick_connection(net, src_gids='bursty')
Copy link
Contributor

Choose a reason for hiding this comment

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

I can't believe it took me so long to make this function. So much easier...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

so much weekend activity. Catching up now :)

I think the name of the function should be pick_connections though (plural) ... probably a separate PR however

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 for pick_connections. I'm fine with doing it here since it probably isn't too much.

Copy link
Contributor

Choose a reason for hiding this comment

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

Any objections?

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's do separate PR :) I think it would make it harder to read

for conn_idx in conn_idxs:
drive_conn = net.connectivity[conn_idx]
target_type = drive_conn['target_type']
assert drive_conn['nc_dict']['A_weight'] == weights_ampa[target_type]
assert drive_conn['nc_dict']['A_delay'] == syn_delays[target_type]

n_drive_cells = 'n_cells' # default for evoked drive
cell_specific = True
Expand All @@ -140,13 +142,16 @@ def test_add_drives():
weights_ampa=weights_ampa, location='distal',
synaptic_delays=syn_delays, cell_specific=True)

n_dist_targets = 270 # 235 with non-legacy mode
n_dist_targets = 235 # 270 with legacy mode
assert (net.external_drives['evoked_dist']
['n_drive_cells'] == n_dist_targets)
assert net.external_drives['evoked_dist']['cell_specific'] == cell_specific
for type_name, drive in net.external_drives['evoked_dist']['conn'].items():
assert drive['ampa']['A_weight'] == weights_ampa[type_name]
assert drive['ampa']['A_delay'] == syn_delays[type_name]
conn_idxs = pick_connection(net, src_gids='evoked_dist')
for conn_idx in conn_idxs:
drive_conn = net.connectivity[conn_idx]
target_type = drive_conn['target_type']
assert drive_conn['nc_dict']['A_weight'] == weights_ampa[target_type]
assert drive_conn['nc_dict']['A_delay'] == syn_delays[target_type]

n_drive_cells = 'n_cells' # default for poisson drive
cell_specific = True
Expand All @@ -155,13 +160,16 @@ def test_add_drives():
location='distal', synaptic_delays=syn_delays,
cell_specific=cell_specific)

n_dist_targets = 270 # 235 with non-legacy mode
n_dist_targets = 235 # 270 with non-legacy mode
assert (net.external_drives['poisson']
['n_drive_cells'] == n_dist_targets)
assert net.external_drives['poisson']['cell_specific'] == cell_specific
for type_name, drive in net.external_drives['poisson']['conn'].items():
assert drive['ampa']['A_weight'] == weights_ampa[type_name]
assert drive['ampa']['A_delay'] == syn_delays[type_name]
conn_idxs = pick_connection(net, src_gids='poisson')
for conn_idx in conn_idxs:
drive_conn = net.connectivity[conn_idx]
target_type = drive_conn['target_type']
assert drive_conn['nc_dict']['A_weight'] == weights_ampa[target_type]
assert drive_conn['nc_dict']['A_delay'] == syn_delays[target_type]

# evoked
with pytest.raises(ValueError,
Expand All @@ -182,6 +190,17 @@ def test_add_drives():
match='Drive evoked_dist already defined'):
net.add_evoked_drive('evoked_dist', mu=10, sigma=1, numspikes=1,
location='distal')
with pytest.raises(ValueError,
match='No target cell types have been given a synaptic '
'weight'):
net.add_evoked_drive('evdist1', mu=10, sigma=1, numspikes=1,
location='distal')
with pytest.raises(ValueError,
match='When adding a distal drive, synaptic weight '
'cannot be defined for the L5_basket cell type'):
net.add_evoked_drive('evdist1', mu=10, sigma=1, numspikes=1,
location='distal', weights_ampa={'L5_basket': 1.},
synaptic_delays={'L5_basket': .1})
with pytest.raises(ValueError,
match='If cell_specific is True, n_drive_cells'):
net.add_evoked_drive('evdist1', mu=10, sigma=1, numspikes=1,
Expand Down Expand Up @@ -217,18 +236,24 @@ def test_add_drives():
with pytest.raises(ValueError,
match='Rate constant must be positive'):
net.add_poisson_drive('poisson1', location='distal',
rate_constant=0.)
rate_constant=0.,
weights_ampa=weights_ampa,
synaptic_delays=syn_delays)

with pytest.raises(ValueError,
match='Rate constants not provided for all target'):
net.add_poisson_drive('poisson1', location='distal',
rate_constant={'L2_pyramidal': 10.},
weights_ampa={'L5_pyramidal': .01})
weights_ampa=weights_ampa,
synaptic_delays=syn_delays)
with pytest.raises(ValueError,
match='Rate constant provided for unknown target cell'):
net.add_poisson_drive('poisson1', location='distal',
rate_constant={'L2_pyramidal': 10.,
'bogus_celltype': 20.})
'bogus_celltype': 20.},
weights_ampa={'L2_pyramidal': .01,
'bogus_celltype': .01},
synaptic_delays=0.1)

with pytest.raises(ValueError,
match='Drives specific to cell types are only '
Expand Down Expand Up @@ -267,19 +292,24 @@ def test_add_drives():
with pytest.raises(ValueError,
match='Drive evoked_dist already defined'):
net.add_poisson_drive('evoked_dist', location='distal',
rate_constant=10.)
rate_constant=10.,
weights_ampa=weights_ampa,
synaptic_delays=syn_delays)
with pytest.raises(ValueError,
match='Allowed drive target locations are:'):
net.add_poisson_drive('weird_poisson', location='inbetween',
rate_constant=10.)
rate_constant=10.,
weights_ampa=weights_ampa,
synaptic_delays=syn_delays)
with pytest.raises(ValueError,
match='Allowed drive target cell types are:'):
net.add_poisson_drive('cell_unknown', location='proximal',
rate_constant=10.,
weights_ampa={'CA1_pyramidal': 1.})
weights_ampa={'CA1_pyramidal': 1.},
synaptic_delays=.01)
with pytest.raises(ValueError,
match='synaptic_delays is either a common float or '
'needs to be specified as a dict for each cell'):
'needs to be specified as a dict for each of the cell'):
net.add_poisson_drive('cell_unknown', location='proximal',
rate_constant=10.,
weights_ampa={'L2_pyramidal': 1.},
Expand Down