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

Represent singularity-safe propagators for IAF models as classes #2385

Merged
merged 55 commits into from
Nov 30, 2022

Conversation

heplesser
Copy link
Contributor

This PR is a follow-up replacing #2356. The content is the same except for the last few commits, which change the class names to IAFPropagator{Exp,Alpha}, make some changes to what is inlined and what not, and update documentation.

#2294 depends on this one.

@heplesser heplesser added S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. I: Internal API Changes were introduced in basic internal workings of the simulator that developers need to know labels May 5, 2022
@heplesser heplesser requested a review from jougs May 5, 2022 21:51
Copy link
Contributor

@jougs jougs left a comment

Choose a reason for hiding this comment

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

I mostly had a look at the code structure and this looks good to me. Regarding the propagator implementations itself, I trust the review of @clinssen.

doc/htmldoc/neurons/exact-integration.rst Outdated Show resolved Hide resolved
Copy link
Contributor

@jessica-mitchell jessica-mitchell left a comment

Choose a reason for hiding this comment

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

Just a few commetns, see in line

doc/htmldoc/neurons/exact-integration.rst Outdated Show resolved Hide resolved
libnestutil/iaf_propagator.h Outdated Show resolved Hide resolved
testsuite/pytests/test_iaf_singularity.py Outdated Show resolved Hide resolved
testsuite/pytests/test_iaf_singularity.py Outdated Show resolved Hide resolved
testsuite/pytests/test_iaf_singularity.py Outdated Show resolved Hide resolved
heplesser and others added 7 commits November 10, 2022 13:04
Co-authored-by: Jochen Martin Eppler <jougs@gmx.net>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
Co-authored-by: jessica-mitchell <mitchell20j@gmail.com>
@heplesser
Copy link
Contributor Author

@clinssen Ping!

Copy link
Contributor

@clinssen clinssen left a comment

Choose a reason for hiding this comment

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

Approving with only a few minor comments. Cheers!

@clinssen clinssen moved this from To do to Review in Models Nov 29, 2022
@heplesser
Copy link
Contributor Author

@clinssen I am confused by the suggestions containing images. Could you clarify what you mean? In cells creating plots I tend to be generous with semicolons to suppress text output. Strictly speaking a semicolon is only needed after the final plot command in a cell, but tracking that during development is a bit tedious. I will review your comment on the plausibility argument.

@heplesser
Copy link
Contributor Author

@clinssen I merged the newest changes from master in the hope that all tests will pass again and beautified the notebook a bit more.

@heplesser heplesser merged commit c39cf7b into nest:master Nov 30, 2022
@heplesser heplesser moved this from Review to Done in Models Dec 1, 2022
@heplesser heplesser deleted the stinebuu_propagator_class branch December 19, 2022 21:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: Internal API Changes were introduced in basic internal workings of the simulator that developers need to know S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation.
Projects
Models
  
Done
Development

Successfully merging this pull request may close these issues.

Replacing deprecated model iaf_psc_alpha_canon with iaf_psc_alpha_ps leads to significant time increase
5 participants