jonescompneurolab / hnn-core

Simulation and optimization of neural circuits for MEG/EEG source estimates
https://jonescompneurolab.github.io/hnn-core/
BSD 3-Clause "New" or "Revised" License
51 stars 50 forks source link

BUG: `pick_connection` returns incorrect values when applied to network with no cell-cell connections #734

Closed ntolley closed 3 months ago

ntolley commented 3 months ago

Found by @gtdang

The following code returns drive connections when it should only return cell-cell connections

import hnn_core
from hnn_core import pick_connection, Network, read_params
import os.path as op

hnn_core_root = op.dirname(hnn_core.__file__)
params_fname = op.join(hnn_core_root, 'param', 'default.json')
params = read_params(params_fname)

net = Network(params, add_drives_from_params=True)
conn_indices = pick_connection(net, src_gids='L2_basket', target_gids='L2_basket',
                               loc='distal')
for conn_idx in conn_indices:
    print(net.connectivity[conn_idx])
    print(' ')
evdist1 -> L2_basket
cell counts: 35 srcs, 35 targets
connection probability: 1.0 
loc: 'distal'; receptor: 'ampa'
weight: 0.006562; delay: 0.1; lamtha: 3.0

evdist1 -> L2_basket
cell counts: 35 srcs, 35 targets
connection probability: 1.0 
loc: 'distal'; receptor: 'nmda'
weight: 0.019482; delay: 0.1; lamtha: 3.0
gtdang commented 3 months ago

https://github.com/jonescompneurolab/hnn-core/blob/54ddd4080b8db2661867fde8c0a25ee345aa4a40/hnn_core/network.py#L282-L294

Tracing the issue... I think lines 291-292 are nested too far in? That if statement can't ever trigger since it's nested in an a conditional where it must be True.

jasmainak commented 3 months ago

Fix with an accompanying unit test welcome !

gtdang commented 3 months ago

https://github.com/jonescompneurolab/hnn-core/blob/54ddd4080b8db2661867fde8c0a25ee345aa4a40/hnn_core/network.py#L282-L294

Tracing the issue... I think lines 291-292 are nested too far in? That if statement can't ever trigger since it's nested in an a conditional where it must be True.

Nevermind I'm incorrect. The intersect can change the set to none. I have a different solution.