fNIRS / snirf

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

Local to global indexing #106

Closed samuelpowell closed 4 weeks ago

samuelpowell commented 2 years ago

If we use local indexing, that is to say that:

Is the nature of the global indexing into, e.g., /nirs(i)/probe/sourcePos3D defined by specification?

samuelpowell commented 1 year ago

@nidadu could you have a look at this and tell me if I’m missing something?

nidadu commented 1 year ago

@samuelpowell I would think that's correct from the description of /nirs(i)/probe/useLocalIndex: https://github.com/fNIRS/snirf/blob/master/snirf_specification.md#nirsiprobeuselocalindex

samuelpowell commented 1 year ago

In which case the indexing is undefined and thus there cannot be a valid implementation that uses local indexing. Whilst problematic (!) this does mean that we are able to add such a specification as a non-breaking change.

fangq commented 1 year ago

@samuelpowell, I initially added the language related to modular probe indexing, and it was refined once following the discussion in #50 opened by Morris. I agree it was not yet able to work with other field such as sourcePos3D.

The idea of having such data fields is to simplify data streaming settings where each module can work with a fixed local index/sequence to send out data packets, without needing to communicate with global indexing information. I was also hoping to squeeze some redundancy of the indexing/book-keeping by knowing the repeating nature of the modules. I admit that I was not thinking about mapping to sourcePos* where only a global index is used.

I am also aware that the majority of the use cases of SNIRF, from a DAQ perspective, are not being used as a direct output format from the firmware; instead, firmware uses some internal formats to produce the entire measurements, and then post-processed to convert to SNIRF. For modular systems, the internal packet format will likely use local indexing. Having these data fields in the snirf file could potentially simplify data conversion, but I agree additional information is needed to let local indices work with other fields.

I think there might be two possible solutions to this

  1. define a modular index to global index mapping under /nirs(i)/probe, or
  2. ask sourcePos* detectorPos* to be locally-indexed positions (based on a local reference) and add another data structure to store module positions/orientations.

the 2nd one may need more work (such as how to define module positions/orientations), but may offer the most natural mapping (thus less redundancy) to modular optode spatial mapping - nonetheless, I suspect it is the least preferred as it overlaps with typical use cases of sourcePos.

again, this feature is not fully working (so folks may have to use the global indexing approach), but the hope is that it can take out of some redundancy of a modular system and make it closer to the on-board data structure and simplify data conversion.

Feel free to keep working on this, and I will be happy to chime in.

samuelpowell commented 1 year ago

Thanks @fangq. I suspect option (1) might be the most prudent. An alternative would be to enforce a global indexing scheme, but allow the module fields to be used to record the logical layout of the system.

Since practical use of the data eventually requires a flat / global representation (even if only implicit) I am tempted by the latter. We can discuss this on the next call @rob-luke .

(As an aside, I appreciate your motivation here; at the firmware level, Gowerlabs' LUMO operates in a manner similar to that which you describe. It's all very elegant, but in practice we maintain an associated mapping to a global / flat representation at a very low level of the software stack, and I suspect most people would do the same in a practical implementation).

samuelpowell commented 3 months ago

Following discussion in May 24 meeting I propose that we remove the use of local indexing for the purposes of enumerating channels. Instead, we enforce that a global representation is always defined.

To begin, we remove the field /nirs(i)/probe/useLocalIndex from the specification.

It is accepted that we may still wish to convey a description of the modular layout of the system.

Option 1:

Option 2:

Horschig commented 3 months ago

Option 2 is a bit hacky, but since we do not only have to make a specification but also make it usable for others to work with, it is the easier option (especially when it comes to supporting the spec in toolboxes like Homer). I would prefer option 1, but pragmatically it feels to me like option 2 is the way to go (at least for now).

samuelpowell commented 3 months ago

@Horschig noted, thank you. And of course nothing prevents us from doing (2) now and (1) later. Will await some more feedback before considering a PR to this effect.

samuelpowell commented 2 months ago

In call of 10/07 it was agreed to take option (2). More generally, the details of acquisiton (insofar as this relates to the data) might be considered further in the future, but a proper design is required.