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

Problems with stdp_synapse weight update #1604

Open
golosio opened this issue May 18, 2020 · 10 comments
Open

Problems with stdp_synapse weight update #1604

golosio opened this issue May 18, 2020 · 10 comments
Assignees
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation.

Comments

@golosio
Copy link

golosio commented May 18, 2020

The synaptic weight should decrease when presynaptic spike time > postsynaptic spike time
However, in simple pynest scripts it seems that things go in the opposite way with stdp_synapse.
Furthermore, it seems that the first pre-post couple is always lost, i.e. the update always starts from the second pre-post couple.
Comparison with theoretical calculation shows that the weight increases as if post-pre is inverted, and the update starts from the second pre-post couple, missing the first one.
stdp.py.zip

@clinssen
Copy link
Contributor

Some confusion might arise because the word "delay" is used ambiguously in NEST Simulator. Intuitively we expect this to be an axonal delay (time from presynaptic neuron firing until time of spike arrival at the synapse), but actually it typically refers to the dendritic delay (time from spike arrival at the synapse to time at which the spike has an effect on the membrane potential).

With this interpretation, from the perspective of the STDP synapse in your example, the spike arrival actually occurs as pre-before-post, so the synapse is potentiated, as observed. No spikes should be "lost" regardless of the timing.

Please let us know if you continue to observe any behaviour that is not explained by the interpretation of delays as purely dendritic.

@golosio
Copy link
Author

golosio commented May 18, 2020 via email

@golosio
Copy link
Author

golosio commented May 18, 2020

I get consistent results if I change the sign of dendritic_delay in models/stdp_connection.h ,

243c243
<   target->get_history( t_lastspike_ - dendritic_delay, t_spike - dendritic_delay, &start, &finish );
>   target->get_history( t_lastspike_ + dendritic_delay, t_spike + dendritic_delay, &start, &finish );
248c248
<     minus_dt = t_lastspike_ - ( start->t_ + dendritic_delay );
>     minus_dt = t_lastspike_ + dendritic_delay - start->t_; 
256c256
<   const double _K_value = target->get_K_value( t_spike - dendritic_delay );
>   const double _K_value = target->get_K_value( t_spike + dendritic_delay );

or alternatively if I assume that the "delay "parameter in stdp_synapse refers to the time the postsynaptic action potential takes to travel "backward" through the dendrites until it reaches the input synapse. Please let me know if the latter is the intended behavior. If this is the case, I must say yes, it is really confusing, also because I would expect in most cases the axonal delay to be larger.
Furthermore, I have to say that while the fix to the dendritic_delay sign completely solves the issue, the interpretation of the delay as "backward propagation time of the postsynaptic action potential through the dendrites" does not solve the problem that the first couple of pre-post spikes is always lost.

@clinssen
Copy link
Contributor

This interpretation of the delays is indeed correct. I agree that we should make this more clear in the documentation.

As for the first pair being "lost": might this be due to another implementation quirk? In NEST, for performance reasons, the weight is only updated when a presynaptic spike is received by the synapse. Postsynaptic spikes are buffered (from the perspective of the synapse), and only result in a weight update at the time that the next presynaptic spike arrives. This is entirely consistent on a network dynamics level, but if you start inspecting weight values at arbitrary times during the simulation, you might hit a not-yet-updated value, making it seem wrong.

@golosio
Copy link
Author

golosio commented May 19, 2020

Thank you Charl. I made a drawing that in my opinion clarifies the intended behavior.
synaptic_post_pre_diff.pdf
In NEST, the value of the stdp_synapse "delay" parameter is assigned to the variable dendritic_delay, which corresponds to what is called Dt_post in the drawing.
Most users will assume "delay" to represent the axonal delay. In my opinion it would not be sufficient to make this more clear in the documentation, a modification of the code would be appropriate.
I suggest two possible modifications:

  1. If the increase in memory requirements for having one more parameter is not an issue, use two delay parameters, e.g. "axonal_delay" (or simply "delay", because this is what most users expect to be the meaning of the delay parameter) and "pspd_delay" (postsynaptic potential dendritic delay).
  2. If the increase in memory requirement is an issue, considering that what really matters is the difference Dt_pre - Dt_post , let the parameter "delay" represent this difference, i.e. the difference between the axonal delay and the time it takes for the postsynaptic action potential to propagate bacward through the dendrite. The current NEST behavior, which neglects axonal delay, could be obtained by setting "delay" to a negative value, equal to -Dt_post. The behavior that most users expects could be obtained by setting "delay" equal to Dt_pre, i.e. the axonal delay.
    EDIT: from the following comment it seems that solution 2) would not be appropriate, because
    while for STDP weight update only the difference Dt_pre - Dt_post is relevant, for evaluating the time when the dg_exc variable of synaptic conductance should be modified by a spike the relevant delay is Dt_pre (the axonal delay), so the only appropriate solution seems to be solution 1)

@golosio
Copy link
Author

golosio commented May 19, 2020

I must add that when the target neuron of the stdp synapse is a conductance-based model, the
time when the synaptic conductance is modified (i.e. when dg_exc is increased) due to an incoming spike is evaluated as the time of the presynaptic spike plus the "delay" parameter. This use of the delay parameter is not consistent with the dendritic delay interpretation.

@terhorstd terhorstd added I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation. labels May 19, 2020
@github-actions
Copy link

github-actions bot commented Sep 3, 2021

Issue automatically marked stale!

@github-actions github-actions bot added the stale Automatic marker for inactivity, please have another look here label Sep 3, 2021
@heplesser
Copy link
Contributor

@JanVogelsang Could you take a look at this?

@JanVogelsang
Copy link
Contributor

JanVogelsang commented Nov 26, 2022

What Charl said is correct and as Bruno said, we will need an implementation with both dendritic and axonal delays. And I can promise, we will soon have one, in combination with a nice thesis explaining all the details of delays in neural simulators, with a special focus on synaptic plasticity.

@github-actions github-actions bot removed the stale Automatic marker for inactivity, please have another look here label Dec 5, 2022
@jessica-mitchell jessica-mitchell moved this to In progress in Models Aug 6, 2024
@clinssen
Copy link
Contributor

Action points:

  • Update the documentation of STDP synapse models to make it more clear what "delay" means
  • Add subpanel from figure 8 from Morrison et al. Biol Cybern (2008) to further illustrate this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Maintenance Work to keep up the quality of the code and documentation.
Projects
Status: In progress
Development

No branches or pull requests

5 participants