Skip to content
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

Local to global indexing #106

Closed
samuelpowell opened this issue Apr 16, 2022 · 9 comments · Fixed by #151
Closed

Local to global indexing #106

samuelpowell opened this issue Apr 16, 2022 · 9 comments · Fixed by #151
Assignees

Comments

@samuelpowell
Copy link
Collaborator

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

  • for each channel we set a sourceModuleIndex, and a detectorModuleIndex, and use local indices for sourceIndex and detectorIndex
  • we set useLocalIndex non-zero             

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

@samuelpowell samuelpowell self-assigned this Apr 10, 2023
@samuelpowell
Copy link
Collaborator Author

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

@nidadu
Copy link

nidadu commented May 16, 2023

@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
Copy link
Collaborator Author

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
Copy link
Member

fangq commented May 23, 2023

@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
Copy link
Collaborator Author

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
Copy link
Collaborator Author

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:

  • Retain the sourceModuleIndex and detectorModuleIndex optional fields
  • Add optional fields sourceLocalIndex and detectorLocalIndex to complete the description of a modular system

Option 2:

  • Remove the sourceModuleIndex and detectorModuleIndex optional fields, and allow manufacturers to use the source and detector labels to convey information about the modular configuration of the system, which can be parsed if required.

@Horschig
Copy link
Collaborator

Horschig commented Jun 3, 2024

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
Copy link
Collaborator Author

@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
Copy link
Collaborator Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants