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

What is the coordinate system for these datasets (see list)? #215

Closed
9 tasks done
sappelhoff opened this issue Dec 3, 2020 · 8 comments · Fixed by #226
Closed
9 tasks done

What is the coordinate system for these datasets (see list)? #215

sappelhoff opened this issue Dec 3, 2020 · 8 comments · Fixed by #226

Comments

@sappelhoff
Copy link
Member

sappelhoff commented Dec 3, 2020

The following datasets currently all fail validation due to wrong coordinate system naming:

we should fix them.

As a reminder, the coordinate systems appendix in the BIDS specification describes the permissible values for the *CoordinateSystem* fields.

Until recently, we did not validate this field properly, so many BIDS datasets have erroneous entries there. The lack of "errors" has also made people believe that all sorts of values can be used for *CoordinateSystem* fields, so I imagine that this may come as a surprise for some.

We have several options:

  • some datasets may be in an accepted coordinate system, but the value in the JSON field is just wrong --> these will be easy to correct
  • other datasets may be in an as-of-yet unaccepted coordinate system, here we have two options:
    • either we expand the "coordinate systems appendix" to turn the unaccepted into and accepted system
    • or we convert the coordinate system of the data to something that is accepted

I would be very happy for input from @dorahermes and @ftadel regarding the ieeg datasets. Do we need to extend the BIDS "coordinate systems" appendix to cover more acceptable fields for iEEG?

@ftadel may also be able to help with ds000246 and ds000247.

I'd also like to loop in @robertoostenveld for this discussion for his experience with many data formats and coordinate systems.

@sappelhoff
Copy link
Member Author

For those who are generally interested in coordinate systems in BIDS, I have created a wiki page that summarizes the status quo and also lists currently open questions and issues. --> https://github.com/bids-standard/bids-specification/wiki/Coordinate-Systems-for-MEG-EEG-iEEG

It's a WIKI, so please add and/or correct items!

@dorahermes
Copy link
Member

I am not sure how this coordsystem description from ieeg_motorMiller2007 fails the validator, sorry if I am missing something obvious:

content or coordsystem:
{
"IntendedFor": "/derivatives/surfaces/sub-bp/ses-01/anat/sub-bp_pial.surf.gii",
"iEEGCoordinateSystem": "ACPC",
"iEEGCoordinateUnits": "mm",
"iEEGCoordinateSystemDescription": "Coordinate system with the origin at anterior commissure (AC), negative y-axis going through the posterior commissure (PC), z-axis going to a mid-hemisperic point which lies superior to the AC-PC line, x-axis going to the right",
"iEEGCoordinateProcessingReference": "Hermes et al., 2010 JNeuroMeth",
"iEEGCoordinateProcessingDescription": "surface_projection_Hermes"
}

Validator schema:
"properties": {
"IntendedFor": { "type": "string", "minLength": 1 },
"iEEGCoordinateSystem": { "type": "string", "enum": ["Pixels", "ACPC", "Other"] },
"iEEGCoordinateUnits": { "$ref": "common_definitions.json#/definitions/CoordUnits" },
"iEEGCoordinateSystemDescription": { "type": "string", "minLength": 1 },
"iEEGCoordinateProcessingDescription": { "type": "string", "minLength": 1 },
"iEEGCoordinateProcessingReference": { "type": "string", "minLength": 1 }
},

@sappelhoff
Copy link
Member Author

I am not sure how this coordsystem description from ieeg_motorMiller2007 fails the validator, sorry if I am missing something obvious:

some fields in the motorMiller2007 dataset have values like Talairach. See for example:

{
"IntendedFor": "/derivatives/surfaces/sub-Talairach/ses-01/anat/sub-Talairach_hemi-R_pial.surf.gii",
"iEEGCoordinateSystem": "Talairach",
"iEEGCoordinateUnits": "mm",
"iEEGCoordinateSystemDescription": "Talairach space RAS",
"iEEGCoordinateProcessingReference": "Miller et al., 2007 JNeuroMeth"
}

@dorahermes
Copy link
Member

I see, so Talairach is in this accepted list, but is not an accepted format for the coordsystem.json schema.

There are iEEG datasets that will only share/have available atlas based coordinates, and I think it would be best to allow the fields from the appendix.

@ftadel
Copy link
Contributor

ftadel commented Dec 5, 2020

ds000117

"Elekta/Neuromag" => "ElektaNeuromag"
PR: #216

ds000246

"MEGCoordinateSystem":"ctf"
Are all the fields case sensitive?
If this is the problem: #217

ds000248

"ctf" => "CTF"
PR: #218

ieeg_epilepsy / ieeg_epilepsy_ecog

Renamed files: space-MNI152 => MNI152NLin6Sym
Fixed iEEGCoordinateSystem: "MNI152" => "Other"
PR: #219

There are iEEG datasets that will only share/have available atlas based coordinates, and I think it would be best to allow the fields from the appendix.

I'm not sure I understand the logic of not supporting the same list of identifiers for -space and for the CoordinateSystem.
Right now we have to set "CoordinateSystem: Other" even if we have precise information about the coordinate system we are using.

@sappelhoff
Copy link
Member Author

There are iEEG datasets that will only share/have available atlas based coordinates, and I think it would be best to allow the fields from the appendix.

I'm not sure I understand the logic of not supporting the same list of identifiers for -space and for the CoordinateSystem.

To be clear: I am not opposed at all to the idea that all entries in the table at the bottom of appendix VIII would be valid fields. It's just that so far the specification does not say that. And when I updated the validator recently to behave according to the spec, these issues turned up.

To me this suggests that we should go back to the specification and clarify several parts, making things like MNI152NLin6Sym or Talairach "officially" (as in ... how it's written) accepted.

Thanks for opening the PRs @ftadel I'll take a look at them soon!

@sappelhoff
Copy link
Member Author

Please see bids-standard/bids-specification#700 where I suggest some changes to the bids-specification that will turn the current problems into non-problems. Together with @ftadel's fixes we'd then have solved all problematic datasets I listed in this issue.

@sappelhoff
Copy link
Member Author

These issues have now been solved in a series of PRs to spec, validator, and examples.

Thanks everybody for your feedback!

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

Successfully merging a pull request may close this issue.

3 participants