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

Make sure $(batch) is set to zero on CPU #523

Merged
merged 2 commits into from
May 23, 2022
Merged

Conversation

neworderofjamie
Copy link
Contributor

@neworderofjamie neworderofjamie commented May 17, 2022

Code using $(batch) previously didn't work on the single-threaded CPU backend. This PR fixes this by substituting zero for $(batch) in all CPU kernels

Fixes #513

@neworderofjamie neworderofjamie added this to the GeNN 4.7.2 milestone May 17, 2022
@codecov
Copy link

codecov bot commented May 17, 2022

Codecov Report

Merging #523 (a94754d) into master (93a50ca) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #523   +/-   ##
=======================================
  Coverage   86.96%   86.96%           
=======================================
  Files          84       84           
  Lines       18089    18091    +2     
=======================================
+ Hits        15731    15733    +2     
  Misses       2358     2358           
Impacted Files Coverage Δ
src/genn/backends/cuda/backend.cc 82.75% <ø> (-0.02%) ⬇️
src/genn/backends/single_threaded_cpu/backend.cc 56.28% <100.00%> (+0.09%) ⬆️

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 93a50ca...a94754d. Read the comment docs.

@neworderofjamie neworderofjamie requested a review from tnowotny May 17, 2022 14:14
@tnowotny
Copy link
Member

... does this imply that models with model.batch_size = 1 will now run on the CPU backend (or where batch_size is not set by the user)?

@neworderofjamie
Copy link
Contributor Author

If the reason they didn't run before was because you referenced $(batch) in your code and it didn't compile, yes they will!

@neworderofjamie neworderofjamie modified the milestones: GeNN 4.7.2, GeNN 4.8.0 May 19, 2022
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.

Yes, this is very sensible. I will test if it allows to run my eventprop code on CPU-only by simply setting batch=1 once you've merged.

@neworderofjamie neworderofjamie merged commit b1787ae into master May 23, 2022
@neworderofjamie neworderofjamie deleted the batch_zero branch May 23, 2022 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

$(batch) not present on SingleThreadedCPU backend
2 participants