Glm improvement

[thomas-vincent]

• Add possibility to enter external regressors in the 1st GLM, as a second process input
• Add process to extract short channels (to later use them as external GLM regressors)
• Add explicit process to temporally crop data

[Edouard2laire]

Thx I'll review it as soon as possible.

Quick note :

• Can we still use the GLM with only one input?
• why adding a process to extract short channels to use it as a regressor in the GLM when it is already removed from the signal during the MBLL computation?

I also saw that you moved back the process to wip, I don't think that's a good idea as we moved the GLM process intro the distribution for the mini-course we had in Montreal. Also, I am not very familiar with the concept of reviewing, but shouldn't you have to wait for me to review the change before merging it?

Best,
Edouard

[thomas-vincent]

Hello Edouard, I put you as reviewer just to let you know about the changes instead of merging "silently" ;-) It is more methodoligically sound to use the short channel as regressors in the GLM. That way it is taken into account in the degrees of freedom. The other issue is that the MBLL is not always perform before the glm if you go through surface reconstruction from dOD. Last, doing a regression while doing the MBLL is not very modular. The MBLL process is doing many different things. I did not see it pass in fact. I think we should remove this short channel regression from the MBLL. We can even have a dedicated process to regress out any signal (could respiration belt or accelerometer). But we should encourage embedding regression from external regressors within the GLM. Finally, I think we can still use the GLM with one input. In any case, I will do a new pass very soon on everything and write some more utests and doc. Le ven. 6 mars 2020 09 h 20, Edouard Delaire <notifications@github.com> a écrit :

…

Thx I'll review it as soon as possible. Quick note : - Can we still use the GLM with only one input? - why adding a process to extract short channels to use it as a regressor in the GLM when it is already removed from the signal during the MBLL computation? I also saw that you moved back the process to wip, I don't think that's a good idea as we moved the GLM process intro the distribution for the mini-course we had in Montreal. Also, I am not very familiar with the concept of reviewing, but shouldn't you have to wait for me to review the change before merging it? Best, Edouard — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#115?email_source=notifications&email_token=AAE2GDSFOQZ3VYB3Z264JRLRGEBCDA5CNFSM4LCRE3X2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOBQE3I#issuecomment-595788397>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAE2GDSXJOMVVMB4X3DHTYLRGEBCDANCNFSM4LCRE3XQ> .

[Edouard2laire]

Finally, I think we can still use the GLM with one input.

I just tested. The process now only appears if you put the files in the process2 box; which is not intuitive if you are only using one file (and have no external output)

it has been implemented by @zhengchencai in the two following processes that are still in the 'wip' category: https://github.com/Nirstorm/nirstorm/blob/master/bst_plugin/process_nst_dOD_SSR.m and https://github.com/Nirstorm/nirstorm/blob/master/bst_plugin/process_nst_mbll_SSR.m.

I was going to merge them with the 'official' one soon

[thomas-vincent]

Ok, now I remember Zhengchen told about some custom MBLL but I did not catch it was short channel regression. As I mentionned, it is preferable to use another process to do the regression out : either in the GLM or in a dedicated process. Then we can get rid of the MBLL_SSR version.

…

On Fri, Mar 6, 2020 at 10:46 AM Edouard Delaire ***@***.***> wrote: it has been implemented by @zhengchencai <https://github.com/zhengchencai> in the two following processes that are still in the 'wip' category: https://github.com/Nirstorm/nirstorm/blob/master/bst_plugin/process_nst_dOD_SSR.m and https://github.com/Nirstorm/nirstorm/blob/master/bst_plugin/process_nst_mbll_SSR.m . I was going to merge them with the 'official' one soon — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#115?email_source=notifications&email_token=AAE2GDWUTVAIBRUYHVU4YKTRGELDTA5CNFSM4LCRE3X2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEOBZXJA#issuecomment-595827620>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAE2GDVAD2PQTTUSHJ66BBLRGELDTANCNFSM4LCRE3XQ> .