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

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

Closed
kdarti opened this issue Oct 28, 2020 · 2 comments · Fixed by #143
Closed

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

kdarti opened this issue Oct 28, 2020 · 2 comments · Fixed by #143

Comments

@kdarti
Copy link

kdarti commented Oct 28, 2020

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');

@kdarti kdarti changed the title Check for negative values in process_nst_mbll.m is also done on AUX data Check for negative values in process_nst_mbll.m is done on NIRS and AUX data Oct 28, 2020
Edouard2laire added a commit to Edouard2laire/nirstorm that referenced this issue Oct 29, 2020
@Edouard2laire
Copy link
Collaborator

Nice catch ! Fixed in #143.

Thanks for your contribution

@Edouard2laire
Copy link
Collaborator

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.

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

Successfully merging a pull request may close this issue.

2 participants