-
Notifications
You must be signed in to change notification settings - Fork 57
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
Fixed nasty race condition #414
Conversation
…roupMerged into NeuronPrevSpikeTimeUpdateGroupMerged
Codecov Report
@@ Coverage Diff @@
## master #414 +/- ##
==========================================
- Coverage 87.13% 87.10% -0.03%
==========================================
Files 78 78
Lines 15245 15309 +64
==========================================
+ Hits 13283 13335 +52
- Misses 1962 1974 +12
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Classic trap. Glad you spotted it. Trying to think of more efficient solutions but coming up blank atm.
Sadly not the cause of the bug that led me to fix it but hey 😄 |
In #376 I added support for tracking 'previous' spike times. However, this generated
preNeuronResetKernel
code like this:Which has a nasty race condition in that the spike count may have been reset by the time other blocks try and access it. Presumably, this wasn't hit in our tests or the Pavlovian conditioning model because they were small enough that the whole kernel ran simultaneously. Better performing (this adds approximately 1.5 microseconds per-timestep to the Pavlovian conditioning model), more complex solutions might be possible but for now I've just split the two stages into two separate kernels which has had the side benefit of tidying up several nasty bits of the previous implementation)