Exponential lookup tables for homogenous STDP synapses #3247
Conversation
Co-authored-by: Susanne Kunkel <[email protected]> Co-authored-by: Jari Pronold <[email protected]>
JanVogelsang
added
S: Normal
otcathatsya
force-pushed
the
exp_lookup_hom_stdp
branch
from
e4b9e0e to
96cd3b0
Compare
otcathatsya
force-pushed
the
exp_lookup_hom_stdp
branch
from
96cd3b0 to
1807697
Compare
| @@ -144,7 +144,6 @@ def | |||
| /V_reset 0.0 mV % Reset Potential (mV) | |||
| /tau_syn_ex tau_syn % time const. postsynaptic excitatory currents (ms) | |||
| /tau_syn_in tau_syn % time const. postsynaptic inhibitory currents (ms) | |||
| /tau_minus 30.0 ms %time constant for STDP (depression) | |||
There was a problem hiding this comment.
Should this be changed in the hpc_benchmark? Should it not use the default (non-lookup-table) version of exp?
| @@ -0,0 +1,408 @@ | |||
| /* | |||
There was a problem hiding this comment.
Perhaps this could be named differently from "hom[ogeneous]", like "lookup" or "fast"? Please indicate in the model docstring how this model is different from iaf_psc_alpha.
|
|
||
| constexpr double TAU_MINUS_PLACEHOLDER = 1.0; // indicate that tau minus has not been set yet | ||
|
|
||
| /** |
There was a problem hiding this comment.
Please add documentation here about the fixed time step based lookup table.
| // Includes from sli: | ||
| #include "dictdatum.h" | ||
|
|
||
| #define DEBUG_ARCHIVER 1 |
| @@ -43,6 +43,18 @@ class histentry | |||
| size_t access_counter_; //!< access counter to enable removal of the entry, once all neurons read it | |||
| }; | |||
|
|
|||
| class histentry_step | |||
There was a problem hiding this comment.
please add a docstring describing how this is different from histentry
| @@ -761,7 +763,9 @@ class Node | |||
| * return the Kminus value at t (in ms). | |||
| * @throws UnexpectedEvent | |||
| */ | |||
| virtual double get_K_value( double t ); | |||
| virtual double get_K_value( long t, size_t& dt_steps ); | |||
There was a problem hiding this comment.
please add a detailed docstring
| @@ -777,6 +781,11 @@ class Node | |||
| * return the spike history for (t1,t2]. | |||
| * @throws UnexpectedEvent | |||
| */ | |||
| virtual void get_history( long t1, | |||
There was a problem hiding this comment.
please add a detailed docstring
| @@ -224,10 +224,10 @@ def build_network(logger): | |||
| nest.overwrite_files = True | |||
|
|
|||
| nest.message(M_INFO, "build_network", "Creating excitatory population.") | |||
| E_neurons = nest.Create("iaf_psc_alpha", NE, params=model_params) | |||
| E_neurons = nest.Create("iaf_psc_alpha_hom", NE, params=model_params) | |||
There was a problem hiding this comment.
should it be a user-configurable option which version of the model to select? Should the default remain the same (as the changed proposed in this PR is a numerical approximation for the purpose of runtime performance benefit)?
| @@ -18,12 +18,12 @@ | |||
| # | |||
| # You should have received a copy of the GNU General Public License | |||
| # along with NEST. If not, see <http://www.gnu.org/licenses/>. | |||
|
|
|||
| import unittest | |||
There was a problem hiding this comment.
Could you please use pytest instead? unittest is deprecated.
| @@ -42,8 +42,8 @@ M_ERROR setverbosity | |||
| /total_num_virtual_procs 1 | |||
| >> SetKernelStatus | |||
|
|
|||
| /iaf_psc_alpha Create /n1 Set | |||
| /iaf_psc_alpha Create /n2 Set | |||
| /iaf_psc_alpha_hom Create /n1 Set | |||
There was a problem hiding this comment.
Should this remain the original? Should the test for the "hom" version be added rather than replacing the original?
Updated implementation of lookup tables for exponential computations in homogenous STDP synapses (original commits by @suku248 @jarsi), for now just applied to
stdp_pl_synapse_hom. There are a few potential concerns:Ideally we'd be able to keep
ArchivingNodeas a common base class and support both (potentially) precise spike times as well assize_t-implemented grid steps, but due to the way history is stored this is impossible without costly conversion on multiple occasions. A dynamic polymorphism approach + conversions was implemented, but did considerably worse in benchmarks (albeit still better than the baseline, see figures below). For a simplified implementation, see: Make regular histentry step-based #3307Since we now require a separate archiving node (could be a static template impl but that also has minor overhead), we also need to duplicate the nodes that support homogenous STDP synapses; we introduced
iaf_psc_alpha_hom. A better solution would probably require a restructuring for the way archivers are implemented, e.g. as pointers.In the figures below, "plain" is an implementation that just replaced existing structures for testing purposes while "separate" is the current version with duplicated classes, they are both listed as a sanity check and performance should be the same. "Converted" uses a templated histentry and conversion approach in archiving node. Std and mean values taken over 3 rng seeds.
