-
Notifications
You must be signed in to change notification settings - Fork 92
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
Sample NIRS data not read, problem with channel naming #1117
Comments
I might be wrong, but it seems that the channels in the raw file (*.snirf) and the *channels.tsv file are in different order. |
To tell if this is a MNE-Python or MNE-BIDS problem (it probably is not a MNE-NIRS problem actually!), can you try just |
I believe the problem is that channel names are in different order in the snirf file and the channels.tsv file:
|
Hello! 👋 Thanks for opening your first issue here! ❤️ We will try to get back to you soon. 🚴🏽♂️ |
FYI @rob-luke @sappelhoff I've transferred this here because I think it's probably an issue with how MNE-BIDS handles channel names. My guess is that the dataset natively in SNIRF has some order that |
Thanks for triaging, @larsoner
yes, that's unfortunate ... for EEG, iEEG, and MEG it's a RECOMMENDATION that the order of channels in the raw file and in
yet for NIRS, I don't even see this recommendation. Was this deliberately omitted @rob-luke, or is that something we should add post-hoc? Not that the recommendation would really cut any work for us, as it's not a REQUIREMENT and currently we seem to be handling this as if it were, leading to this issue 🤔 |
@sappelhoff do you expect it's "just" a matter of sorting the dataframe (or list or whatever) we get from reading the TSV by |
That's what I am thinking right now, but it's been a long time that I've looked into that specific code. |
I explored this issue a bit more using my own data. I went from the data in original NIRx format to MNE raw, to snirf and finally to BIDS format. It looks all fine with regard to the channel names. I seems that the channels.tsv file provided with sample data is the problem. When I read MNE-NIRS sample data with Below the code to create a new channels.tsv from a snirf file.
|
Hi @loew, thanks for reporting this issue. I believe this is an unintended consequence of mne-tools/mne-python#10642, when MNE changed the way it ordered channels. Can you post a link to the dataset you are testing so I can verify if this is indeed the issue and how we can fix it. Thanks |
Hi @rob-luke, which dataset are you refering to? The ones that have the issue are part of the MNE-NIRS sample data. |
@rob-luke I believe you can find the MWE in the original post, your help in resolving this would be appreciated! |
To recap, the problem is that you may have channels When reading, MNE-BIDS tries to rename each channel to meet the channels.tsv definition, but renaming Proposal for a fix: Do two passes when populating the BIDS channel names
|
By this do you mean some |
@larsoner I edited my above comment to add more clarity as to how I understand the issue, could you please check if this description makes sense to you? |
I think we have an ambiguous situation here in case we have an order mismatch between raw and channels.tsv In my mind we would keep the original order in raw and assign the names stored in channels.tsv But another possible approach could be to re-order the channel data in raw such that it matches channels.tsv |
I still don't quite follow what you mean by "assign" here. You could mean |
Would it? I think it's one of two valid approaches. In fact, this was actually what I expected would happen when I first thought about this mismatch. |
Thinking about |
Yes, but think they have named the channels in raw
and realized they swapped TP9 and TP10, they may want to fix the problem by editing channels.tsv:
(this is what I believe happened in the fNIRS example, but I might be mistaken) But as I said, it's ambiguous what to do here. |
I don't think so. A while ago we used to enforce writing fNIRS channels in a specific order. I'm guessing they wrote the
Coming back to the spec as posted above, this at least mentions the idea of an order mismatch: Is there somewhere in the spec that says |
unfortunately there is still no official BIDS policy on whether the raw data or the BIDS metadata should be preferred in case of a mismatch. This is the relevant issue: Also, I am very unhappy that this sentence from the spec as linked by Eric above is a "SHOULD" and not a "MUST" 😕
|
Mismatch in order of names, or in set of names? I see in the spec where the former could happen (the SHOULD stuff), but not where the sets of names could differ. |
my first comment pertains to mismatch in set names, my second comment pertains to the order only (where a mismatch is apparently "fine but not recommended") |
Yikes so there's no real way to know. Maybe we should raise an error but allow people to pass |
+1 |
Describe the bug
Sample NIRS data in the BIDS format can not be read due to some problems with channel names. This applies to the data set in the example below, but can be replicated with the data "Auditory Speech and Noise" dataset.
Upgrading
mne
,mne_bids
andmne_nirs
from the latest official versions to the dev versions did not resolve the issue.If the
channels.tsv
file in the subjects directory is renamed (channels.tsvxxx
), the data can be read.Steps to reproduce
Expected results
I expect the data to be read.
Actual results
Additional information
The text was updated successfully, but these errors were encountered: