brian-team / brian2cuda

A brian2 extension to simulate spiking neural networks on GPUs
https://brian2cuda.readthedocs.io/
GNU General Public License v3.0
60 stars 12 forks source link

Check if we use `_vectorisation_idx` correctly in our templates #120

Open denisalevi opened 6 years ago

denisalevi commented 6 years ago

I just found a bug because of a wrong _vectorisation_idx. And this is not the first time. I guess I never really knew how to treat it. So this should be checked once again.

EDIT And same goes for _idx. Sometimes we have array[_idx] indexing in {{scalar code}} E.g. for Synapses(..., model="w = 2", on_pre="v_post += w") the {{scalar_code}} will have const double w = _ptr_array_synapses_w[_idx] Does this only occur when w is the same for all synapses? Otherwise we might have a bug in our implementation. And our _idx = bid * THREADS_PER_BLOCK + tid might be larger then the w array, we need to check for that (assert or if (syn_N <= #blocks*#threads) ...).

@mstimberg could you maybe give a short comment what I need to take care of when setting _idx and_vectorisation_idx or how they are intended to be used?

mstimberg commented 6 years ago

Hi, just quickly: the use of _idx is a bit explained here: http://brian2.readthedocs.io/en/stable/developer/variables_indices.html#indices There should be no _idx indexing in scalar code I think, if this is the case then some variable is either incorrectly classified as scalar, or its index is not set correctly (for scalar variables it should be set to 0).

I am a bit surprised that _vectorisation_idx leads to an error in your code, we are actually not really using this yet... The idea behind it is that we could use it to have reproducible random numbers regardless of parallelization. If a neuron contains noise or a statement refers to something like rand(), then the rand function gets passed in the, say, neuron index as _vectorisation_idx. This way, it could return the same random number for a given neuron regardless of whether the statements are executed sequentially or in parallel. In scalar code, the index should be set to a special value -1. But as I said, we are not currently making use of this feature (and I think e.g. in scalar code we might sometimes set it to 1 instead of -1) -- the rand() function simply ignores the index, we'd need a counter-based RNG like Random123 to properly do this (or pre-generate numbers and do some clever book-keeping).

denisalevi commented 6 years ago

Thanks for the link and comment. I will check again, I thought I had cases where _idx appeared in scalar code. But I can't reproduce it right now. Will check that again. And for _vectorisation_idx, I did actually use it to index p regenerated random numbers, but indexed them wrong :).

denisalevi commented 4 years ago

Check brian2 PR referenced in #174, some changes related to _vectorization_idx