Ring buffer saturation implementation request

[rjames91]

I've noticed the method for saturating the ring buffer value that is in the processing of fixed synapses isn't implemented here.

I've created a version that copies the saturation check that is used in the fixed synapses then by adding a pointer to the same saturation check counter in the process plastic synapses function arguments we can add the detected plastic synapse saturations to the total count. Let me know if you think this would be suitable.

I'm slightly concerned that without the saturation method in place we could experience some unreported overflow problems in STDP experiments.

[alan-stokes]

here is a bit ambigious. i persume your on about palstic synapses vs fixed. i havent seen the pr for this, but my question is, do you want a seperate provance tracker for palstic vs fixed? or a total? my inclination is a seperate one, but me not expert

[rjames91]

Yes apologies I thought the issue would come under "synapse_dynamics_stdp_mad_impl.c"

I don't really mind about whether they would be separated for reporting, it's more a comment about whether or not the C code needs saturation of the plastic ring buffer entries to 0xFFFF.

[mmikaitis]

Yeah I think Rob is correct. This can overflow on VERY high rates. Hence the comment that Dave suspects overflow: https://github.com/SpiNNakerManchester/sPyNNaker/blob/6897ef022017079c4eb1eb488cb36190a5f8a4b6/neural_modelling/src/neuron/plasticity/stdp/synapse_dynamics_stdp_mad_impl.c#L276

But I think saturation was left out because STDP is a critical loop where every instruction counts and it is quite hard to overflow.

[rowleya]

The current scaling makes it almost certain that it will overflow at some point, depending on how long you run it for. Though it is a critical loop, it is unlikely that the couple of extra instructions will compare to the number of instructions in the plasticity itself, thus I would advise that this is added as suggested.

[rjames91]

After looking at the STDP mechanism with the current weight scaling in a bit more detail I'm unsure if using the 16bit fixed point multiplier along with scaled values for the alpha_plus and alpha_minus constants are going to produce valid results.

https://github.com/SpiNNakerManchester/sPyNNaker/blob/6897ef022017079c4eb1eb488cb36190a5f8a4b6/neural_modelling/src/neuron/plasticity/stdp/weight_dependence/weight_additive_one_term_impl.h#L68-L73

The multiplier seems to use a constant fixed point reference of 11 defined in: https://github.com/SpiNNakerManchester/sPyNNaker/blob/6897ef022017079c4eb1eb488cb36190a5f8a4b6/neural_modelling/src/neuron/plasticity/common/stdp_typedefs.h#L12

It looks like the exponential LUTs are scaled to this fixed point number system (S4.11), and therefore so are the generated a_plus and a_minus state values that are used in the multiply. The state weight region a_plus and a_minus constants however appear to have the fixed point in a variable position depending on the calculated weight scale, lets say it's 2^14. The output from a int16 fixed point multiply in this example would be in the form S 4+1.11+14, then finally it performs a >> 11 operation to give an output in the expected form of S4.11. In this example however we would need to shift right by 14 bits to get this result. What I believe may be happening is some of the decimal bits from this multiply may be later interpreted as integer bits.

I'm not entirely confident with this theory so I'm expecting to have got something wrong along the way so let me know what you think.

Rob

[mmikaitis]

I think this is fine because one wants scaled_a2_plus at this point to be in weight format and not in STDP format anymore (i.e. this is called just before adding this new weight to ring buffer, which has the same format as weight_region->a2_plus/minus).