LIMO-EEG-Toolbox / limo_tools

Hierarchical Linear Modelling for MEEG data
https://limo-eeg-toolbox.github.io/limo_meeg/
Other
59 stars 27 forks source link

[BUG] LIMO design matrix messup at lines 156-166 function limo_glm_handling.m #137

Closed jnvandermeer closed 2 years ago

jnvandermeer commented 2 years ago

Describe the bug A clear and concise description of what the bug is.

Limo crashes with vague message : "issue with GLM design" upon estimating 1st level parameters. Upon checking the logs I came across the limo_glm_handling.m at line 164 where concatenation fails. I then reran it and tried to figure out what was going in.

There are some dark magicks happening in lines 156-166. A tmp variable is made, and then 1:2:end is used and 2:2:end (L 157 and 158). This assumes that LIMO.model.model_df is a cell array of cell arrays of dimenions 2X1. This works well.

Then at lines 162-163 the same is attempted, but now for LIMO.model.conditions_df, which is also assumed to be the same shape. In my case, they are not the same shape as model_df, but they are 1X2. The 'tmp' variable then becomes a matrix instead of a vector, and the concatenation on L164 fails, because it only can find 32 channels instead of my original 63.

I can fix this issue by forcing it to be a Nx1 vector with: tmp=tmp(:) , adding that at the end of lines 156 and 162, of function limi_glm_handling.m. Not sure if that is intended, but I don't see how it could otherwise work.

I have a feeling that this code could be made more robust by using a reshape function with the number of channels, rather than using 1:2:end, 1 and 2:2:end,: on a temporary variable that is then deleted?

I'm sorry this is not a full bug report as expected, but hopefully it illustrates the issue.

Many thanks for making the toolbox Johan

jnvandermeer commented 2 years ago

I get this when I estimate 1st level Betas, with having 2 factors in my STUDY.design, and have clicked the 'interaction model for categorical Indep. var.', to make that factorial GLM into an interaction model at first level (so I can estimate the interactions at 2nd level). If I don't click the 'interaction model' checkbox, LIMO estimates beta's with no error (but I have the wrong model at 1st level).

CPernet commented 2 years ago

Hi Johan,

CPernet commented 2 years ago

ok not as straightforward as I thought - I just computed a full factorial on 1 subject and no bug image

I did push a small patch into master to record all df, but I don't think it will solve your problem - try and if still bugs, maybe you can share 1 subject with me (with the imported data and LIMO.mat that failed) I should be able to get on from there - thx

jnvandermeer commented 2 years ago

What happened for me was that I have a STUDY in which I specify 1 factor (nback), using EEG.event.nback=1 (or 2), and then specify below that (in the STUDY create design GUI) another factor (RF), which is ON or OFF, and this I coded into EEG.event.rf being (also) 1 or 2. When I estimated in the standard way, the first lvl GLMs were factorial designs, so: [1 0 1 0; 1 0 0 1; 0 1 1 0; 0 1 0 1]; But, I wanted not to have the factorial design, but interaction design; so as to estimate interactions properly at 2nd level. So I want the design at 1st level to be: [1 0 0 0; 0 1 0 0; 0 0 1 0; 0 0 0 1]. I could click the first checkmark in the LIMO GUI to convert the factorial into the interaction design, but then the error occurred.

I tried it again with the recent eeglab version and the most recent limo_tools, and now the popup error message changes. "All of the input arguments must be of the same size and shape. Previous Inputs had size 10 in dimension 2. Input #3 has size 1. This is not a bug (Error occurred in function limo_check_ppool() at line 11."

The bug might be related to how EEGlab.event is converted into STUDY design and how LIMO pick up from that? I can share a subject (55M). How should I send it over?

jnvandermeer commented 2 years ago

Dear Cyril,

I made a dropbox transfer of my subject 1001:

https://www.dropbox.com/t/y8FFDjq9RSrFr1aP https://www.dropbox.com/l/AABSH1mKbMlHI81U-FfruhGXd9mjO8g-MmI

On Fri, Nov 4, 2022 at 12:33 AM Cyril Pernet @.***> wrote:

ok not as straightforward as I thought - I just computed a full factorial on 1 subject and no bug [image: image] https://user-images.githubusercontent.com/4772878/199749316-1c204139-ee79-4688-ac16-8637700b5e7c.png

maybe you can share 1 subject with me (with the imported data and LIMO.mat that failed) I should be able to get on from there - thx

— Reply to this email directly, view it on GitHub https://github.com/LIMO-EEG-Toolbox/limo_tools/issues/137#issuecomment-1302206581, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAUSXKBK2WQ4TDF6C2HIUJDWGPELXANCNFSM6AAAAAARV3CU34 . You are receiving this because you authored the thread.Message ID: @.***>

CPernet commented 2 years ago

'I could click the first checkmark in the LIMO GUI to convert the factorial into the interaction design, but then the error occurred.' this tick box does not exist --- what it says is to make a full factorial, ie not what you want

CPernet commented 2 years ago

I see, EEG.event.nback=1 or 2, EEG.event.rf = 1 or 2, and you want to combine them. The STUDY interface allows to combine events within EEG.event types but not across -- maybe @arnodelorme as a quick fix to do that?

CPernet commented 2 years ago

I can still look at why LIMO bugs, as indicated above I don't want the .set --> 'share 1 subject with me (with the imported data and LIMO.mat that failed)' that is just the LIMO folder (Yr.mat and LIMO.mat)

jnvandermeer commented 2 years ago

Oh I'm sorry, yes, I shall send it tomorrow. It is now 18.42 here.

Many thanks again! Johan

On Fri, 4 Nov 2022, 18:19 Cyril Pernet, @.***> wrote:

I can still look at why LIMO bugs, as indicated above I donæt want to .set --> 'share 1 subject with me (with the imported data and LIMO.mat that failed)' that is just the LIMO folder (Yr.mat and LIMO.mat)

— Reply to this email directly, view it on GitHub https://github.com/LIMO-EEG-Toolbox/limo_tools/issues/137#issuecomment-1303105205, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAUSXKC6ZH6BEJGYJ4G3C5DWGTBJZANCNFSM6AAAAAARV3CU34 . You are receiving this because you authored the thread.Message ID: @.***>

jnvandermeer commented 2 years ago

Hi Cyril, the fix you did worked, it doesn't hang anymore, so the issue is resolved. Many thanks!

To deal with the events I just rerun the preprocessing event-labeling part so I have 4 different EEG.event.type (s): A1, A2, B1 and B2. Estimating the LIMO parameters on the type then gives a good 1st level LIMO.design.X (i.e. looking diagonally across the reshuffled trials).

Am I right into believing that the full factorial design at 1st level - with the 9 columns in my case, is pretty much only used if you are looking/trying to infer from ERP/spectra of one single subject (for example across many sessions /factors), and not try to average across subjects?

CPernet commented 2 years ago

yes, the idea with full factorial is to add bootstrap and test, per subject, when this happens. Then you can have a group statistics on the percentage of subjects showing an effect (binomial test) and maybe try to get more out of neural dynamics (mean and var of onsets, although onsets are noticeably hard to determine). For the 'standard' analysis, you are good to go with a repeated measure ANOVA 2*2

jnvandermeer commented 1 year ago

Dear Cyril,

How (in LIMO) would you perform a bootstrap/test per subject? - or could you point me in the right direction? I was under the impression that bootstrapping is a 2nd level thing, as implemented in the LIMO GUI(s).

Many thanks Johan

On Sun, Nov 6, 2022 at 5:48 AM Cyril Pernet @.***> wrote:

yes, the idea with full factorial is to add bootstrap and test, per subject, when this happens. Then you can have a group statistics on the percentage of subjects showing an effect (binomial test) and maybe try to get more out of neural dynamics (mean and var of onsets, although onsets are noticeably hard to determine). For the 'standard' analysis, you are good to go with a repeated measure ANOVA 2*2

— Reply to this email directly, view it on GitHub https://github.com/LIMO-EEG-Toolbox/limo_tools/issues/137#issuecomment-1304621517, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAUSXKEPIXHG7TIBQBUN7NLWG22XTANCNFSM6AAAAAARV3CU34 . You are receiving this because you authored the thread.Message ID: @.***>

CPernet commented 1 year ago

to do a full factorial design with Bootstrap use the LIMO import - not STUDY

LIMO creates the factor and the interaction term -- you can choose to bootstrap.

Alternatively -- you can use STUDY to create all conditions and use contrasts to estimate factors' effects ; the variance partition of the model is not correct and the stat test should still be ok. Then use some dark magic :-) set LIMO.design.bootstrap to say 800, LIMO.design.status to 'to do' save and call limo_glm(4, path to LIMO file)