search

treder / MVPA-Light

Matlab toolbox for classification and regression of multi-dimensional data
MIT License
70 stars 35 forks source link

update allocation of memory in case of non-square neighbourhoods #20

Closed schoffelen closed 3 years ago

schoffelen commented 3 years ago

ENH - placeholder change to allow for non-square neighbourhoud matrices in case of cfg.append = true

schoffelen commented 3 years ago

Hi Matthias, happy new year! I was wondering whether you'd be willing to merge this PR to get my local spielerei in sync again with the main branch of your repo. It remains a challenge to explain to my collaborators to check out the correct branch of the code for their purposes, and I'd rather point them to your main branch rather than to some obscure version of mine.

treder commented 3 years ago

Thank you, and Happy New Year to you, too. You raise a point I didn't consider, I implicitly assumed that the neighbourhood matrix is always square. I think for non-square matrices the code breaks irrespective of cfg.append, so this probably needs fixing further up in the code.

I can do that, but first a question in this regard: If it is not square, the rows represent the 'new' features and the columns represent the 'old' features, correct, or is it the other way round? For instance, for a 30 x 64 matrix of EEG electrode neighbours can I assume that 64 is the original number of electrodes and 30 is the new number? In this example, would your issue be fixed if I allocate a dimension of size 30 instead of 64?

schoffelen commented 3 years ago

Indeed the convention would be that the number of rows is the 'new' feature, and the number of columns would be the 'old' feature. Specifically, for spatial type of dimensions, it could be used to combine across planar gradiometers / magnetometers belonging to the same chips, going from Nx3 to N sensors.

treder commented 3 years ago

I've done some modifications to train/test naive bayes and mv_classify. It's now (hopefully) better integrated into mv_classify and the code looks simpler, no reshaping of the data required any more. I've designed a few unit tests which run fine, can you please double check it with your own code / from FieldTrip to make sure it doesn't break any more?

schoffelen commented 3 years ago

Will do, hopefully soon. Do I need to pull your master branch for this?

treder commented 3 years ago

Yes I just pushed it into master.

treder commented 3 years ago

Sorry I hadn't pushed mv_classify yet - can you pull again

schoffelen commented 3 years ago

No worries, I hadn't started yet...

schoffelen commented 3 years ago

Hi Matthias, I pulled your master branch, and it seems to run through fine. I didn't check the code in detail, but at the moment I would be fine with that. I will close this PR, since it has become redundant. Thanks for your efforts and the great toolbox.