Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bad behavior if letter case of eventtype name is incorrect (in cfg.eventypes) #124

Open
olafdimigen opened this issue Sep 10, 2021 · 3 comments

Comments

@olafdimigen
Copy link
Member

olafdimigen commented Sep 10, 2021

uf_designmat looks for 'eventstypes' but using strcmpi(), which ignores case.

Thus, when an event is specified with a wrong case, e.g.
cfgDesign.eventtypes = {'sTiMuLuSoNsEt'} % should be 'stimulusOnset'

then the modeling process does start (no error produced initially), but mayhem happens later at the uf_glmfit stage (without producing an explicit error)

@olafdimigen
Copy link
Member Author

olafdimigen commented Sep 10, 2021

Changed to be case-sensitive in commit 1a38d40

@behinger
Copy link
Member

this is breaking change, so in principle we should bump Unfold Version to 2.0

@olafdimigen
Copy link
Member Author

Tested this a bit more deeply now.

I agree that this is potentially breaking, but it looks like it can't break anything that is not already severly broken.
If an existing user did use an incorrect letter case for any event in the model, they either...

(1) got an immediate error (but this only happens if there is only this one predictor-event in the model!), or
(2) got incorrect betas (i.e. zeros for the predictors associated with this event) or,
(3) uf_glmfit() seemingly runs forever

Finally, I also saw weird warnings by the solver, but unfortunately cannot reconstruct that scenario (may be the same as the "runs forever" one).

So the downstream consequences of case-insensitivy are bad. For example, the non-time-expanded DM will still look okay, initially (with entries in all columns) but only before uf_epoch is run, which removes the epochs for the misspelled events. For Xdc, colums will be zeroed for the misspelled event.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants