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

Lookup data structures not taking batching into account #507

Merged
merged 1 commit into from
Mar 18, 2022

Conversation

neworderofjamie
Copy link
Contributor

I've been trying to use custom updates to do more of the resetting style stuff in mlGeNN and everything was breaking as batch size increased. Turns out this was totally broken as, for custom updates targetting neurons, the indices within which threads binary search to find their population were generated as if batch size = 1.

None of your models use custom updates on duplicated (i.e. batched) neuron group variables do they @tnowotny?

@codecov
Copy link

codecov bot commented Mar 17, 2022

Codecov Report

Merging #507 (6ebdc0d) into master (a84bfa9) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #507      +/-   ##
==========================================
- Coverage   87.04%   87.03%   -0.01%     
==========================================
  Files          84       84              
  Lines       18104    18108       +4     
==========================================
+ Hits        15758    15761       +3     
- Misses       2346     2347       +1     
Impacted Files Coverage Δ
include/genn/genn/code_generator/backendSIMT.h 96.51% <ø> (ø)
src/genn/backends/cuda/backend.cc 82.76% <100.00%> (ø)
src/genn/backends/opencl/backend.cc 91.59% <100.00%> (ø)
src/genn/genn/code_generator/backendSIMT.cc 96.06% <100.00%> (+0.01%) ⬆️
src/genn/backends/cuda/optimiser.cc 72.77% <0.00%> (-0.18%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a84bfa9...6ebdc0d. Read the comment docs.

Copy link
Member

@tnowotny tnowotny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good - cannot say yet whether it makes a difference for eventprop. Potentially not (would have been too convenient)

@neworderofjamie neworderofjamie merged commit 09b74db into master Mar 18, 2022
@neworderofjamie neworderofjamie deleted the batched_neuron_custom_update branch March 18, 2022 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants