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

Extra Global Parameter references #583

Merged
merged 13 commits into from
Jun 1, 2023
Merged

Extra Global Parameter references #583

merged 13 commits into from
Jun 1, 2023

Conversation

neworderofjamie
Copy link
Contributor

@neworderofjamie neworderofjamie commented May 1, 2023

My per-timestep EventProp loss implementation required extra global parameters to be shared between a neuron group and a custom update which, as we have talked for a while, is not an uncommon requirement. However, all my attempts to design some sort of extra global parameter sharing system ended up looking very messy so, instead, here I added the ability to reference extra global parameters in custom updates. This is:

  • Simple - the existing system for updating pointers in merged groups can handle it without modification so it's totally transparent to the lower levels of GeNN
  • Familiar - the syntax is almost identical to variable references so
  • Safer - by forcing custom updates to be involved, it discourages users from trying to use these to connect neuron groups together in weird ways that will probably fail silently and not-deterministically

A model with EGP refs might look like:

custom_update_model = genn_model.create_custom_custom_update_class(
    "custom_update",
    var_refs=[("x", "scalar")],
    extra_global_param_regs=[("e", "scalar*")],
    update_code="""
    if($(id) == (int)round(fmod($(t), 10.0))) {
        $(e)[$(id)] = 1.0;
    }
    else {
        $(e)[$(id)] = 0.0;
    }
    """)

Then you might add it to a model and configure the EGP reference like:

model.add_custom_update("CustomUpdate1", "Group", custom_update_model,
                        {}, {}, {"x": genn_model.create_var_ref(pop, "V"}, 
                        {"e": genn_model.create_egp_ref(pop, "E")})

where pop is some sort of population with a V state variable and an EGP called E. Note that, in this useless example, the x var reference is totally unused - it's just there to define the 'size' of the custom update.

@neworderofjamie neworderofjamie added this to the GeNN 4.9.0 milestone May 1, 2023
@codecov
Copy link

codecov bot commented May 1, 2023

Codecov Report

Patch coverage: 77.77% and project coverage change: -0.06 ⚠️

Comparison is base (ef69e59) 88.79% compared to head (1648e37) 88.74%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #583      +/-   ##
==========================================
- Coverage   88.79%   88.74%   -0.06%     
==========================================
  Files          76       76              
  Lines       12428    12490      +62     
==========================================
+ Hits        11036    11084      +48     
- Misses       1392     1406      +14     
Impacted Files Coverage Δ
include/genn/genn/customUpdateInternal.h 0.00% <ø> (ø)
include/genn/genn/modelSpec.h 100.00% <ø> (ø)
src/genn/genn/models.cc 88.32% <53.84%> (-8.08%) ⬇️
include/genn/genn/models.h 68.75% <75.00%> (+2.08%) ⬆️
src/genn/genn/customUpdateModels.cc 93.75% <83.33%> (-6.25%) ⬇️
...enn/genn/code_generator/customUpdateGroupMerged.cc 90.14% <85.71%> (-0.16%) ⬇️
include/genn/genn/code_generator/groupMerged.h 83.18% <100.00%> (+0.35%) ⬆️
include/genn/genn/code_generator/modelSpecMerged.h 99.19% <100.00%> (ø)
include/genn/genn/customUpdate.h 94.91% <100.00%> (-0.47%) ⬇️
src/genn/genn/customUpdate.cc 94.96% <100.00%> (+0.48%) ⬆️

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@neworderofjamie neworderofjamie marked this pull request as ready for review May 30, 2023 16:53
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.

Seems a straightforward addition

@neworderofjamie neworderofjamie merged commit e684c1f into master Jun 1, 2023
@neworderofjamie neworderofjamie deleted the egp_ref branch June 1, 2023 10:52
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