Nirstorm/nirstorm

Check for negative values in process_nst_mbll.m is done on NIRS and AUX data

kdarti opened this issue · 2 comments

if any(any(sDataIn.F(sDataIn.ChannelFlag~=-1, :) < 0))
msg = 'Good channels contains negative values. Consider running NISTORM -> Set bad channels';
bst_error(msg, '[Hb] quantification', 0);
return;
end

The check for negative values is done on all data, including AUX. This is an issue when AUX channels contain negative values. Check should probably occur after NIRS data is separated from AUX data:

% Separate NIRS channels from others (NIRS_AUX etc.)
[fnirs, fchannel_def, nirs_other, channel_def_other] = ...
filter_data_by_channel_type(good_nirs, good_channel_def, 'NIRS');

Nice catch ! Fixed in #143.

Thanks for your contribution

Hi Edouard, Thanks for the fix! I don't think it is very common to use aux for other signals than triggers, but I could imagine EMG being used for example. I ran into the issue because I have IMU fusion data in the AUX channels. It was easily solved by simply disabling the check for negative values but I figured it was best to report it because this is likely to affect our customers (I work for Artinis medical systemshttps://www.artinis.com/)

Thanks for the report. I think I should also fix the detect bad channel process as it is automatically marking auxiliary channel as bad. IMU is accelerometer data? You don't have trouble with the acquisition frequency as Brainstorm require the auxiliary measurement to be sampled at the same frequency as the nirs signal ?

if they use nirstorm for analyses. Since I have your attention I might as well ask another question. I'm using nirstorm right now because we at Artinis are making a series of blog posts about toolboxes for fNIRS data analysis. The plan is to take a dataset (or multiple) and perform the same basic analysis in all toolboxes. We will then write up a separate post for each toolbox, covering the analysis and some basic information, and pros/cons about the toolbox. The idea is to also involve the developers of the toolboxes in the process. A colleague of mine has tried to contact Thomas Vincent and another developer (not sure whom), but has heard nothing back yet. Do you think there is anyone in the nirstorm development team that would be interested in taking part? It would not involve much work, the idea of having a developer there is mainly to improve the quality of the post by providing more than just our perspective on the toolbox

Oh that's a nice idea. I think you might have contacted Zhengchen Cai who is busy finishing is phd right now. Personally, I started my PhD under the supervision of Christophe Grova in September, and am currently the main maintainer of Nirstorm and I would really appreciate to be part of this project. My email is edouard.delaire@concordia.ca

As academics I know you are all busy, and making a blogpost may not be high on the list of priorities, but if someone in the team could find a little time to put aside it would be great! Best regards, Kristoffer Dahlslätt

Well, as you demonstrated, increasing the visibility of the toolbox is always a good thing as the more people use nistorm, the more likely is is that we will find new bug, or develop new features making, at the end, our research better quality :)

Best regards,
Edouard


________________________________ From: Edouard Delaire notifications@github.com Sent: Thursday, October 29, 2020 17:54 To: Nirstorm/nirstorm nirstorm@noreply.github.com Cc: Kristoffer Dahlslätt kristoffer@artinis.com; Manual manual@noreply.github.com Subject: Re: [Nirstorm/nirstorm] Check for negative values in process_nst_mbll.m is done on NIRS and AUX data (#142) Hello. That's a valid point. When you run detect bad channels; it will remove both nirs and auxiliary channels that contains negative value; that's why here it doesn't make the difference between nirs and auxiliary channels. Also, in the current version, auxiliary channels are not kept after the MBLL. The reason of that is, in our current use, auxiliary channels only contains event triggers; so we doesn't need them after we detected them. Do you have any usage of the auxiliary channels that justify the need to keep them after the mbll? if yes, I can add an option to keep them after the mbll. Best regards, Edouard — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub<#142 (comment)>, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AOTG22UPDMGKVFJKUJJZAUTSNGM4JANCNFSM4TCGMZUQ.