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

Read only vars #247

Merged
merged 10 commits into from
Jul 24, 2019
Merged

Read only vars #247

merged 10 commits into from
Jul 24, 2019

Conversation

neworderofjamie
Copy link
Contributor

@neworderofjamie neworderofjamie commented Jul 21, 2019

This change allows you to specify whether state variables are readonly when you define a model e.g. in the ``IzhikevichVariable (using syntax similar to that suggested in #55):

class IzhikevichVariable : public Izhikevich
{
public:
    DECLARE_MODEL(NeuronModels::IzhikevichVariable, 0, 6);

    SET_PARAM_NAMES({});
    SET_VARS({{"V","scalar"}, {"U", "scalar"}, 
              {"a", "scalar", VarAccess::READ_ONLY}, {"b", "scalar", VarAccess::READ_ONLY}, 
              {"c", "scalar", VarAccess::READ_ONLY}, {"d", "scalar", VarAccess::READ_ONLY}});
};

Knowing whether state variables are read-only is useful in two ways:

  • Immediately: to remove unnecessary writes of neuron and postsynaptic; and pre and postsynaptic post weight update model state variable back to global memory (to some extent aiding Refine global - register -global transfers #55) - depending on dumbness of CUDA compiler this could reduce memory bandwidth and reduce register pressure
  • In future: this allows all kinds of useful 'analysis' e.g. to determine if state variables can be optimised out either in the style of GLOBALG
    (this is something that I think is done in the PyNN, SpineML and, I suspect, Brian frontends) or by the long-dreamed-of Izhikevich style re-sampling

@codecov
Copy link

codecov bot commented Jul 21, 2019

Codecov Report

Merging #247 into master will increase coverage by 0.02%.
The diff coverage is 86.76%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #247      +/-   ##
==========================================
+ Coverage   85.42%   85.44%   +0.02%     
==========================================
  Files          45       45              
  Lines        6319     6342      +23     
==========================================
+ Hits         5398     5419      +21     
- Misses        921      923       +2
Impacted Files Coverage Δ
include/genn/genn/neuronModels.h 80% <ø> (ø) ⬆️
include/genn/genn/code_generator/codeGenUtils.h 95.45% <ø> (ø) ⬆️
include/genn/genn/weightUpdateModels.h 43.75% <0%> (ø) ⬆️
.../genn/genn/code_generator/generateSynapseUpdate.cc 100% <100%> (ø) ⬆️
src/genn/genn/code_generator/codeGenUtils.cc 93.06% <100%> (ø) ⬆️
src/genn/backends/cuda/backend.cc 88.22% <100%> (ø) ⬆️
...c/genn/genn/code_generator/generateNeuronUpdate.cc 96.98% <100%> (+0.13%) ⬆️
src/genn/genn/modelSpec.cc 63.1% <100%> (ø) ⬆️
src/genn/genn/code_generator/generateInit.cc 97.71% <100%> (ø) ⬆️
include/genn/genn/initSparseConnectivitySnippet.h 40% <33.33%> (ø) ⬆️
... and 2 more

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 d69d597...0431fb6. Read the comment docs.

* We were previously using the same ``Var`` struct for extra global params as well where readonlyness is currently meaningless - added separate ``EGP`` struct
* Changed boolean for readonly to ``VarAccess`` enum
@neworderofjamie neworderofjamie merged commit 4af1e7d into master Jul 24, 2019
@neworderofjamie neworderofjamie deleted the read_only_vars branch July 24, 2019 17:24
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