Set delay values (in seconds) only on host

[denisalevi]

Currently all arrays set in CUDAStandaloneDevice.generate_main_source() are set first on the host and then copied to the device. But this does not make sense for the delay array (in seconds), which we do not use on the device. Instead it gets transformed into delays (as integer multiples of the simulations step) in the connectivity matrix of each SynapticPathway (happening in synapses_initialise_queue.cu). Those are then copied to the device and used. The current situation copies both delay representations to the device.

My first fast fix was to delete the device array after creation of the connectivity matrix (in synapses_initialise_queue.cu). This solves the memory issue, but introduces a bug when using multiple run() calls and changing the delay values between them (which sets the host array and copies it to the device array, which is deleted).

What should be done instead, is to catch delay variables in CUDAStandaloneDevice.generate_main_source() and modify the code snippets such, that the arrays are not copied to the device at all. The self.main_queue attribute contains the array names as they will be used in the C++ project (e.g. _dynamic_array_synapses_delay, but not anymore the Variable.name names (which would be delay). The only way I see, is to check if an array name ends with _delay or _delay_n, where n is an integer (for multiple SynapticPathways in the same Synapses object, the delay arrays are numbered). But this seems like a not very clean solution. It would be better to have access to the Variable.name attribute, but that would need overwriting a bunch of methods from the parent class CPPStandaloneDevice, where self.main_queue is filled.
@mstimberg Can I savely assume that in cpp_standalone projects delay variables and ONLY delay variables have array names ending in _delay or _delay_n with n being integer?

And additionally we would need a version of the group_variable_set... templates that does only set variables on the host. Currently they only set the device array values in a kernel call. This is happening, when delays are set with python arrays or string syntax (e.g. syn.delay = "2*ms * rand()").

And there need to be some tests for this. The bug described above with changing the delay values between run() calls was not caught by any test from the test suite.

[mstimberg]

hi @denisalevi , I agree that matching on the name is not very elegant, even on Variable.name (but we do the same for delays in brian2genn to raise an error for heterogeneous delays which GeNN does not support). That said, you should already have access to the Variable.name for an "array name", by inverting the CPPStandaloneDevice.arrays dictionary. This dictionary maps ArrayVariable objects to their array name, you want the opposite.

[denisalevi]

Thanks @mstimberg! At least with Variable.name I can check for string equality, seems a little cleaner :). And I guess I can safely assume that the CPPStandaloneDevice.arrays dictionary values are unique (since they are all global variables in the cpp project)?
And just to make sure, am I right in the assumption, that the delay variable can't be accessed (read or written) in equations or generally during a run() statement, but only through the SynapticPathway.delay attribute?

[mstimberg]

And I guess I can safely assume that the CPPStandaloneDevice.arrays dictionary values are unique (since they are all global variables in the cpp project)?

Yes, the CPPStandaloneDevice.add_array function takes care of that.

And just to make sure, am I right in the assumption, that the delay variable can't be accessed (read or written) in equations or generally during a run() statement, but only through the SynapticPathway.delay attribute?

Not quite. In principle, synaptic on_pre/on_post statements and synaptic equations can refer to the delay variable, which is a shorthand for the pre-synaptic delay. I've never seen a use for this in the wild, though.

[denisalevi]

With 335c42a, the delay array in device memory is freed in synapses_initialise_queue.cu. This allows larger network simulations for our benchmarks, but introduces bugs when the delay variable is accessed in Synapses code or changed between multiple runs (as described above). I added a warning for multiple runs.

It might be, that accessibility to the delay variable in Synapses code will be removed from brian2 side (brian-team/brian2#927). If this is not done when fixing this issue, we need to check if delay is in Synapses code, the same way as is described for lastupdate in #87.

I added tests for changing the delay between run() calls. They should pass once this issue and #86 are fixed.

When fixing this issue, undo the changed in 335c42a.

[denisalevi]

Access to delay variable in Synapses code is removed with PR brian-team/brian2#931

[denisalevi]

Current implementation (will be merged with PR #300): The delay (in seconds) is available on host and device, because setting the delay (with group_variable_set... templates) happens on the device. With PR #299 copying of modified variables back to the host is fixed (that means the delay will be copied back to the host when set on the device). This is required when changing the delay between run calls, since this new delay will then be used to reinitialize the synapses and spike queues (which happens on the host). We now also check if the delay value (in seconds) will be accessed in device code and we delete that array on the device only if it is not accessed! This fixes the bug when changing the delay between runs but also keeps memory demands low when this is not happening.

In this issue I originally wanted all to happen on the host, but that would require having group_variable_set... templates that run on the host. And it's not clear if that would be even beneficial when setting delay values for many synapses, so this solution is fine I think.

We can get rid of all the device to host copies if we implement synapse generation and spike queue initializations on the device.

Closing this with PR #300

[denisalevi]

Follow-up issue for spike loss when changing delays between run calls is #302