baccuslab / pyret

Python tools for analysis of neurophysiology data
https://pyret.readthedocs.io/en/master/
MIT License
35 stars 8 forks source link

filtertools.decompose doesn't work when product of spatial dimensions < 42 #62

Closed lmcintosh closed 8 years ago

lmcintosh commented 8 years ago

To reproduce bug, try

sta = np.random.randn(6,6,40) spatial_rf, temporal_rf = ft.decompose(sta)

The if the first two spatial dimensions of the STA are (6,7), (7,6), (7,7), (8,8), etc. it always, works, but for whenever you have spatial dimensions of (6,6) or smaller, filtertools.decompose improperly concatenates the two spatial dimensions. This results in the error "ValueError: operands could not be broadcast together with shapes (36,40) (36,)" for the case when the STA is of size (6,6,40), or "ValueError: operands could not be broadcast together with shapes (16,40) (16,)" when the STA is of size (4,4,40).

nirum commented 8 years ago

which branch are you using?

nirum commented 8 years ago

The dev branch uses flipped dimensions now, so the temporal axis is first (see https://github.com/baccuslab/pyret/pull/58 for discussion on why we did this).

Therefore (in dev) you want to pass something with the shape of (40,6,6) to ft.decompose :whale:

lmcintosh commented 8 years ago

Yeah I tested it on the master branch. I can see if I get a similar error on dev branch.

Lane McIntosh www.lanemcintosh.com

On Thu, Nov 5, 2015 at 10:57 AM, Niru Maheswaranathan < notifications@github.com> wrote:

The dev branch uses flipped dimensions now, so the temporal axis is first (see #58 https://github.com/baccuslab/pyret/pull/58 for discussion on why we did this).

Therefore (in dev) you want to pass something with the shape of (40,6,6) to ft.decompose [image: :whale:]

— Reply to this email directly or view it on GitHub https://github.com/baccuslab/pyret/issues/62#issuecomment-154153051.

nirum commented 8 years ago

ya I can reproduce the bug on master

lmcintosh commented 8 years ago

works in dev!

nirum commented 8 years ago

re-opening because it needs to get fixed in master