Speed up template similarity computing using numba#4343
Speed up template similarity computing using numba#4343tayheau wants to merge 10 commits intoSpikeInterface:mainfrom
Conversation
for more information, see https://pre-commit.ci
ce491e6 to
99985d4
Compare
|
So everything is supposed to work fine, i think i have some differences mostly due to casting but it's "minimal" in the context of a normalized distance. Just to quickly sum up i moved the computing of the support matrix out of the loop, it will be more 'memory' costly in case of different template matrix but i think since most of the time we have the same ones it's a boost here. Also more leverage of numpy views instead of copies. Im not this familiar with the numba/numpy efficiency stuff but for the union it was faster to do it the "dummy" way (mine lol) that the vectorised one. So if you guys have so rule of thumbs tips for Numba, im in ;) @samuelgarcia
|
for more information, see https://pre-commit.ci
|
Looks great as a user! I get a small ~30% speed up on initial compute, and 10x speed-up on recompute (=> trying out different methods in the gui is super fast, and hard merges are very fast). And on some real data with 800 neurons, the biggest absmax in difference is... Nice!!! |
| # We can use the fact that dist[i,j] at lag t is equal to dist[j,i] at time -t | ||
| # So the matrix can be computed only for negative lags and be transposed | ||
| if same_array: | ||
| # optimisation when array are the same because of symetry in shift |
There was a problem hiding this comment.
I'd leave the comments, the calculation is pretty confusing...
yger
left a comment
There was a problem hiding this comment.
I would be carefull, because in between, we patched a bug in the similarity due to over optimizations. We can not compute only hald the times, and the upper part of the matrix and use symmetry everywhere, otherwise this is not complete. If we symmetrize in time, we need compute for all indices
|
|
||
| if same_array: | ||
| distances[num_shifts_both_sides - count - 1, j, i] = distances[count, i, j] | ||
| distances[count, tgt_unit, src_unit] = distances[count, src_unit, tgt_unit] |
There was a problem hiding this comment.
distances[num_shifts_both_sides - count - 1, j, i] = distances[count, i, j]
| ## So we inline the function here | ||
| # local_mask = get_overlapping_mask_for_one_template(i, sparsity, other_sparsity, support=support) | ||
| for tgt_unit in range(other_num_templates): | ||
| if same_array and tgt_unit < src_unit: |
There was a problem hiding this comment.
This optimization should be removed (see #4345), because otherwise the computations are not correct
|
I would be carefull here, because we recently patched a bug in #4345 and I think this is not propagated here |


Following #4310, @chrishalcrow showed that given that the diff between two merged templates is considered small, we can approximate the distance of a megred template and a new one a as a linear function. This should allow us to speed up significantly template similarity computations for merged ones.