Adding multiple moduleIndex for inter-module channels
Closed this issue · 6 comments
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.
@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.
While I do not totally understand modular fNIRS, this change seems fully backwards compatible and inoffensive
let me know if the above updates solve the problem
Fantastic. Works like a charm!
@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.
just created a PR, and now merged. issue closed.