search

fNIRS / snirf

SNIRF Format Specification
http://fnirs.org/resources/software/snirf/
Other
58 stars 33 forks source link

Adding multiple moduleIndex for inter-module channels #50

Closed morrisvanegas closed 3 years ago

morrisvanegas commented 3 years ago

Rather than a single module index for both source and detector: /nirs(i)/data(j)/measurementList(k)/moduleIndex,

Can we use individual optode module indices /nirs(i)/data(j)/measurementList(k)/sourceModuleIndex /nirs(i)/data(j)/measurementList(k)/detectorModuleIndex

As modular fNIRS become more abundant, inter-module channels (channels with sources and detectors on different modules) are a simple way of increasing channel density. The current snirf specification only allows for intra-module channels (ie channels where sources and detectors are on the same module since they have the same moduleIndex).

I understand that simply globally enumerating sources and detectors (regardless of modules) solves this problem, but it doesn't allow a user to study only intra- or only inter-module channels.

dboas commented 3 years ago

@morrisvanegas , this is an important point to bring up. I don't think this module aspect of SNIRF received much attention up to this point. The specification says very little about this moduleIndex.

I do understand the issue you raise. For an inter-module channel, the source would be on one module and the detector on another module. Your proposed solution would fix this issue.

@jayd1860 @fangq and @sstucker , what do you think about this? We can keep moduleIndex and just add these two additional fields as optional. If moduleIndex is defined then it implies source and detector are on the same module. If it is not defined, then either there is only one module for the whole system or the source and detector are on different modules as indicated in the sourceModuleIndex and detectorModuleIndex.

sstucker commented 3 years ago

While I do not totally understand modular fNIRS, this change seems fully backwards compatible and inoffensive

fangq commented 3 years ago

let me know if the above updates solve the problem

morrisvanegas commented 3 years ago

Fantastic. Works like a charm!

dboas commented 3 years ago

@fangq , how do we propagate this to the master? Or does this have to be in a working draft version for the next version? I guess probably the latter assuming we have already released version 1.0.

fangq commented 3 years ago

just created a PR, and now merged. issue closed.