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

New connection rule for Poisson-distributed number of synapses between neuron pairs #2960

Merged
merged 60 commits into from
Jan 3, 2024

Conversation

ackurth
Copy link
Contributor

@ackurth ackurth commented Sep 25, 2023

In my work, I found it practical to adapt the pairwise_bernoulli connection method to a pairwise_poisson method, where the number of synapses between pre- and post-synaptic neurons is drawn from a Poisson distribution.

The choice of the Poisson distribution is not arbitrary.
Imagine you have the average number of synapses p between two neurons of specific populations.
Oftentimes, these specific populations have distinct targeting patters: same inhibitory neurons target preferentially basal, other distal dendrites etc.
Assume that the process on which a synapses is established has overall length L and that this L is similar for all neurons in the target populations
Since the overall number of synapses between two neurons of pre- and postsynaptic populations is known (p), on a small piece of the target process with length ```x````, the probability of the synapses being established on that piece is p*x.
Here of course we assume that the connection probability is identical everywhere on the target process.
To first order, this approximation seems fine.
You can probably already see where I am going: The synapses are thus a Poisson process on the line, the number of synapses is Poisson distributed.

This has two additional advantages:
First, the heterogeneity in the network is bigger in comparison the the standard Erdos-Renyi graphs generated by the pairwise_bernoulli connection method.
Indeed, in this case variance relative to the mean (i.e. the Fano Factor) is N_1*N_2*p*(1-p) / (N_1 * N_2 * p) = (1-p) <= 1with equality iff p = 1.
Here, N_1, N_2 are the number of neurons in the pre- and post-synaptic populations.
For the Poisson based rule the Fano Factor equals 1.

Additionally, especially when working with spatial networks it frequently occurs that the average number of synapses between to populations (then interpreted as connection probability) on short distances exceeds 1.
This leads to errors when using pairwise_bernoulli, but is not a problem for the pairwise_poissson.

Anyway, in the current PR such a connection scheme is implemented.
If you deem this sufficiently interesting to potentially merge it into master I will add the documentation.

Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

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

Hello @ackurth! Thank you for this contribution, which seems useful. I have only had a first look at the code, so here a few initial observations:

  • Please make sure the formatting is in order.
  • I don't like the lam parameter name, and yes, the one true design flaw in Python is lambda as a keyword, but even lambda would not be good. Can you find a name that is neurobiologically meaningful?
  • As far as I could see, while you give a very nice justification for the new rule in the PR here, there is no documentation or example for the new rule. Could you add that?

@heplesser heplesser added T: Enhancement New functionality, model or documentation S: Normal Handle this with default priority I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) good first issue Good for newcomers labels Sep 27, 2023
@heplesser heplesser added this to In progress in Kernel via automation Sep 27, 2023
@jessica-mitchell
Copy link
Contributor

@ackurth I second the need for documentation; there should be a section added in the doc/htmldoc/synapses/connection_management.rst at the very least

@ackurth
Copy link
Contributor Author

ackurth commented Sep 27, 2023

@heplesser
I changed the variable name from lam to pairwise_avg_num_conns.
It's a bit wordy but I think the most precise. Unless one wants to exchange average with expected.

I also corrected the formatting now. I have to say the new (to me) workflow for this is quite nice.

@heplesser @jessica-mitchell
Sure, documentation must be added.
This is what I was aiming at with the last sentence of the message of the PR.
I now added some documentation in the doc/htmldoc/synapses/connection_management.rst as well as in the doc/htmldoc/networks/spatially_structured_networks.rst (latter I think is in order since the new connection rule is also nice and works for spatial networks).
Anyway, I did find that the description of why the connection rule is nice did not really fit in the doc/htmldoc/synapses/connection_management.rst.
Is there another place where I should write it down? Do we want this to be part of the documentation anyway?

@jessica-mitchell
Copy link
Contributor

@ackurth I see what you mean, but I can't think of another document that would suit such a description. In this case, I would suggest adding a note or subsection in connection_management. @heplesser do you have any other suggestions?

Copy link
Member

@nicolossus nicolossus left a comment

Choose a reason for hiding this comment

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

@ackurth Thanks! Overall this looks good to me. I only have some minor nitpicks, see inline comments.

@heplesser @jessica-mitchell This PR got me thinking that it would be nice to have a minimalistic example where the differences (including the different parametrizations) between all connection rules are shown. I think this can be handled in a separate PR. If you agree, I can create an issue to remind us.

doc/htmldoc/networks/spatially_structured_networks.rst Outdated Show resolved Hide resolved
doc/htmldoc/networks/spatially_structured_networks.rst Outdated Show resolved Hide resolved
nestkernel/connection_creator_impl.h Outdated Show resolved Hide resolved
nestkernel/connection_creator_impl.h Outdated Show resolved Hide resolved
nestkernel/random_generators.h Outdated Show resolved Hide resolved
pynest/nest/lib/hl_api_connection_helpers.py Outdated Show resolved Hide resolved
Kernel automation moved this from In progress to PRs pending approval Sep 28, 2023
ackurth and others added 3 commits September 29, 2023 09:18
Co-authored-by: Nicolai Haug <39106781+nicolossus@users.noreply.github.com>
Co-authored-by: Nicolai Haug <39106781+nicolossus@users.noreply.github.com>
Co-authored-by: Nicolai Haug <39106781+nicolossus@users.noreply.github.com>
Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

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

Hi @ackurth I have a few more suggestions, please see inline.

nestkernel/conn_builder.cpp Outdated Show resolved Hide resolved
nestkernel/connection_creator_impl.h Outdated Show resolved Hide resolved
doc/htmldoc/synapses/connection_management.rst Outdated Show resolved Hide resolved
nestkernel/conn_builder.h Outdated Show resolved Hide resolved
nestkernel/conn_builder.cpp Outdated Show resolved Hide resolved
nestkernel/conn_builder.cpp Outdated Show resolved Hide resolved
nestkernel/connection_creator.cpp Outdated Show resolved Hide resolved
nestkernel/connection_creator_impl.h Outdated Show resolved Hide resolved
nestkernel/connection_creator_impl.h Outdated Show resolved Hide resolved
testsuite/pytests/test_spatial/test_connect_layers.py Outdated Show resolved Hide resolved
ackurth and others added 11 commits January 2, 2024 19:27
Co-authored-by: Hans Ekkehard Plesser <hans.ekkehard.plesser@nmbu.no>
Co-authored-by: Hans Ekkehard Plesser <hans.ekkehard.plesser@nmbu.no>
Co-authored-by: Hans Ekkehard Plesser <hans.ekkehard.plesser@nmbu.no>
Co-authored-by: Hans Ekkehard Plesser <hans.ekkehard.plesser@nmbu.no>
Co-authored-by: Hans Ekkehard Plesser <hans.ekkehard.plesser@nmbu.no>
Co-authored-by: Hans Ekkehard Plesser <hans.ekkehard.plesser@nmbu.no>
Co-authored-by: Hans Ekkehard Plesser <hans.ekkehard.plesser@nmbu.no>
Co-authored-by: Hans Ekkehard Plesser <hans.ekkehard.plesser@nmbu.no>
Co-authored-by: Hans Ekkehard Plesser <hans.ekkehard.plesser@nmbu.no>
@ackurth
Copy link
Contributor Author

ackurth commented Jan 2, 2024

I addressed this in 995f22d .

@ackurth ackurth requested a review from heplesser January 2, 2024 22:21
Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

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

@ackurth Thanks for the changes, this looks good to me now!

@heplesser heplesser changed the title Poisson multapses Add pairwise_poisson connection rule for Poisson-distributed number of synapses between neuron pairs Jan 3, 2024
@heplesser heplesser changed the title Add pairwise_poisson connection rule for Poisson-distributed number of synapses between neuron pairs New connection rule for Poisson-distributed number of synapses between neuron pairs Jan 3, 2024
@heplesser heplesser merged commit d598702 into nest:master Jan 3, 2024
24 checks passed
Kernel automation moved this from PRs pending approval to Done Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Enhancement New functionality, model or documentation
Projects
Kernel
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants