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

`PopulationRateMonitor` can give wrong results for subgroups #772

mstimberg opened this Issue Nov 10, 2016 · 0 comments


None yet
1 participant

mstimberg commented Nov 10, 2016

So I happened to come across this quite severe bug which can lead a monitor to give incorrect results (without any error) for all codegen targets except numpy. It's about a PopulationRateMonitor recording from a subgroup that does not start of the beginning of the main group. The logic for checking which spikes to include in the rate was wrong. More precisely: if a PopulationRateMonitor was supposed to record spikes from neurons with index start <= index < end and not a single spiking neuron had an index >= start, then all spikes with index < end were counted (instead of just counting zero spikes).

Here's a toy example that shows the bug:

    >>> G = NeuronGroup(4, '''do_spike : boolean''', threshold='do_spike')
    >>> G.do_spike = [True, True, False, False]
    >>> rate_1 = PopulationRateMonitor(G[:2])
    >>> rate_2 = PopulationRateMonitor(G[2:])
    >>> run(defaultclock.dt)
    >>> rate_1.rate  # correct
    <ratemonitor_1.rate: array([ 10.]) * khertz>
    >>> rate_2.rate  # wrong!
    <ratemonitor_2.rate: array([ 10.]) * khertz>

This is obviously the worst kind of bug (wrong result, no error or other indication that something went wrong), however it applies to a quite specific situation (and involves PopulationRateMonitor which is probably the least used type of monitor) and therefore did hopefully not affect that many simulations.

The fix is ready (will open a PR in a sec), I just wanted to put it here as an issue so that it is clearly documented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment