-
Notifications
You must be signed in to change notification settings - Fork 35
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
[Suggestion] Making CoordinateSystem mandatory if Pos3D is present #163
Comments
@Edouard2laire , I was just looking at this coordinate system requirement in the BIDS spec today. It goes in the coordsystem.json file. I agree this would be good to get into the snirf file. And I like the way you describe the conditional requirement. I believe the validator could handle this language. But I personally wouldn't want to burden ourselves to have the SNIRF validator check if the provided coordinate system is a valid coordinate system since the BIDS validator already handles this. Unless someone volunteered to make that change to the SNIRF validator. I was thinking that in the use cases in my lab that I would use 'other' for the 'NIRSCoordinateSystem' field and then I would indicate in the 'NIRSCoordinateSystemDescription' field that our landmarks and optodes are in the same coordinate system with mm units. I wonder what other maintainers and steering committee members think? |
So you're suggesting to turn the I agree it would be better to encourage having it though. |
@HanBnrd , I think the suggestion is making /nirs(i)/probe/coordinateSystem required if 3D positions is present. And recommending, but not requiring /nirs(i)/probe/landmarkPos3D. One can always using 'other' for the coordinateSystem and indicate that positions are provided in 'mm' units or something like that. So I don't see any issue for those users who aren't aware of the coordinate. In the least, they should be using physical units for the distances if they don't know the coordinate system. |
Sure, I guess I was more thinking in the case where it's made required in snirf specs, and therefore snirf-loading libraries make it impossible to load files that don't have coordinateSystem, people who want to analyse legacy files (eg. from already published open-access datasets) and were just planning to compute distances from 3D pos will not be able to load without tweaking their snirf files. But it's probably a very small use case. Personally, I'm fine with making it a requirement, it's a good step towards standardisation I think. |
The plan is for the validator to be aware of the snirf version number, and so this should not break older snirf files, provided they have the older version number indicated in the files contents. |
I expect that the vast majority of fNIRS recordings are made in some arbitrary space with landmarks for registration. Most fNIRS software is written for this case and may be unlikely to support 'standard' spaces more commonly encountered in e.g. fMRI. Though of course, the data may move into a standard space during processing. So I'd support this so long as this 'default' case is easy to specify - I guess this is as you are suggesting @dboas ?
This need is dependent on the specific co-ordinate system? In any case, best to say less than more unless we're enforcing something or guidance is required. |
@samuelpowell , yes, you captured what I am suggesting. Looks like you and I are on the same page. Do we want to request that someone make the modification to make /nirs(i)/probe/coordinateSystem required if 3D positions is present. And recommending, but not requiring the presence of /nirs(i)/probe/landmarkPos3D? And then do a pull request. |
I can work on the PR |
Hi @samuelpowell @dboas, We met with the maintainers yesterday to chat about issues. It seems this issue would require a change in the validator as currently it's only possible to conditionally require a subfield based on the parent with |
Hello, If it is difficult to have the field required only if 3D positions are present, what about having it required all the time? |
I expect the result will be that the vast majority of SNIRF files will then feature a co-ordinate system specified as 'other'. Perhaps it would be useful to be more specific about the problem that we need to solve here? |
@dboas and I discussed this issue. David suggested making /nirs(i)/probe/coordinateSystem a recommended field for now until the validator is updated to work with measurementList and measurementLists. @dboas, the specifications have so far only designated fields as either optional or required, without using "recommended." However, I believe we can start incorporating this classification. @sam, should we update the field description to indicate that "other" is the most common option for fNIRS and recommend adding landmarks? |
Hello,
Is your suggestion related to a problem? Please describe.
The main objective of having access to Pos3D (sourcePos3D, detectorPos3D) is to be able to coregister the montage with a template or the subject anatomy. This can however become tricky without information about the coordinate system used.
Additionally, the co-registration can be made much easier with access to landmark position (nasion, left/right ear...)
Describe the solution you'd like
Have the coordinate system be mandatory if sourcePos3D and detectorPos3D are present.
Have landmarkPos3D and landmarkLabels be recommended if sourcePos3D and detectorPos3D are present.
Additional context
The description of the coordinate system is required when including nirs data in a BIDS dataset: https://bids-specification.readthedocs.io/en/stable/modality-specific-files/near-infrared-spectroscopy.html#coordinate-system-json-_coordsystemjson
The text was updated successfully, but these errors were encountered: