Changes to implementation and test of iaf_psc_exp_ps_lossless for Nest PR #901 #22
Conversation
There was a problem hiding this comment.
@ChristianKeup Thanks for fixing this, and yes, you created the PR in the correct way. I just found one small point to improve, see inline comments. Furthermore, the static code checker is unhappy about some lines in iaf_psc_exp_ps_lossless.cpp. Could you run extras/check_code_style.sh in your source directory and fix the errors indicated (using clang-format-3.6 -i ...?
precise/iaf_psc_exp_ps_lossless.cpp
Outdated
| g and f are interchanged. (compare to Fig.6) | ||
| */ | ||
|
|
||
| double f = |
There was a problem hiding this comment.
@ChristianKeup Make this const double, since f does not change below. This allows the compiler to generate more optimal code.
|
During checking the spike detection algorithm, I found that while the model allows to set two different time constants for exc. and inh. synapses, the spike detection rests on the assumption that the two time constants are equal. Therefore, should I build in a check that simply forbids the user to set different synaptic time constants, or should I completely replace 'tau_ex_' and 'tau_in_' by a single parameter 'tau_syn' ? |
|
@ChristianKeup Good catch! I would suggest that you add a test that enforces tau_ex == tau_in, and add a note in the documentation. In this way, we can later modify the model to support tau_ex != tau_in without any change to the user interface. Maybe you could create a follow-up issue for that. |
|
@ChristianKeup Travis is now failing due to an old test. I merge your PR here and will fix the test myself. |
Tried to improve C++-testing under OSX.
Removed buffers from spin_detector.
Resolves the problem I found in the spike detection algorithm and fixes the existing test.
The extension of the test to look at the region of missed spikes will follow after Easter.
For general comments please see Nest PR nest#901