Merged
Conversation
…s`` a ``std::optional<unsigned int>`` so ``VarAccess`` enum is only for API purposesand started inserting default access types at callsites
* Replace queries with VarAccessDim
* VarAccess is now a wrapper class around a std::variant holding NeuronVarAccess, SynapseVarAccess and std::monostate (for the defaults) * Mostly functional aside from custom updates
…Types`` to reflect all it now does
…s`` so correct defaults will be used
* Variables always have ``CustomUpdateVarAccess`` access type * ``getDims`` should be called on variable reference not directly on underlying variable
… rather than on Var
* moved resolution of variable dimensions into adaptor classes * used this in generic initialisation and runner code * tidied up variable size calculation in runner
…mode is set on variables/variable references in model but no variables with matching shape are referenced
* ELEMENT and BATCH dimensions * VarAccess and CustomUpdateVarAccess * Var and CustomUpdateVar
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## transpiler #598 +/- ##
==============================================
- Coverage 85.76% 79.66% -6.11%
==============================================
Files 99 4 -95
Lines 13525 954 -12571
==============================================
- Hits 11600 760 -10840
+ Misses 1925 194 -1731
☔ View full report in Codecov by Sentry. |
tnowotny
approved these changes
Jan 8, 2024
Member
tnowotny
left a comment
tnowotny
left a comment
There was a problem hiding this comment.
Approving based on our general discussion of the aims and design decisions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a bit of a boring internal one that was originally going to wider reaching but got de-scoped. Basically the
VarAccessDuplicationenumeration evolved a bit organically with GeNN 4 as I started adding reductions and custom updates and, in its previous form, didn't really make sense for custom update variables as the meaning of the various dimension flags can get ignored depending on what it's attached to.This PR replaces
VarAccessDuplicationwith theVarAccessDimbitfield enumeration specifying what dimensions variables have so e.g. 'normal' neuron or synapse variable now haveVarAccessDim::ELEMENT| VarAccessDim::BATCH. However, specifying these at this level is really annoying and lots of permutations aren't valid in various locations. This PR redefinesVarAccessin terms of valid combinations ofVarAccessModeandVarAccessDimand addsCustomUpdateVarAccesswhich defines dimensions subtractively , starting with ORing together the dimensions of all the variables your custom update is referencing and then determine what shape each variable should be e.g. the standard custom update variable access isCustomUpdateVarAccess::READ_WRITEwhich is defined asVarAccessMode::READ_WRITEmeaning it inherits the OR'd together dimensionality. However,CustomUpdateVarAccess::READ_ONLY_SHARED_ELEMENTis defined asVarAccessMode::READ_ONLY | VarAccessDim::ELEMENTmeaning that it's dimensionality is the result of clearing theELEMENTbit from the OR'd together dimensionality.