[MRG] Add functions to modify and visualize connectivity #318

ntolley · 2021-04-19T22:33:02Z

At long last we're here! This PR is meant to flush out the API for modifying and visualizing the connectivity now that it has been decoupled from network_builder.py (#276), and the data structure is lightweight and interpretable (#313).

Thanks to @jasmainak's suggestion to store separate connection types as objects, there is now a very clean path towards modifying existing connections. To demonstrate, this PR enables modification of existing connections with net.connectivity[idx].update_probability(0.5). I have additionally added a probability argument to net.add_connection.

Here are my currently planned functions to add:

net.connectivity[idx].plot which will produce a "checkerboard" to visualize connections
net.connectivity[idx].update_srcs and net.connectivity[idx].update_targets to filter connections to a subset of srcs and targets. This will be really useful for modifying drives to the network that target different sub populations of cells.

Let me know if you guys have any other suggestions!

P.S. I think this mode of modifying connections will be quite amenable to the GUI since it modifies everything in place.

codecov-commenter · 2021-04-19T22:59:02Z

Codecov Report

Merging #318 (4c760c7) into master (2f1da11) will decrease coverage by 0.60%.
The diff coverage is 83.09%.

@@            Coverage Diff             @@
##           master     #318      +/-   ##
==========================================
- Coverage   90.50%   89.90%   -0.61%     
==========================================
  Files          13       13              
  Lines        2434     2497      +63     
==========================================
+ Hits         2203     2245      +42     
- Misses        231      252      +21

Impacted Files	Coverage Δ
hnn_core/network_builder.py	`92.16% <ø> (ø)`
hnn_core/viz.py	`81.45% <4.54%> (-6.05%)`	⬇️
hnn_core/network.py	`92.78% <97.47%> (-0.01%)`	⬇️
hnn_core/__init__.py	`100.00% <100.00%> (ø)`
hnn_core/parallel_backends.py	`83.57% <0.00%> (+0.86%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f1da11...4c760c7. Read the comment docs.

hnn_core/network.py

jasmainak · 2021-04-19T23:41:26Z

examples/plot_connectivity.py

+for conn in net.connectivity:
+    if conn['src_type'] == 'L2_basket':
+        conn.update_probability(0.1)


Can we use the net.add_connection(..., probability) method so we can still play around with _Connectivity class. I suspect we'll need to make changes when we unify it with the drive connectivity

jasmainak · 2021-04-19T23:52:33Z

I like that connectivity is becoming a proper object slowly! But it's not yet clear to me if the object we expose to the user stores the "list of connectivities" or just a single connectivity instance? It seems to me it might be more useful to do:

net.connectivity.plot()  # this will show all the connectivities as subplots?
net.connectivity.filter(drop_probability, src_types, target_types)

etc. In order to not let this decision slow down the development, I suggest starting from functions plot_connectivity(...), filter_connectivity etc. and then building methods (in a later PR) when it becomes clear some functions belong together in a class.

jasmainak · 2021-04-19T23:57:23Z

@ntolley are you familiar with the Iris dataset plot?

I am imagining something like this might be useful to show connections between different cell classes? Instead of scatterplot, you'll have matshow/imshow

ntolley · 2021-04-20T00:11:05Z

Great suggestion! I was actually just about to gripe that there is too much to plot for the default network, but filtering to a subset of connections might work here. This sort of plot would require fixing the loc and receptor which would make some plots a bit uninformative (invalid loc receptor combinations would be a big blob).

In general though I would like to emphasize the list of connections (or dict of connections that allows efficient indexing) rather than a generic net.connectivity class. We can still add auxillary functions to Network that call the methods of _Connection.

My rationale is that the "canonical" neocortical circuit is characterized by a unique set of connectivity rules. I think the API should reflect this conserved circuit pattern, and allow users to interact with it. For me the motivation is to probe what makes these connection rules so special, not so much looking at how a singular cell class impacts cortical function.

jasmainak · 2021-04-20T04:41:53Z

okay I see, makes sense.

How have neuro papers made plots like this? Do you have any examples? Might be good to conform to the expectations of most users ...

ntolley · 2021-04-20T13:56:07Z

Interestingly neither the Blue Brain Project or the Brain Modeling Toolkit plot connectivity matrices as we've discussed. Instead they have matrices of connection probabilities, where the rows/columns are the different cell classes. While these are some the most closely related projects, their emphasis on 1:1 morphologic reconstructions makes the visualizations not very relevant.

Papers constructing functional networks with single unit recordings will likely be the best place mimic visualizations. Mainly because the method forces grouping cells into broad categories (and there's only 4 in HNN). This Scherberger Paper does a nice plot showing how single units from different areas are connected:

Also Olaf Sporns has done a lot of work applying graph theory to neural networks, and his papers may have more visualization examples. However, his focus is on inter-areal connectivitiy rather than single neurons.

jasmainak · 2021-04-20T14:15:18Z

I like the subplots A and B in the figure you share above. Agree that we don't want to go the Blue Brain way ...

But are we going to colorcode and show lamtha? I still have difficulty visualizing how the weights are chosen and how these weights drive the currents. Agree that some domain knowledge is missing on my part. I am thinking that there needs to be a diagram like one of these at some point:

where the thickness of the line indicates the weight/probability and color indicates if it's inhibitory/excitatory? And the populations are arranged spatially as they would be in a cortical column. That way you can literally take the diagram and explain to someone what the mechanism is ...

For now let's go with A (and optionally B) though

jasmainak · 2021-04-23T23:57:37Z

examples/plot_connectivity.py


 net_remove = net.copy()
 dpl_remove = simulate_dipole(net_remove, n_trials=1)
 net_remove.cell_response.plot_spikes_raster()
+net.connectivity[10].plot()


can you change the sphinx gallery thumbnail to point to this plot?

jasmainak · 2021-04-24T00:00:23Z

examples/plot_connectivity.py

+for conn in net.connectivity:
+    if conn['src_type'] == 'L2_basket':
+        conn.drop(0.1)


connectivity is a private class. You shouldn't expose it (yet) ... could you perhaps expose probability using add_connection ? This feels like you're trying to modify the connection but we're still trying to settle on API for adding a connection

jasmainak · 2021-04-24T00:06:18Z

hnn_core/network.py

+        The probability attribute will store the most recent value passed to
+        this function. As such, this number does not accurately describe the
+        connections probability of the original set after successive calls.


keep the complexity bounded, otherwise software will become unmaintainable. You should try to address 80 percent usecases. Look up feature creep. I'd suggest throwing an error if someone tries to drop connections more than once

jasmainak · 2021-04-24T00:09:17Z

hnn_core/network.py

@@ -1170,6 +1196,74 @@ def __repr__(self):

        return entr

+    def drop(self, probability):


honestly I would keep this for future PR. I think you can add probability to add_connection with 1 or 2 lines and expose it in an example. This falls under the usecase of modifying existing connections which we haven't started looking into yet.

But we'll probably want to modify the "default" connections probabilistically, too?

cjayb · 2021-04-26T15:46:18Z

examples/plot_connectivity.py

-# spiking. L2 basket and pyramidal cells rhythymically fire in the gamma
-# range (30-80 Hz). As a final step, we can see how this change in spiking
-# activity impacts the aggregate current dipole.
+# Adding a single inhibitory connection didn't completely restored the normal


Suggested change

# Adding a single inhibitory connection didn't completely restored the normal

# Adding a single inhibitory connection did not completely restore the normal

cjayb · 2021-04-26T15:53:19Z

hnn_core/network.py

+
+        Parameters
+        ----------
+        conn : Instance of _Connectivity object


This is now a class method, document ax and show instead

cjayb · 2021-04-26T16:00:07Z

How about adding an argument to Network that makes the call to _set_default_connections conditional. Then we could have a demo of creating the default connectivity using (many) consecutive calls to add_ connection, with and without the probabilistic angle. It would make the default connectivity explicit (not that users would want to do that), and allow exploring less rigid patterns with probability. The drop-method could indeed wait, though I do see why you've included it here, @ntolley

jasmainak · 2021-04-28T17:34:44Z

related idea is to move _set_default_connectivity out of the Network class and into a separate function. I would do something like:

from hnn_core import default_network

network = default_network(params)

and then:

def default_network(params):
    net = Network()
    net.add_connection(...)
    return net

That way the Network class has less baggage of the past. We should try to make the init thin. It is doing too much right now

jasmainak · 2021-05-07T21:15:35Z

@ntolley you need a rebase here

ntolley · 2021-05-07T21:19:37Z

Can do! I'm a bit underwater with research projects until mid next week but I can focus on wrapping up this PR afterwards.

ntolley · 2021-05-14T21:52:49Z

The default network is now created with net = default_network(params) as suggested by @jasmainak.

I think all that is left to do is to fix the connectivity example. Since we are not supporting modifications of existing connectivity, I think this will have to be turned into the example suggested by @cjayb where we create a network (not necessarily the default!) with successive calls to net.add_connection in the notebook.

In that case it will likely be useful to move away from instantiating the entire default network, but just add a few sparse connections and show off the plotting functions, as well as how they behave in simulation.

jasmainak · 2021-05-14T23:36:22Z

Can you make the CIs happy so I can see how the example looks like?

Also @ntolley would you have some time to brainstorm the connection API over zoom sometime? I would like to make sure that we're not overseeing any critical usecases

jasmainak · 2021-05-14T23:38:32Z

Just wanted to mention that as such the PR looks good. But overall, we need to think a bit about the connectivity API and what needs to be done to make sure users have the best experience

ntolley · 2021-05-15T02:10:15Z

Just finished updating the example to be more appropriate for the current API. I've opted to create hand-made networks and compare their activity patterns.

Also I have a super free schedule for zoom Wed-Fri next week!

ntolley · 2021-05-15T02:19:07Z

In terms of critical use cases, two things come to mind. The first is implementing the src-target definition conventions in the drives API, allowing for drives to only a subset of cells. In my own research/reading. In my own research/reading this abililty seems fairly important for understanding circuit-EEG activity (how does the dipole look when subpopulations are driven from indepedent sources?).

The second thing we should consider is adding connection probabilities that decay with distance. While a fixed connection probability is fine for now, distance dependent probability is much more biologically realistic topology. My understanding is that the motivation for Gaussian decaying synaptic strengths was to generally how net synaptic inputs from one location fall off with distance. In reality though the strength of a synaptic connection isn't really influenced by distance (as far as I know...).

jasmainak · 2021-05-15T03:30:25Z

Cool, could you send me a zoom/calendar invite on one of those days? I'm pretty free. Everything works except Wednesday 9 am

examples/plot_simulate_gamma.py

hnn_core/network.py

cjayb

Looks great! I have a few minor comments/questions, sorry about being last-minute.

cjayb · 2021-05-18T07:57:29Z

examples/plot_connectivity.py

-src_gid = net.gid_ranges['L2_basket'][0]
-target_gids = 'L2_pyramidal'
+
+net_all = Network(params, add_drives_from_params=True)


Maybe it's because I'm an "old" user, but it's quite confusing to me that a "Network" can be initialised without connections. That's not really a network, but rather a bag of cells, right?

And why is this called net_all? Isn't it more like net_none? I really am confused...

net_all refers to the default all-to-all connectivity pattern which is later compared with a net_sparse pattern.

I see why the terminology is a bit confusing, but I am not sure how to get around it without reintroducing the add_connection steps back to the __init__.

I think one way this could be improved is a more robust way to remove connections. Something like

net = default_network(params) net.clear_connectivity(remove_drives=False)

This way the user never really has to deal with the Network constructor.

Or how about

net = default_network(params, connect=True, attach_drives=True)

I don't know if it should be a priority to avoid Network? There's just something odd about the example right now in the way that default_network and Network are used.

@jasmainak did you have a vision in mind for how default_network() would be used? Going down this path, it seems like we're just recreating the original Network.__init__ in some other location.

In my mind instantiating an empty network should require some intention by the user as very few people have the background to create a realistic network de novo.

Also I think this brings up a more important question of how different "supported" templates will be imported. For example if we create a well tuned sparse network, will there be a sparse_network(params) function? Or calcium_network(params)...

see my other comment. I think Network should be generalized a bit and default_network is where the heavy lifting of the params extraction should be done. I want the network class to go the same way as we did with the Cell class. I can't make up my mind whether we should have different param file specifications for every new network or if we can leverage existing specifications. I think the latter. However, once loaded into HNN-core, it should be represented with our Network class which is fairly generalizable.

hnn_core/network.py

hnn_core/tests/test_network.py

jasmainak · 2021-05-18T14:30:24Z

@ntolley don't tag anyone in commits -- they'll get notified every time you rebase and push that commit

ntolley · 2021-05-18T14:47:04Z

they'll get notified every time you rebase and push that commit

😨 sorry chris! We'll try and get this merged soon haha

@jasmainak I am adding methods to clear connectivity and drives separately to avoid using net = Network() in any of the examples. Then I will call it quits until a followup PR.

jasmainak · 2021-05-18T17:21:16Z

@ntolley are you aware of this paper: https://www.biorxiv.org/content/biorxiv/early/2021/04/18/2021.04.16.440210.full.pdf ?

@stephanie-r-jones mentioned to me that the connectivity is changed in there. Could that be a good example for us?

ntolley · 2021-05-18T20:35:34Z

@jasmainak regarding our previous discussion I was wrong. We can totally add that new connection with the current API. Here's the relevant line from the supplemental:

There were also some network structure differences between the 2009 model and current model. In the current model, we added
a  Martinotti-like recurrent tuft connections from the L5 interneuron to the L5 pyramidal neurons’ distal dendrites and removed
the L2/3 GABAA interneuron synapses onto those same dendrites.

I just checked and this is easily done in the model.

jasmainak · 2021-05-18T20:46:27Z

Nice! Is there something from the paper that we can easily demo/illustrate due to the new connectivity? I'm fine leaving this for another PR though if you think it's outside the scope.

ntolley · 2021-05-18T20:52:56Z

Let's leave it for another PR, I'll make it my next one to focus on. I agree that it will be great to show off as many examples as possible with published (almost) results. It's possible the influence of beta events on ERP's is partially present with just this new connection, but I'll need to play with the code some more.

There were a lot of modified parameters in this paper so it may require a separate params file. Of course this can be more easily done with #321 merged!

jasmainak · 2021-05-19T03:02:31Z

hnn_core/network.py

+    Notes
+    ----
+    `net = default_network(params)` is the reccomended path for creating a
+    network. Instantiating the network as `net = Network(params)` will


we should migrate the stuff that is specific to params specification to default_network slowly leaving a generalized Network class in place. That is to say, it should not take a params dictionary as input and should not have add_drives_from_params. Happy to delegate this to another PR though

I agree this is a great direction to go. Let's iterate on generalising Network and removing add_drives_from_params together with the legacy-flag.

jasmainak · 2021-05-19T03:13:30Z

hnn_core/network.py

@@ -201,8 +344,6 @@ def __init__(self, params, add_drives_from_params=False,
        self.n_cells = sum(len(self.pos_dict[src]) for src in
                           self.cell_types)

-        self._set_default_connections()
-


in a future PR, add_drives_from_params should be moved to default_network function

hnn_core/network.py

examples/plot_connectivity.py

jasmainak

looks good other than nitpicks. Can you address them and I'll merge first thing tomorrow morning.

Is there anything to update in whats_new @ntolley ?

Co-authored-by: Mainak Jas <jasmainak@users.noreply.github.com>

ntolley · 2021-05-19T17:38:03Z

I believe I've addressed the remaining comments! Let me know if anything else catches your eye, otherwise feel free to merge.

jasmainak · 2021-05-19T18:44:41Z

Thanks @ntolley !

jasmainak reviewed Apr 19, 2021

View reviewed changes

hnn_core/network.py Outdated Show resolved Hide resolved

jasmainak reviewed Apr 19, 2021

View reviewed changes

hnn_core/network.py Outdated Show resolved Hide resolved

jasmainak reviewed Apr 19, 2021

View reviewed changes

jasmainak reviewed Apr 23, 2021

View reviewed changes

jasmainak reviewed Apr 24, 2021

View reviewed changes

cjayb reviewed Apr 26, 2021

View reviewed changes

ntolley force-pushed the connectivity_functions branch from 991dd91 to 60b45a6 Compare May 14, 2021 18:47

ntolley changed the title ~~WIP: Add functions to modify and visualize connectivity~~ [MRG] Add functions to modify and visualize connectivity May 15, 2021

jasmainak reviewed May 15, 2021

View reviewed changes

examples/plot_simulate_gamma.py Show resolved Hide resolved

jasmainak reviewed May 15, 2021

View reviewed changes

hnn_core/network.py Outdated Show resolved Hide resolved

DOC: Fix incorrect statement about copying

1ea1fa9

ntolley force-pushed the connectivity_functions branch from e1276c8 to 1ea1fa9 Compare May 18, 2021 01:51

cjayb approved these changes May 18, 2021

View reviewed changes

cjayb mentioned this pull request May 18, 2021

[MRG] Improved LFP calculation #329

Merged

12 tasks

DOC: Address @cjayb comment on instantiating empty networks

c63c09b

ntolley added 2 commits May 18, 2021 10:55

Add methods to clear cell-cell connections and drives separately

89dd225

Flake8

7f87c05

jasmainak reviewed May 19, 2021

View reviewed changes

hnn_core/network.py Outdated Show resolved Hide resolved

jasmainak reviewed May 19, 2021

View reviewed changes

hnn_core/network.py Outdated Show resolved Hide resolved

jasmainak reviewed May 19, 2021

View reviewed changes

hnn_core/network.py Outdated Show resolved Hide resolved

jasmainak reviewed May 19, 2021

View reviewed changes

hnn_core/network.py Outdated Show resolved Hide resolved

jasmainak reviewed May 19, 2021

View reviewed changes

examples/plot_connectivity.py Show resolved Hide resolved

jasmainak approved these changes May 19, 2021

View reviewed changes

ntolley mentioned this pull request May 19, 2021

Clean up connectivity API and add tests #293

Closed

ntolley and others added 4 commits May 19, 2021 13:19

Apply suggestions from code review

a340d37

Co-authored-by: Mainak Jas <jasmainak@users.noreply.github.com>

Move connection probability method to function

abda596

Add line for example plots

7319280

DOC: Update whats_new.rst

4c760c7

jasmainak merged commit 8c07186 into jonescompneurolab:master May 19, 2021

ntolley mentioned this pull request May 21, 2021

Improvements to Network instantiation #338

Closed

2 tasks

		@@ -1170,6 +1196,74 @@ def __repr__(self):

		return entr

		def drop(self, probability):

	# Adding a single inhibitory connection didn't completely restored the normal
	# Adding a single inhibitory connection did not completely restore the normal

[MRG] Add functions to modify and visualize connectivity #318

[MRG] Add functions to modify and visualize connectivity #318

Conversation

ntolley commented Apr 19, 2021 • edited

codecov-commenter commented Apr 19, 2021 • edited

Codecov Report

Choose a reason for hiding this comment

jasmainak commented Apr 19, 2021 • edited

jasmainak commented Apr 19, 2021

ntolley commented Apr 20, 2021

jasmainak commented Apr 20, 2021

ntolley commented Apr 20, 2021

jasmainak commented Apr 20, 2021 • edited

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

cjayb commented Apr 26, 2021

jasmainak commented Apr 28, 2021 • edited

jasmainak commented May 7, 2021

ntolley commented May 7, 2021

ntolley commented May 14, 2021 • edited

jasmainak commented May 14, 2021

jasmainak commented May 14, 2021

ntolley commented May 15, 2021

ntolley commented May 15, 2021

jasmainak commented May 15, 2021

cjayb 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

Choose a reason for hiding this comment

jasmainak commented May 18, 2021

ntolley commented May 18, 2021 • edited

jasmainak commented May 18, 2021

ntolley commented May 18, 2021 • edited

jasmainak commented May 18, 2021

ntolley commented May 18, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jasmainak left a comment

Choose a reason for hiding this comment

ntolley commented May 19, 2021

jasmainak commented May 19, 2021

ntolley commented Apr 19, 2021 •

edited

codecov-commenter commented Apr 19, 2021 •

edited

jasmainak commented Apr 19, 2021 •

edited

jasmainak commented Apr 20, 2021 •

edited

jasmainak commented Apr 28, 2021 •

edited

ntolley commented May 14, 2021 •

edited

ntolley commented May 18, 2021 •

edited

ntolley commented May 18, 2021 •

edited