Flipped dimensions

[nirum]

Flips the expected stimulus dimensions in filtertools

Now, stimuli are expected to be of the form (time, space, space) or (time, space) or (time,). Having the time axis come first means that scanning over that dimension is quicker (due to the way contiguous data is stored on memory). A quick test using 100x100 dim. stimuli and 40 time points indicates that this switch makes computing an STA roughly 20% faster. :dash:

Note: to see these improvements, you need to update how you are storing the stimulus arrays on disk.

[nirum]

@bnaecker take a look and let me know what you think. This will break old code, so that's why I made it a PR.

@lmcintosh and @pjadzinsky, any thoughts?

[bnaecker]

Also, just poking around the docs suggest they've not been updated. E.g., they say that filtertools.getste returns an array, but it's an iterator now.

[nirum]

@bnaecker re: your first comment, using ... as indices in a slice preserves the shape of the rest of the indices. For example:

>>> x = np.random.randn(3,4,5)
>>> print(x[:2, ...].shape)
(2, 4, 5)

re: your second comment, I'm not using np.outer. I'm using the low-level BLAS function for computing an outer product and adding it to an existing matrix, this is faster than using np.outer.

re: your third comment, Yep I haven't updated the docs. I'll do that

[bnaecker]

@nirum Maybe I put the comment at the wrong line or something, but my first comment is not about about any ellipsis indexing, but the -1 semantics in reshaping. I'm talking about line #143 in filtertools.py. The line I see is:

np.linalg.svd(f.reshape(-1, f.shape[-1]) - np.mean(f), full_matrices=False)

I assume we need to collapse all spatial dimensions of the possibly 3D filter, f, into one axis of the resulting matrix, leaving time on the other, yes? If that's true, then I think this should be flipped, no?

Check it:

>>> x = np.arange(24).reshape(2, 3, 4)
>>> x.shape
(2, 3, 4)
>>> x.reshape(-1, x.shape[-1]).shape # this is the reshaping now on line 143
(6, 4) # this groups axes 0 and 1, which are time and space respectively
>>> x.reshape(x.shape[0], -1).shape
(2, 12) # this groups axes 1 and 2, which are both space

Don't we want the latter?

Other comments are all gravy.

[nirum]

Oh ya good finding. I'll fix that too

we should really write some tests

[nirum]

ok! I fixed the reshape bug in lowranksta :bug: :boom:

and added documentation :scroll: for the new getste, getsta, and getstc methods

[bnaecker]

Vunderbar! I don't see anything else, so I'm OK with merging the request when you feel satisfied @nirum. Check with @lmcintosh and @pjadzinsky, though.