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.

Sign up for GitHub

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

Jump to bottom

Re-organise tripartite connectivity generation to support multiple primary connection rules #3204

Merged
merged 53 commits into from
Sep 3, 2024
Merged

Re-organise tripartite connectivity generation to support multiple primary connection rules #3204

merged 53 commits into from
Sep 3, 2024

Conversation

heplesser
Copy link
Contributor

This PR re-organises the tripartite connectivity generation to (a) allow combination with all primary connection rules and (b) support parallelization.

@HanjiaJiang Could you provide benchmarks?

@heplesser
Experimental work on new tripartite implementation; does not yet compile
72c7b88
@heplesser
Code now compiles, not complete
4f34121
@heplesser
All tests not related to disconnect, structural plasticity or tripart…
3f6f7e9 
…ite connectivity pass
@heplesser
First minimal example works
0dab76e
@heplesser
Version working for single MPI process, random pool, no disconnect/st…
4e1d0d9 
…ructural plast
@heplesser
Adapted tripartite tests to new syntax
7376c6b
@heplesser
Structural plasticity and disconnect work
dce1ef1
@heplesser
Block pools now supported again
240f553
@heplesser
Add MPI support for new tripartite scheme
9adaf6f
@heplesser
Fix formatting and pylint problems; improve example names
a403765
@heplesser
Fix conngen builder
a16cbea
@heplesser
Prohibit 'make_symmetric' in for tripartite connectivity and extend t…
a883317 
…ests
@heplesser
Merge branch 'master' of github.com:nest/nest-simulator into new_trip…
4a3396b 
…artite
@heplesser
Unregister third-factor conn factories on ResetKernel
6fd3995
@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) labels May 6, 2024
@heplesser heplesser added this to the NEST 3.8 milestone Jun 20, 2024
@heplesser heplesser removed the request for review from YounesBouhadjar June 20, 2024 11:13
clinssen
clinssen reviewed Jun 20, 2024
View reviewed changes
nestkernel/conn_builder.cpp Outdated Show resolved Hide resolved
nestkernel/conn_builder.h Outdated Show resolved Hide resolved
nestkernel/conn_builder.h Outdated Show resolved Hide resolved
nestkernel/connection_manager.cpp Show resolved Hide resolved
nestkernel/connection_manager.cpp Show resolved Hide resolved
nestkernel/conn_builder.h Outdated Show resolved Hide resolved
@heplesser heplesser added I: User Interface Users may need to change their code due to changes in function calls and removed I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) labels Jun 20, 2024
@heplesser heplesser requested review from IiroAhokainen and removed request for HanjiaJiang June 20, 2024 16:19
@heplesser heplesser marked this pull request as ready for review August 26, 2024 07:24
@heplesser
Copy link
Contributor Author

@IiroAhokainen @clinssen I have now fixed the problem with pooling, so the PR is ready for review again.

@HanjiaJiang Could you create a PR towards my branch that would adjust the parameters in the examples as discussed above? I think you know the pertaining example files much better :).

@HanjiaJiang
Copy link
Contributor

@IiroAhokainen @clinssen I have now fixed the problem with pooling, so the PR is ready for review again.

@HanjiaJiang Could you create a PR towards my branch that would adjust the parameters in the examples as discussed above? I think you know the pertaining example files much better :).

Yes @heplesser I will do this. Do you think I should also include the change for default delta_IP3 (to 0.0002) as well? I know I can change it in astrocyte_lr_1994.cpp but just to double check.

HanjiaJiang and others added 2 commits August 26, 2024 10:42
@HanjiaJiang @IiroAhokainen
Change parameter values in astrocyte example scripts according to Iir…
7c5cd28 
…o's suggestions

Co-Authored-by: Iiro Ahokainen <59561196+IiroAhokainen@users.noreply.github.com>
Co-Authored-by: Jugoslava Acimovic <jugoslava.acimovic@tuni.fi>
@HanjiaJiang
Replace astrocyte_interaction.png with new version
2e65b02
@heplesser
Copy link
Contributor Author

Yes @heplesser I will do this. Do you think I should also include the change for default delta_IP3 (to 0.0002) as well? I know I can change it in astrocyte_lr_1994.cpp but just to double check.

@HanjiaJiang Since @IiroAhokainen pointed out that the current default value makes no sense physiologically, please change the default value for delta_IP3. Please do this change in a separate commit that changes nothing else, with a commit message like "Change astrocyte_lr_1994 parameter default value for delta_IP3 to physiologically plausible value"

@heplesser
Copy link
Contributor Author

Parameter changes are now integrated, so the PR is complete from my side.

@clinssen @IiroAhokainen Looking forward to your re-reviews.

IiroAhokainen
IiroAhokainen reviewed Aug 27, 2024
View reviewed changes
nestkernel/nestmodule.h Outdated Show resolved Hide resolved
nestkernel/conn_builder.cpp Outdated Show resolved Hide resolved
nestkernel/conn_builder.h Show resolved Hide resolved
nestkernel/conn_builder.h Outdated Show resolved Hide resolved
nestkernel/conn_builder_factory.h Outdated Show resolved Hide resolved
pynest/examples/astrocytes/astrocyte_small_network.py Outdated Show resolved Hide resolved
pynest/examples/astrocytes/astrocyte_brunel_fixed_indegree.py Outdated Show resolved Hide resolved
pynest/examples/astrocytes/astrocyte_brunel_fixed_indegree.py Outdated Show resolved Hide resolved
pynest/examples/astrocytes/astrocyte_brunel_fixed_indegree.py Outdated Show resolved Hide resolved
@heplesser
Copy link
Contributor Author

@IiroAhokainen If you are done with your re-review, kindly approve explicitly :).

clinssen
clinssen approved these changes Sep 3, 2024
View reviewed changes
doc/htmldoc/synapses/connectivity_concepts.rst Outdated Show resolved Hide resolved
doc/htmldoc/synapses/connectivity_concepts.rst Outdated Show resolved Hide resolved
@heplesser heplesser changed the title Re-organise tripartite connectivity generation Re-organise tripartite connectivity generation to support multiple primary connection rules Sep 3, 2024
@heplesser heplesser merged commit ef0074a into nest:master Sep 3, 2024
24 checks passed
@HanjiaJiang
Copy link
Contributor

Thank you everyone but especially @heplesser for the hard work! This is really a great achievement. 🥳

@jugoslavaacimovic
Copy link
Contributor

Thanks everyone for your hard work and dedication. It is great to see the project and the code advancing to this point. Looking forward to see it's impact on the CNS community.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HanjiaJiang HanjiaJiang HanjiaJiang left review comments

@jugoslavaacimovic jugoslavaacimovic jugoslavaacimovic left review comments

@clinssen clinssen clinssen approved these changes

@IiroAhokainen IiroAhokainen IiroAhokainen approved these changes

Assignees
No one assigned
Labels
I: User Interface Users may need to change their code due to changes in function calls S: Normal Handle this with default priority T: Enhancement New functionality, model or documentation
Projects
Kernel
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@heplesser @HanjiaJiang @jugoslavaacimovic @clinssen @IiroAhokainen