hnn-core icon indicating copy to clipboard operation
hnn-core copied to clipboard

follow-up GUI improvements

Open rythorpe opened this issue 2 years ago • 22 comments

Checklist for follow-up adjustments to the initial implementation of the IPywidgets GUI in #76. Feel free to modify this list as needed!

MAINT:

  • [x] Remove hardcoded values and/or keys that assume specific configurations of a Network instance (e.g., assuming that Network contains 4 cell types). Instead, pull from general attributes of the Network object whenever possible so that the GUI can flexibly support various network models (see here for initial discussion).
  • [ ] Modify plot_tfr_morlet() so that hitting plot does not render multiple color bars

ENH:

  • [ ] There's currently a tab for "Cell connectivity" that allows the user to set the synaptic strength from one cell type to another. We should incorporate the ability to set connectivity probability etc. and differentiate between "network connectivity" and "connection strength". Maybe we could rename "Cell connectivity" -> "Network connectivity" which then has textboxes or sliders for connection probability, connection delay, and connection strength for each of the connection types.
  • [x] Specify units for user-controlled variables.
  • [ ] Prompt the user to specify which network model they wish to use (e.g., jones_2009_model, law_2021_model).
  • [ ] Set the step size in ipywidgets.FloatText to something much smaller (e.g., 0.001 uS maybe?).
  • [ ] Add a button that allows the user to set/reset a given drive's parameters to some arbitrary "reasonable" values. Alternatively, we could just display some sort of "default" value/range to the right of each parameter so that new users aren't overwhelmed with the task of coming up with a good starting value.
  • [ ] Align parallel axis whenever possible in neighboring split-layout widget windows.

rythorpe avatar Jul 11 '22 23:07 rythorpe

I'm wondering if it's better to just use matplotlib subplots instead of two Output windows so that the plots are always aligned ...

jasmainak avatar Jul 11 '22 23:07 jasmainak

That could work, although it might conflict with @ntolley's dream of eventually having widgets that can be moved around in a drag-and-drop fashion.

One benefit of going the subplots route is that it would allow users to produce custom figures that could more easily be incorporated into publications.

rythorpe avatar Jul 12 '22 02:07 rythorpe

we're wasting a lot of real-estate in the current approach. One could use plt.tight_layout() to get clean and professional looking plots ...

jasmainak avatar Jul 12 '22 02:07 jasmainak

here's a prototype on powerpoint:

image

jasmainak avatar Jul 12 '22 02:07 jasmainak

Following up on our conversation about the connectivity. I think we can avoid using the hardcoded values referencing the params file through a combination of the attribute net.connectivity and pick_connection()

@chenghuzi happy to chat about this when you think it's a good time to prioritize this change

ntolley avatar Jul 17 '22 20:07 ntolley

Following up on our conversation about the connectivity. I think we can avoid using the hardcoded values referencing the params file through a combination of the attribute net.connectivity and pick_connection()

@chenghuzi happy to chat about this when you think it's a good time to prioritize this change

Thanks fore reminding me of this. Yes we can definitely add the probability parameter into all boxes and I like the idea of extracting connectivity from the network object. But if my understanding is correct, doing so involves rewriting the jones_2009_model method to include all possible arguments. I'm not sure if @jasmainak is ok with this as he tries to reduce the complexity of gui. Besides, we're not actually modifying connectivity in a network but instead creating a new network with the modified parameter object every time we change any parameters. So I think probably the best way is to specify all (possible) parameters in a file and we construct the UI from that.

chenghuzi avatar Jul 17 '22 22:07 chenghuzi

@chenghuzi the jones_2009_model function returns an instance of Network. This class is supposed to be more general than the current model. For example, net.cell_types.keys() will give you the names of the cells. You can use that here instead of hardcoding 'L5_Pyramidal', 'L2_Pyramidal' etc. It won't increase the complexity much with small changes like this.

One of the long-term goals of the project is to be able to incorporate more complex models of the neocortex, so we should try to anticipate and keep the code general to ease such transitions in the future.

jasmainak avatar Jul 18 '22 03:07 jasmainak

@ntolley can you share a code snippet how to avoid the hardcoded values so @chenghuzi can get an idea? I'm guessing you're referring to this part of the code?

https://github.com/jonescompneurolab/hnn-core/blob/c7460ba27cfc95343c83a41f2415f510ebf67fcd/hnn_core/gui/gui.py#L978-L979

jasmainak avatar Jul 18 '22 03:07 jasmainak

Here is an example of the logic I've recently implemented in #419 that allows you to see if a given receptor and loc combination is valid.

You could loop through every combination of cell in net.cell_types combined with this logic to automatically generate a list of possible connection that won't throw an error.

ntolley avatar Jul 18 '22 13:07 ntolley

As for modifying connections directly, here's an example of how it'd be done:

new_weight = 0.1
new_prob = 0.5

conn_indices = pick_connection(net, src_gids='L2_pyramidal', target_gids='L5_pyramidal', loc='proximal', receptor='ampa')
assert len(conn_indices) == 1 # should only be modifying one specific connection
conn_idx = conn_indices[0]

net.connectivity[conn_idx]['nc_dict']['A_weight'] = new_weight
net.connectivity[conn_idx]['probability'] = new_prob

ntolley avatar Jul 18 '22 14:07 ntolley

Here is an example of the logic I've recently implemented in #419 that allows you to see if a given receptor and loc combination is valid.

You could loop through every combination of cell in net.cell_types combined with this logic to automatically generate a list of possible connection that won't throw an error.

I think I got this part. Will add this to the GUI soon

chenghuzi avatar Jul 19 '22 05:07 chenghuzi

As for modifying connections directly, here's an example of how it'd be done:

new_weight = 0.1
new_prob = 0.5

conn_indices = pick_connection(net, src_gids='L2_pyramidal', target_gids='L5_pyramidal', loc='proximal', receptor='ampa')
assert len(conn_indices) == 1 # should only be modifying one specific connection
conn_idx = conn_indices[0]

net.connectivity[conn_idx]['nc_dict']['A_weight'] = new_weight
net.connectivity[conn_idx]['probability'] = new_prob

Okay this allows us to directly modify an existing network. We can incorporate this to the GUI later. For now we can stick to the current solution.

chenghuzi avatar Jul 19 '22 05:07 chenghuzi

You mean something like this? Below lists all possible connections and it will be a really long list... and for each of them we need to include both the strength and probability input widgets.

cell_types = [ct for ct in gui.variables['net'].cell_types.keys()]
receptors = ('ampa', 'nmda', 'gabaa', 'gabab')
locations = ('proximal', 'distal', 'soma')
for src_gids in cell_types:
    for target_gids in cell_types:
        for receptor in receptors:
            for location in locations:
                conn_indices = pick_connection(net=gui.variables['net'],
                                               src_gids=src_gids,
                                               target_gids=target_gids,
                                               loc=location, receptor=receptor)
                if len(conn_indices)>0:
                    conn_idx = conn_indices[0]
                    current_w = gui.variables['net'].connectivity[conn_idx]['nc_dict']['A_weight']
                    current_p = gui.variables['net'].connectivity[conn_idx]['probability']
                    print(f"{src_gids}_{target_gids}_{receptor}_{location}_w_{current_w}_p_{current_p}")
                pass

And the output is:

L2_basket_L2_basket_ampa_proximal_w_0.0005_p_1.0
L2_basket_L2_basket_ampa_distal_w_0.0005_p_1.0
L2_basket_L2_basket_nmda_proximal_w_0.0005_p_1.0
L2_basket_L2_basket_nmda_distal_w_0.0005_p_1.0
L2_basket_L2_basket_gabaa_proximal_w_0.05_p_1.0
L2_basket_L2_basket_gabaa_distal_w_0.05_p_1.0
L2_basket_L2_basket_gabaa_soma_w_0.02_p_1.0
L2_basket_L2_basket_gabab_proximal_w_0.05_p_1.0
L2_basket_L2_basket_gabab_distal_w_0.05_p_1.0
L2_basket_L2_pyramidal_ampa_proximal_w_0.0005_p_1.0
L2_basket_L2_pyramidal_ampa_distal_w_0.0005_p_1.0
L2_basket_L2_pyramidal_nmda_proximal_w_0.0005_p_1.0
L2_basket_L2_pyramidal_nmda_distal_w_0.0005_p_1.0
L2_basket_L2_pyramidal_gabaa_proximal_w_0.05_p_1.0
L2_basket_L2_pyramidal_gabaa_distal_w_0.05_p_1.0
L2_basket_L2_pyramidal_gabaa_soma_w_0.05_p_1.0
L2_basket_L2_pyramidal_gabab_proximal_w_0.05_p_1.0
L2_basket_L2_pyramidal_gabab_distal_w_0.05_p_1.0
L2_basket_L2_pyramidal_gabab_soma_w_0.05_p_1.0
L2_basket_L5_basket_ampa_proximal_w_0.0005_p_1.0
L2_basket_L5_basket_ampa_distal_w_0.00025_p_1.0
L2_basket_L5_basket_ampa_soma_w_0.0005_p_1.0
L2_basket_L5_basket_nmda_proximal_w_0.0005_p_1.0
L2_basket_L5_basket_nmda_distal_w_0.0_p_1.0
L2_basket_L5_basket_gabaa_distal_w_0.001_p_1.0
L2_basket_L5_basket_gabaa_soma_w_0.05_p_1.0
L2_basket_L5_basket_gabab_soma_w_0.05_p_1.0
L2_basket_L5_pyramidal_ampa_proximal_w_0.0005_p_1.0
L2_basket_L5_pyramidal_ampa_soma_w_0.0005_p_1.0
L2_basket_L5_pyramidal_nmda_proximal_w_0.0005_p_1.0
L2_basket_L5_pyramidal_nmda_soma_w_0.0005_p_1.0
L2_basket_L5_pyramidal_gabaa_proximal_w_0.05_p_1.0
L2_basket_L5_pyramidal_gabaa_distal_w_0.001_p_1.0
L2_basket_L5_pyramidal_gabaa_soma_w_0.05_p_1.0
L2_basket_L5_pyramidal_gabab_proximal_w_0.05_p_1.0
L2_basket_L5_pyramidal_gabab_soma_w_0.05_p_1.0
L2_pyramidal_L2_basket_ampa_proximal_w_0.0005_p_1.0
L2_pyramidal_L2_basket_ampa_distal_w_0.0005_p_1.0
L2_pyramidal_L2_basket_ampa_soma_w_0.0005_p_1.0
L2_pyramidal_L2_basket_nmda_proximal_w_0.0005_p_1.0
L2_pyramidal_L2_basket_nmda_distal_w_0.0005_p_1.0
L2_pyramidal_L2_basket_gabaa_proximal_w_0.05_p_1.0
L2_pyramidal_L2_basket_gabaa_distal_w_0.05_p_1.0
L2_pyramidal_L2_basket_gabab_proximal_w_0.05_p_1.0
L2_pyramidal_L2_basket_gabab_distal_w_0.05_p_1.0
L2_pyramidal_L2_pyramidal_ampa_proximal_w_0.0005_p_1.0
L2_pyramidal_L2_pyramidal_ampa_distal_w_0.0005_p_1.0
L2_pyramidal_L2_pyramidal_ampa_soma_w_0.0005_p_1.0
L2_pyramidal_L2_pyramidal_nmda_proximal_w_0.0005_p_1.0
L2_pyramidal_L2_pyramidal_nmda_distal_w_0.0005_p_1.0
L2_pyramidal_L2_pyramidal_nmda_soma_w_0.0005_p_1.0
L2_pyramidal_L2_pyramidal_gabaa_distal_w_0.05_p_1.0
L2_pyramidal_L2_pyramidal_gabaa_soma_w_0.05_p_1.0
L2_pyramidal_L2_pyramidal_gabab_distal_w_0.05_p_1.0
L2_pyramidal_L2_pyramidal_gabab_soma_w_0.05_p_1.0
L2_pyramidal_L5_basket_ampa_proximal_w_0.0005_p_1.0
L2_pyramidal_L5_basket_ampa_distal_w_0.0005_p_1.0
L2_pyramidal_L5_basket_ampa_soma_w_0.00025_p_1.0
L2_pyramidal_L5_basket_nmda_proximal_w_0.0005_p_1.0
L2_pyramidal_L5_basket_nmda_distal_w_0.0005_p_1.0
L2_pyramidal_L5_basket_gabaa_proximal_w_0.05_p_1.0
L2_pyramidal_L5_basket_gabaa_distal_w_0.05_p_1.0
L2_pyramidal_L5_basket_gabab_proximal_w_0.05_p_1.0
L2_pyramidal_L5_basket_gabab_distal_w_0.05_p_1.0
L2_pyramidal_L5_pyramidal_ampa_proximal_w_0.00025_p_1.0
L2_pyramidal_L5_pyramidal_ampa_distal_w_0.00025_p_1.0
L2_pyramidal_L5_pyramidal_ampa_soma_w_0.0005_p_1.0
L2_pyramidal_L5_pyramidal_nmda_soma_w_0.0005_p_1.0
L2_pyramidal_L5_pyramidal_gabaa_soma_w_0.05_p_1.0
L2_pyramidal_L5_pyramidal_gabab_soma_w_0.05_p_1.0
L5_basket_L2_basket_ampa_proximal_w_0.0005_p_1.0
L5_basket_L2_basket_ampa_distal_w_0.00025_p_1.0
L5_basket_L2_basket_ampa_soma_w_0.0005_p_1.0
L5_basket_L2_basket_nmda_proximal_w_0.0005_p_1.0
L5_basket_L2_basket_nmda_distal_w_0.0_p_1.0
L5_basket_L2_basket_gabaa_distal_w_0.001_p_1.0
L5_basket_L2_basket_gabaa_soma_w_0.05_p_1.0
L5_basket_L2_basket_gabab_soma_w_0.05_p_1.0
L5_basket_L2_pyramidal_ampa_proximal_w_0.0005_p_1.0
L5_basket_L2_pyramidal_ampa_distal_w_0.00025_p_1.0
L5_basket_L2_pyramidal_ampa_soma_w_0.0005_p_1.0
L5_basket_L2_pyramidal_nmda_proximal_w_0.0005_p_1.0
L5_basket_L2_pyramidal_nmda_distal_w_0.0_p_1.0
L5_basket_L2_pyramidal_gabaa_distal_w_0.001_p_1.0
L5_basket_L2_pyramidal_gabaa_soma_w_0.05_p_1.0
L5_basket_L2_pyramidal_gabab_soma_w_0.05_p_1.0
L5_basket_L5_basket_ampa_proximal_w_0.0005_p_1.0
L5_basket_L5_basket_ampa_distal_w_0.0005_p_1.0
L5_basket_L5_basket_nmda_proximal_w_0.0005_p_1.0
L5_basket_L5_basket_nmda_distal_w_0.0005_p_1.0
L5_basket_L5_basket_gabaa_proximal_w_0.05_p_1.0
L5_basket_L5_basket_gabaa_distal_w_0.05_p_1.0
L5_basket_L5_basket_gabaa_soma_w_0.02_p_1.0
L5_basket_L5_basket_gabab_proximal_w_0.05_p_1.0
L5_basket_L5_basket_gabab_distal_w_0.05_p_1.0
L5_basket_L5_pyramidal_ampa_proximal_w_0.0005_p_1.0
L5_basket_L5_pyramidal_ampa_distal_w_0.0005_p_1.0
L5_basket_L5_pyramidal_nmda_proximal_w_0.0005_p_1.0
L5_basket_L5_pyramidal_nmda_distal_w_0.0005_p_1.0
L5_basket_L5_pyramidal_gabaa_proximal_w_0.05_p_1.0
L5_basket_L5_pyramidal_gabaa_distal_w_0.05_p_1.0
L5_basket_L5_pyramidal_gabaa_soma_w_0.025_p_1.0
L5_basket_L5_pyramidal_gabab_proximal_w_0.05_p_1.0
L5_basket_L5_pyramidal_gabab_distal_w_0.05_p_1.0
L5_basket_L5_pyramidal_gabab_soma_w_0.025_p_1.0
L5_pyramidal_L2_basket_ampa_proximal_w_0.0005_p_1.0
L5_pyramidal_L2_basket_ampa_distal_w_0.00025_p_1.0
L5_pyramidal_L2_basket_ampa_soma_w_0.0005_p_1.0
L5_pyramidal_L2_basket_nmda_proximal_w_0.0005_p_1.0
L5_pyramidal_L2_basket_nmda_distal_w_0.0_p_1.0
L5_pyramidal_L2_basket_gabaa_distal_w_0.001_p_1.0
L5_pyramidal_L2_basket_gabaa_soma_w_0.05_p_1.0
L5_pyramidal_L2_basket_gabab_soma_w_0.05_p_1.0
L5_pyramidal_L2_pyramidal_ampa_proximal_w_0.0005_p_1.0
L5_pyramidal_L2_pyramidal_ampa_distal_w_0.00025_p_1.0
L5_pyramidal_L2_pyramidal_ampa_soma_w_0.0005_p_1.0
L5_pyramidal_L2_pyramidal_nmda_proximal_w_0.0005_p_1.0
L5_pyramidal_L2_pyramidal_nmda_distal_w_0.0_p_1.0
L5_pyramidal_L2_pyramidal_gabaa_distal_w_0.001_p_1.0
L5_pyramidal_L2_pyramidal_gabaa_soma_w_0.05_p_1.0
L5_pyramidal_L2_pyramidal_gabab_soma_w_0.05_p_1.0
L5_pyramidal_L5_basket_ampa_proximal_w_0.0005_p_1.0
L5_pyramidal_L5_basket_ampa_distal_w_0.0005_p_1.0
L5_pyramidal_L5_basket_ampa_soma_w_0.0005_p_1.0
L5_pyramidal_L5_basket_nmda_proximal_w_0.0005_p_1.0
L5_pyramidal_L5_basket_nmda_distal_w_0.0005_p_1.0
L5_pyramidal_L5_basket_gabaa_proximal_w_0.05_p_1.0
L5_pyramidal_L5_basket_gabaa_distal_w_0.05_p_1.0
L5_pyramidal_L5_basket_gabab_proximal_w_0.05_p_1.0
L5_pyramidal_L5_basket_gabab_distal_w_0.05_p_1.0
L5_pyramidal_L5_pyramidal_ampa_proximal_w_0.0005_p_1.0
L5_pyramidal_L5_pyramidal_ampa_distal_w_0.0005_p_1.0
L5_pyramidal_L5_pyramidal_ampa_soma_w_0.0005_p_1.0
L5_pyramidal_L5_pyramidal_nmda_proximal_w_0.0005_p_1.0
L5_pyramidal_L5_pyramidal_nmda_distal_w_0.0005_p_1.0
L5_pyramidal_L5_pyramidal_nmda_soma_w_0.0005_p_1.0
L5_pyramidal_L5_pyramidal_gabaa_distal_w_0.05_p_1.0
L5_pyramidal_L5_pyramidal_gabaa_soma_w_0.05_p_1.0
L5_pyramidal_L5_pyramidal_gabab_distal_w_0.05_p_1.0
L5_pyramidal_L5_pyramidal_gabab_soma_w_0.05_p_1.0

chenghuzi avatar Jul 19 '22 06:07 chenghuzi

Perhaps I misunderstood, but isn't this what you meant by "specify all (possible) parameters in a file and we construct the UI from that."?

I think my main point was to show that this information doesn't need to be hard-coded into a file, and can leverage the objects themself.

As for implementation, likely the best way to go forward is to generate a nested dictionary that stores the valid connections for the Network, but the nesting is done in a way that mirrors the connectivity widgets. For instance

valid_conns = {'target_cells' : {'L2_pyramidal': {'src_cells': 'L2_basket': {'L2Basket_L2Pyr_gabaa_soma': {'weight': 0.5, ...}}}}}

This object would only need to be generated once at the start (using the loop from above), and then the panels of the GUI could be constructed based on it's elements.

Feel free to take another approach if you see a better way! I remember you commenting that you wanted to have access to this sort of information so this is mainly just to show you how to access it.

ntolley avatar Jul 19 '22 07:07 ntolley

Perhaps I misunderstood, but isn't this what you meant by "specify all (possible) parameters in a file and we construct the UI from that."?

I think my main point was to show that this information doesn't need to be hard-coded into a file, and can leverage the objects themself.

As for implementation, likely the best way to go forward is to generate a nested dictionary that stores the valid connections for the Network, but the nesting is done in a way that mirrors the connectivity widgets. For instance

valid_conns = {'target_cells' : {'L2_pyramidal': {'src_cells': 'L2_basket': {'L2Basket_L2Pyr_gabaa_soma': {'weight': 0.5, ...}}}}}

This object would only need to be generated once at the start (using the loop from above), and then the panels of the GUI could be constructed based on it's elements.

Feel free to take another approach if you see a better way! I remember you commenting that you wanted to have access to this sort of information so this is mainly just to show you how to access it.

Check the latest commit in #500. Now all connections are generated from the initialized network object using pick_connection.

chenghuzi avatar Jul 19 '22 23:07 chenghuzi

Also, I just noticed that for certain combinations of source and targets, pick_connection return the same connectivity id conn_idx . Is this an expected behavior? See the attached snippets.

Since we're using conn_idx to modify weight and probability values,

net.connectivity[conn_idx]['nc_dict']['A_weight'] = new_weight
net.connectivity[conn_idx]['probability'] = new_prob

it seems this leads to parameter overwriting.

Attached output:

for connectivity_slider in gui.connectivity_sliders:
    for vbox in connectivity_slider:
        # specify connection
        conn_indices = pick_connection(
            net=gui.variables['net'],
            src_gids=vbox._belongsto['src_gids'],
            target_gids=vbox._belongsto['target_gids'],
            loc=vbox._belongsto['location'],
            receptor=vbox._belongsto['receptor'])

        assert len(conn_indices) > 0
        conn_idx = conn_indices[0]
        w = gui.variables['net'].connectivity[conn_idx][
                        'nc_dict']['A_weight']
        print(f"src_gids={vbox._belongsto['src_gids']}, target_gids={vbox._belongsto['target_gids']},location={vbox._belongsto['location']},conn_idx={conn_idx}")

generates:

src_gids=L2_basket, target_gids=L2_basket,location=proximal,conn_idx=1
src_gids=L2_basket, target_gids=L2_basket,location=proximal,conn_idx=0
src_gids=L2_basket, target_gids=L2_basket,location=proximal,conn_idx=4
src_gids=L2_basket, target_gids=L2_basket,location=proximal,conn_idx=5
src_gids=L2_basket, target_gids=L2_basket,location=distal,conn_idx=1
src_gids=L2_basket, target_gids=L2_basket,location=distal,conn_idx=0
src_gids=L2_basket, target_gids=L2_basket,location=distal,conn_idx=4
src_gids=L2_basket, target_gids=L2_basket,location=distal,conn_idx=5
src_gids=L2_basket, target_gids=L2_basket,location=soma,conn_idx=12
src_gids=L2_basket, target_gids=L2_pyramidal,location=proximal,conn_idx=1
src_gids=L2_basket, target_gids=L2_pyramidal,location=proximal,conn_idx=0
src_gids=L2_basket, target_gids=L2_pyramidal,location=proximal,conn_idx=4
src_gids=L2_basket, target_gids=L2_pyramidal,location=proximal,conn_idx=5
src_gids=L2_basket, target_gids=L2_pyramidal,location=distal,conn_idx=1
src_gids=L2_basket, target_gids=L2_pyramidal,location=distal,conn_idx=0
src_gids=L2_basket, target_gids=L2_pyramidal,location=distal,conn_idx=4
src_gids=L2_basket, target_gids=L2_pyramidal,location=distal,conn_idx=5
src_gids=L2_basket, target_gids=L2_pyramidal,location=soma,conn_idx=4
src_gids=L2_basket, target_gids=L2_pyramidal,location=soma,conn_idx=5
src_gids=L2_basket, target_gids=L5_basket,location=proximal,conn_idx=1
src_gids=L2_basket, target_gids=L5_basket,location=proximal,conn_idx=0
src_gids=L2_basket, target_gids=L5_basket,location=distal,conn_idx=9
src_gids=L2_basket, target_gids=L5_basket,location=distal,conn_idx=10
src_gids=L2_basket, target_gids=L5_basket,location=soma,conn_idx=11
src_gids=L2_basket, target_gids=L5_basket,location=soma,conn_idx=4
src_gids=L2_basket, target_gids=L5_basket,location=soma,conn_idx=5
src_gids=L2_basket, target_gids=L5_pyramidal,location=proximal,conn_idx=1
src_gids=L2_basket, target_gids=L5_pyramidal,location=proximal,conn_idx=0
src_gids=L2_basket, target_gids=L5_pyramidal,location=proximal,conn_idx=4
src_gids=L2_basket, target_gids=L5_pyramidal,location=proximal,conn_idx=5
src_gids=L2_basket, target_gids=L5_pyramidal,location=distal,conn_idx=10
src_gids=L2_basket, target_gids=L5_pyramidal,location=soma,conn_idx=1
src_gids=L2_basket, target_gids=L5_pyramidal,location=soma,conn_idx=0
src_gids=L2_basket, target_gids=L5_pyramidal,location=soma,conn_idx=4
src_gids=L2_basket, target_gids=L5_pyramidal,location=soma,conn_idx=5
src_gids=L2_pyramidal, target_gids=L2_basket,location=proximal,conn_idx=1
src_gids=L2_pyramidal, target_gids=L2_basket,location=proximal,conn_idx=0
src_gids=L2_pyramidal, target_gids=L2_basket,location=proximal,conn_idx=4
src_gids=L2_pyramidal, target_gids=L2_basket,location=proximal,conn_idx=5
src_gids=L2_pyramidal, target_gids=L2_basket,location=distal,conn_idx=1
src_gids=L2_pyramidal, target_gids=L2_basket,location=distal,conn_idx=0
src_gids=L2_pyramidal, target_gids=L2_basket,location=distal,conn_idx=4
src_gids=L2_pyramidal, target_gids=L2_basket,location=distal,conn_idx=5
src_gids=L2_pyramidal, target_gids=L2_basket,location=soma,conn_idx=11
src_gids=L2_pyramidal, target_gids=L2_pyramidal,location=proximal,conn_idx=1
src_gids=L2_pyramidal, target_gids=L2_pyramidal,location=proximal,conn_idx=0
src_gids=L2_pyramidal, target_gids=L2_pyramidal,location=distal,conn_idx=1
src_gids=L2_pyramidal, target_gids=L2_pyramidal,location=distal,conn_idx=0
src_gids=L2_pyramidal, target_gids=L2_pyramidal,location=distal,conn_idx=4
src_gids=L2_pyramidal, target_gids=L2_pyramidal,location=distal,conn_idx=5
src_gids=L2_pyramidal, target_gids=L2_pyramidal,location=soma,conn_idx=1
src_gids=L2_pyramidal, target_gids=L2_pyramidal,location=soma,conn_idx=0
src_gids=L2_pyramidal, target_gids=L2_pyramidal,location=soma,conn_idx=4
src_gids=L2_pyramidal, target_gids=L2_pyramidal,location=soma,conn_idx=5
src_gids=L2_pyramidal, target_gids=L5_basket,location=proximal,conn_idx=1
src_gids=L2_pyramidal, target_gids=L5_basket,location=proximal,conn_idx=0
src_gids=L2_pyramidal, target_gids=L5_basket,location=proximal,conn_idx=4
src_gids=L2_pyramidal, target_gids=L5_basket,location=proximal,conn_idx=5
src_gids=L2_pyramidal, target_gids=L5_basket,location=distal,conn_idx=1
src_gids=L2_pyramidal, target_gids=L5_basket,location=distal,conn_idx=0
src_gids=L2_pyramidal, target_gids=L5_basket,location=distal,conn_idx=4
src_gids=L2_pyramidal, target_gids=L5_basket,location=distal,conn_idx=5
src_gids=L2_pyramidal, target_gids=L5_basket,location=soma,conn_idx=15
src_gids=L2_pyramidal, target_gids=L5_pyramidal,location=proximal,conn_idx=8
src_gids=L2_pyramidal, target_gids=L5_pyramidal,location=distal,conn_idx=9
src_gids=L2_pyramidal, target_gids=L5_pyramidal,location=soma,conn_idx=1
src_gids=L2_pyramidal, target_gids=L5_pyramidal,location=soma,conn_idx=0
src_gids=L2_pyramidal, target_gids=L5_pyramidal,location=soma,conn_idx=4
src_gids=L2_pyramidal, target_gids=L5_pyramidal,location=soma,conn_idx=5
src_gids=L5_basket, target_gids=L2_basket,location=proximal,conn_idx=1
src_gids=L5_basket, target_gids=L2_basket,location=proximal,conn_idx=0
src_gids=L5_basket, target_gids=L2_basket,location=distal,conn_idx=9
src_gids=L5_basket, target_gids=L2_basket,location=distal,conn_idx=10
src_gids=L5_basket, target_gids=L2_basket,location=soma,conn_idx=11
src_gids=L5_basket, target_gids=L2_basket,location=soma,conn_idx=4
src_gids=L5_basket, target_gids=L2_basket,location=soma,conn_idx=5
src_gids=L5_basket, target_gids=L2_pyramidal,location=proximal,conn_idx=1
src_gids=L5_basket, target_gids=L2_pyramidal,location=proximal,conn_idx=0
src_gids=L5_basket, target_gids=L2_pyramidal,location=distal,conn_idx=9
src_gids=L5_basket, target_gids=L2_pyramidal,location=distal,conn_idx=10
src_gids=L5_basket, target_gids=L2_pyramidal,location=soma,conn_idx=11
src_gids=L5_basket, target_gids=L2_pyramidal,location=soma,conn_idx=4
src_gids=L5_basket, target_gids=L2_pyramidal,location=soma,conn_idx=5
src_gids=L5_basket, target_gids=L5_basket,location=proximal,conn_idx=1
src_gids=L5_basket, target_gids=L5_basket,location=proximal,conn_idx=0
src_gids=L5_basket, target_gids=L5_basket,location=proximal,conn_idx=4
src_gids=L5_basket, target_gids=L5_basket,location=proximal,conn_idx=5
src_gids=L5_basket, target_gids=L5_basket,location=distal,conn_idx=1
src_gids=L5_basket, target_gids=L5_basket,location=distal,conn_idx=0
src_gids=L5_basket, target_gids=L5_basket,location=distal,conn_idx=4
src_gids=L5_basket, target_gids=L5_basket,location=distal,conn_idx=5
src_gids=L5_basket, target_gids=L5_basket,location=soma,conn_idx=13
src_gids=L5_basket, target_gids=L5_pyramidal,location=proximal,conn_idx=1
src_gids=L5_basket, target_gids=L5_pyramidal,location=proximal,conn_idx=0
src_gids=L5_basket, target_gids=L5_pyramidal,location=proximal,conn_idx=4
src_gids=L5_basket, target_gids=L5_pyramidal,location=proximal,conn_idx=5
src_gids=L5_basket, target_gids=L5_pyramidal,location=distal,conn_idx=1
src_gids=L5_basket, target_gids=L5_pyramidal,location=distal,conn_idx=0
src_gids=L5_basket, target_gids=L5_pyramidal,location=distal,conn_idx=4
src_gids=L5_basket, target_gids=L5_pyramidal,location=distal,conn_idx=5
src_gids=L5_basket, target_gids=L5_pyramidal,location=soma,conn_idx=6
src_gids=L5_basket, target_gids=L5_pyramidal,location=soma,conn_idx=7
src_gids=L5_pyramidal, target_gids=L2_basket,location=proximal,conn_idx=1
src_gids=L5_pyramidal, target_gids=L2_basket,location=proximal,conn_idx=0
src_gids=L5_pyramidal, target_gids=L2_basket,location=distal,conn_idx=9
src_gids=L5_pyramidal, target_gids=L2_basket,location=distal,conn_idx=10
src_gids=L5_pyramidal, target_gids=L2_basket,location=soma,conn_idx=11
src_gids=L5_pyramidal, target_gids=L2_basket,location=soma,conn_idx=4
src_gids=L5_pyramidal, target_gids=L2_basket,location=soma,conn_idx=5
src_gids=L5_pyramidal, target_gids=L2_pyramidal,location=proximal,conn_idx=1
src_gids=L5_pyramidal, target_gids=L2_pyramidal,location=proximal,conn_idx=0
src_gids=L5_pyramidal, target_gids=L2_pyramidal,location=distal,conn_idx=9
src_gids=L5_pyramidal, target_gids=L2_pyramidal,location=distal,conn_idx=10
src_gids=L5_pyramidal, target_gids=L2_pyramidal,location=soma,conn_idx=11
src_gids=L5_pyramidal, target_gids=L2_pyramidal,location=soma,conn_idx=4
src_gids=L5_pyramidal, target_gids=L2_pyramidal,location=soma,conn_idx=5
src_gids=L5_pyramidal, target_gids=L5_basket,location=proximal,conn_idx=1
src_gids=L5_pyramidal, target_gids=L5_basket,location=proximal,conn_idx=0
src_gids=L5_pyramidal, target_gids=L5_basket,location=proximal,conn_idx=4
src_gids=L5_pyramidal, target_gids=L5_basket,location=proximal,conn_idx=5
src_gids=L5_pyramidal, target_gids=L5_basket,location=distal,conn_idx=1
src_gids=L5_pyramidal, target_gids=L5_basket,location=distal,conn_idx=0
src_gids=L5_pyramidal, target_gids=L5_basket,location=distal,conn_idx=4
src_gids=L5_pyramidal, target_gids=L5_basket,location=distal,conn_idx=5
src_gids=L5_pyramidal, target_gids=L5_basket,location=soma,conn_idx=14
src_gids=L5_pyramidal, target_gids=L5_pyramidal,location=proximal,conn_idx=3
src_gids=L5_pyramidal, target_gids=L5_pyramidal,location=proximal,conn_idx=2
src_gids=L5_pyramidal, target_gids=L5_pyramidal,location=distal,conn_idx=1
src_gids=L5_pyramidal, target_gids=L5_pyramidal,location=distal,conn_idx=0
src_gids=L5_pyramidal, target_gids=L5_pyramidal,location=distal,conn_idx=4
src_gids=L5_pyramidal, target_gids=L5_pyramidal,location=distal,conn_idx=5
src_gids=L5_pyramidal, target_gids=L5_pyramidal,location=soma,conn_idx=1
src_gids=L5_pyramidal, target_gids=L5_pyramidal,location=soma,conn_idx=0
src_gids=L5_pyramidal, target_gids=L5_pyramidal,location=soma,conn_idx=4
src_gids=L5_pyramidal, target_gids=L5_pyramidal,location=soma,conn_idx=5

chenghuzi avatar Jul 19 '22 23:07 chenghuzi

hmm this is definitely not the expected behavior. If you look at the net.connectivity for the jones_2009_model(), there should only be one connection returned when all parameters in pick_connection() are specified (instead of their default of None).

I'll need to play around with the code to see what's going on.

ntolley avatar Jul 20 '22 06:07 ntolley

hmm this is definitely not the expected behavior. If you look at the net.connectivity for the jones_2009_model(), there should only be one connection returned when all parameters in pick_connection() are specified (instead of their default of None).

I'll need to play around with the code to see what's going on.

This seems also to give us repeating indices:

conn_idxs = []
for connectivity_slider in gui.connectivity_sliders:
    for vbox in connectivity_slider:
        # specify connection
        conn_indices = pick_connection(
            net=gui.variables['net'],
            src_gids=vbox._belongsto['src_gids'],
            target_gids=vbox._belongsto['target_gids'],
            loc=vbox._belongsto['location'],
            receptor=vbox._belongsto['receptor'])

        assert len(conn_indices) > 0
        if len(conn_indices)==1:
            conn_idx = conn_indices[0]
            conn_idxs.append(conn_idx)
            w = gui.variables['net'].connectivity[conn_idx][
                            'nc_dict']['A_weight']
            print(f"src_gids={vbox._belongsto['src_gids']}, target_gids={vbox._belongsto['target_gids']},location={vbox._belongsto['location']},conn_idx={conn_idx}")
            
print(len(conn_idxs)-len(set(conn_idxs)))

generates:

src_gids=L2_basket, target_gids=L2_basket,location=soma,conn_idx=12
src_gids=L2_basket, target_gids=L2_pyramidal,location=soma,conn_idx=4
src_gids=L2_basket, target_gids=L2_pyramidal,location=soma,conn_idx=5
src_gids=L2_basket, target_gids=L5_basket,location=distal,conn_idx=9
src_gids=L2_basket, target_gids=L5_basket,location=distal,conn_idx=10
src_gids=L2_basket, target_gids=L5_pyramidal,location=distal,conn_idx=10
src_gids=L2_pyramidal, target_gids=L2_basket,location=soma,conn_idx=11
src_gids=L2_pyramidal, target_gids=L2_pyramidal,location=proximal,conn_idx=1
src_gids=L2_pyramidal, target_gids=L2_pyramidal,location=proximal,conn_idx=0
src_gids=L2_pyramidal, target_gids=L5_basket,location=soma,conn_idx=15
src_gids=L2_pyramidal, target_gids=L5_pyramidal,location=proximal,conn_idx=8
src_gids=L2_pyramidal, target_gids=L5_pyramidal,location=distal,conn_idx=9
src_gids=L5_basket, target_gids=L2_basket,location=distal,conn_idx=9
src_gids=L5_basket, target_gids=L2_basket,location=distal,conn_idx=10
src_gids=L5_basket, target_gids=L2_pyramidal,location=distal,conn_idx=9
src_gids=L5_basket, target_gids=L2_pyramidal,location=distal,conn_idx=10
src_gids=L5_basket, target_gids=L5_basket,location=soma,conn_idx=13
src_gids=L5_basket, target_gids=L5_pyramidal,location=soma,conn_idx=6
src_gids=L5_basket, target_gids=L5_pyramidal,location=soma,conn_idx=7
src_gids=L5_pyramidal, target_gids=L2_basket,location=distal,conn_idx=9
src_gids=L5_pyramidal, target_gids=L2_basket,location=distal,conn_idx=10
src_gids=L5_pyramidal, target_gids=L2_pyramidal,location=distal,conn_idx=9
src_gids=L5_pyramidal, target_gids=L2_pyramidal,location=distal,conn_idx=10
src_gids=L5_pyramidal, target_gids=L5_basket,location=soma,conn_idx=14
src_gids=L5_pyramidal, target_gids=L5_pyramidal,location=proximal,conn_idx=3
src_gids=L5_pyramidal, target_gids=L5_pyramidal,location=proximal,conn_idx=2
10

Are we supposed to use short names instead of long names?

chenghuzi avatar Jul 20 '22 07:07 chenghuzi

After running some tests something is indeed going wrong with pick_connection(). When looking up non-existing connections

Not sure if this issue has always existed, or if it was introduced by a more recent PR. Will work on it today and open an issue/PR soon.

ntolley avatar Jul 20 '22 08:07 ntolley

I might be misunderstanding the issue here, but it looks like the duplicate connection indices belong to different receptors (i.e., AMPA, NMDA, GABAA, and GABAB).

rythorpe avatar Jul 20 '22 18:07 rythorpe

you can use git bisect to figure out when a bug cropped up. This is why it is very important to add tests, so we do not have regressions as the software expands

jasmainak avatar Jul 20 '22 19:07 jasmainak

So the bug always existed. The problem was that I accidentally wrote a test for the one situation where an empty list is actually returned when the connection doesn't exist (has to do with the order in which parameters are checked in the function). This just didn't come up previously because typically we know what connection we want.

I added several more tests that check different ways a connection searched should return an empty list.

ntolley avatar Jul 21 '22 08:07 ntolley

Closing this in favor of the more up-to-date to-do list, #671.

rythorpe avatar Oct 12 '23 16:10 rythorpe