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

Incorrect summation of synaptic currents for iaf_psc_exp_ps #368

Closed
apdavison opened this issue May 26, 2016 · 2 comments
Closed

Incorrect summation of synaptic currents for iaf_psc_exp_ps #368

apdavison opened this issue May 26, 2016 · 2 comments
Assignees
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: High Should be handled next T: Bug Wrong statements in the code or documentation ZC: Model DO NOT USE THIS LABEL ZP: PR Created DO NOT USE THIS LABEL
Milestone

Comments

@apdavison
Copy link

When two inputs, with equal but opposite weights, converge on an iaf_psc_exp_ps neuron with different time constants for the ex and in synapses, the voltage trace is flat. This problem does not occur with iaf_psc_exp or if the inputs arrive at different times.

To reproduce, run the following once with precise=True, again with precise=False:

import matplotlib.pyplot as plt
import nest

precise = True
relative_delay = 0.0  # the problem does not occur if this is positive

nest.SetKernelStatus({'off_grid_spiking': precise})

if precise:
    neuron_model = 'iaf_psc_exp_ps'
    filename = "with_ps"
else:
    neuron_model = 'iaf_psc_exp'
    filename = "without_ps"

if relative_delay:
    filename += "_delay"

neuron = nest.Create(neuron_model, 1, {'tau_syn_ex': 12.0, 'tau_syn_in': 4.0, 't_ref': 0.1})

spike_sourceD = nest.Create('spike_generator', 1)
spike_sourceR = nest.Create('spike_generator', 1)
nest.SetStatus(spike_sourceD, 'spike_times', [[40.0]])
nest.SetStatus(spike_sourceR, 'spike_times', [[40.0]])

nest.CopyModel('static_synapse', 'decay_syn', {'weight': 200.0, 'delay': 0.5})
nest.CopyModel('static_synapse', 'rise_syn', {'weight': -200.0, 'delay': 0.5 + relative_delay})

nest.Connect(spike_sourceD, neuron, syn_spec='decay_syn')
nest.Connect(spike_sourceR, neuron, syn_spec='rise_syn')

recorder = nest.Create('multimeter')
nest.SetStatus(recorder, {'withtime': True, 'record_from': ['V_m']})
nest.Connect(recorder, neuron, 'all_to_all')

nest.Simulate(200.0)

data = nest.GetStatus(recorder)[0]['events']
senders = data['senders']
times = data['times']
vm = data['V_m']

for n in neuron:
    plt.plot(times[senders==n], vm[senders==n])

plt.savefig(filename + ".png")

precise = False
without_ps

precise = True
with_ps

NEST: 2.10.0, Python: 3.4.2

Thanks to Hajime Yamauchi for the original bug report to the NeuralEnsemble Google group.

@heplesser heplesser added the T: Bug Wrong statements in the code or documentation label May 26, 2016
@heplesser
Copy link
Contributor

One ignores Donald K at ones own peril: premature optimization is the root of all evil .... The problem is that SliceRingBuffer::get_next_spike() sums all spikes arriving at exactly the same time and then returns the summed weight. In the test case, this sum is zero, thus the flat membrane potential.

This was not a problem really in the original iaf_psc_alpha_canon model, because that had only a single synaptic time constant for both excitatory and inhibitory input. iaf_psc_exp_ps allows for different time constants, but still puts all spikes into one buffer and only distinguishes between E and I spikes when the buffer is read out. At this point, it gets the summed weights of simultaneous spikes and we are in trouble.

The proper solution for this is to place excitatory and inhibitory spikes in separate buffers in the handle() method, as is done in iaf_psc_exp.

One should probably also remove the spike accumulation from SliceRingBuffer::get_next_spike(), since the probability of simultaneous spikes in a precise simulation is vanishing (spike offsets would have to agree to the last bit).

@heplesser heplesser added ZC: Model DO NOT USE THIS LABEL I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) ZP: Pending DO NOT USE THIS LABEL S: High Should be handled next labels Nov 17, 2016
@heplesser heplesser assigned heplesser and unassigned abigailm Jun 29, 2017
@heplesser heplesser added this to the NEST 2.12.1 milestone Jun 29, 2017
@terhorstd
Copy link
Contributor

Note also the discussion on buffers in multi-synapse models in the 2016-09-19-Open-NEST-Developer-Video-Conference...

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: High Should be handled next T: Bug Wrong statements in the code or documentation ZC: Model DO NOT USE THIS LABEL ZP: PR Created DO NOT USE THIS LABEL
Projects
None yet
Development

No branches or pull requests

4 participants