Adding glif_psc_double_alpha model

[shixnya]

An extension of the glif_psc. This model uses sum of two alpha functions instead of a single alpha function as the post synaptic current input.

[heplesser]

I just put in a formal block until formalities are in place.

[janskaar]

Since this is the only model with this type of postsynaptic current, should there perhaps also be included a test of its correctness?

[shixnya]

Since this is the only model with this type of postsynaptic current, should there perhaps also be included a test of its correctness?

Hi janskaar,

Thank you for the code review. I want to make a file for testing the synaptic connection of this model. Where do you think is the most appropriate place for it? Will it be 'testsuite/pytests/sli2py_neurons' or 'testsuite/unittests/?

[janskaar]

Since this is the only model with this type of postsynaptic current, should there perhaps also be included a test of its correctness?

Hi janskaar,

Thank you for the code review. I want to make a file for testing the synaptic connection of this model. Where do you think is the most appropriate place for it? Will it be 'testsuite/pytests/sli2py_neurons' or 'testsuite/unittests/?

Hi, thanks for your reply.

The tests can be placed in testsuite/pytests/

[heplesser]

The formalities are in place now, so I approve concerning the formalities. The usual code review should continue.

[shixnya]

I made change to my PR so that all the parameters associated with the fast synaptic components are marked with '_fast'. I also added a file for checking some properties of the double alpha synapses (based on the test for glif_psc model), and fixed two other files that required extra parameters for testing the 'glif_psc_double_alpha' model.

[clinssen]

Apologies, the underscore should not be prefixed when it's in a \text{} block. I made some suggestions to fix the rendering.

As these are just some small changes, I am already approving this PR. Many thanks!

I would recommend to tackle the issues with glif_psc.h in a separate PR to keep each individual PR lean.

[nicolossus]

@shixnya Thanks for this PR! This looks overall good to me. I have a few nitpicks, however, see comments below.

Could you also include a simple example that highlights the difference between this model and glif_psc?

[shixnya]

Thank you all, for thorough reviews on the code and documentation. I think I addressed all the issues raised. Please let me know if there are any other things that need to be changed. Now I see "1 workflow awaiting approval". How can we proceed from here?

[nicolossus]

@shixnya I also think the code and documentation is good to go. I still think an example that showcases the difference in dynamics of glif_psc_double_alpha and glif_psc should be added.

Examples reside in the pynest/examples/ directory. You can for instance take a look at the glif_psc example ( pynest/examples/glif_psc_neuron.py) for how examples are structured and formatted. In order to include the example in the documentation, you need to add it to

nest-simulator/doc/htmldoc/examples/index.rst

Lines 74 to 79 in 75718a8

	.. grid-item-card:: GLIF (from Allen institute)
	:img-top: ../static/img/pynest/glif_cond.png
	* :doc:`../auto_examples/glif_cond_neuron`
	* :doc:`../auto_examples/glif_psc_neuron`

and also at the bottom of the same file

nest-simulator/doc/htmldoc/examples/index.rst

Lines 261 to 262 in 75718a8

	../auto_examples/glif_cond_neuron
	../auto_examples/glif_psc_neuron

We need either @clinssen or @heplesser to start the CI workflow.

[shixnya]

@nicolossus Thank you for reminding me! Yes. I added an example file for the double alpha synapse. And reapplied clang-format for the .h file that was modified. I hope this addresses all the concerns!

[nicolossus]

@shixnya Thanks! I have some minor grammatical suggestions in the example, but overall this looks good to me so I will go ahead and approve 👍

[shixnya]

Sorry. I accidentally closed this PR while trying to rebase to the updated upstream master.
Please let me open another PR for this.

[nicolossus]

@shixnya I think you just can undo the rebase and reopen this PR.

[shixnya]

@nicolossus Thanks for the comment. There is a newly added test in the updated portion of the code that requires some changes to accept the glif_psc_double_alpha model, so undoing the rebase will fail the test.

Now I see an option to 'Reopen' this PR, but somehow I see 0 changes between nest-simulator/master and shixnya/master in this page, even though mine contains all the changes I made.

Without seeing your message, I've already created a new PR. Should I proceed with it, or try to stick with this one?

[nicolossus]

I see, I am not quite sure what the preferred option is then. We could probably proceed with the new PR, but I think we need green light from a senior developer first (@heplesser).