fNIRS loading issue with read_raw_bids

[HanBnrd]

Description of the problem

With fNIRS files (in SNIRF format) mne_bids.read_raw_bids is sorting channel names differently than channel locations, leading to mixed up channel distances. This can be reproduced for example using data from this tutorial.

Steps to reproduce

import mne
import mne_nirs
import mne_bids

root = mne_nirs.datasets.audio_or_visual_speech.data_path()
dataset = mne_bids.BIDSPath(
    root=root,
    suffix="nirs",
    extension=".snirf",
    subject="04",
    task="AudioVisualBroadVsRestricted",
    datatype="nirs",
    session="01",
)
raw = mne_bids.read_raw_bids(bids_path=dataset)
print(mne.preprocessing.nirs.source_detector_distances(raw.info))

Expected results

We'd expect the same result as using mne.io.read_raw_snirf

raw = mne.io.read_raw_snirf(dataset.fpath)
print(mne.preprocessing.nirs.source_detector_distances(raw.info))

Actual results

mne_bids.read_raw_bids is sorting channels differently than locations and the channel distances are mixed up.

Additional information

Seems to only be an issue when SNIRF files have a specific structure, which is the case for the dataset of the linked tutorial.

[welcome]

Hello! 👋 Thanks for opening your first issue here! ❤️ We will try to get back to you soon. 🚴🏽‍♂️

[HanBnrd]

This is related to #1117. However in this case, unlike #1117, it fails silently.

[sappelhoff]

Thanks for the report @HanBnrd. Would you be willing to contribute a fix for this issue?

[HanBnrd]

Actually I can't reproduce the #1117 error with the latest package versions, did anything change since then? Seems channels.tsv have the same channel order as the snirf when I load fnirs_motor_group now. However they are different when loading audio_or_visual_speech.

Shouldn't having different channel order in the metadata and snirf file actually return an error?

I suspect maybe the fnirs_motor_group dataset has been fixed, but also the loading of inconsistent dataset is now no longer returning an error and silently mixing up channel names (it's using the ones from the tsv) and locations (it's using the ones from the snirf).

[sappelhoff]

Actually I can't reproduce the #1117 error with the latest package versions, did anything change since then?

interesting, yes maybe -- we'd need to dig into the commit history a little. Perhaps we failed to close that issue appropriately, or it was fixed in another library (mne?)

Shouldn't having different channel order in the metadata and snirf file actually return an error?

at least a warning, I think.

I suspect maybe the fnirs_motor_group dataset has been fixed

I don't think this is the case. That data is downloaded via this function: https://mne.tools/mne-nirs/stable/_modules/mne_nirs/datasets/fnirs_motor_group/fnirs_motor_group.html#data_path

And in the code, this points to this dataset: https://github.com/rob-luke/BIDS-NIRS-Tapping

Which has not been updated in >2 years.