search

bids-standard / bids-examples

A set of BIDS compatible datasets with empty raw data files that can be used for writing lightweight software tests.
http://bids-standard.github.io/bids-examples/
173 stars 136 forks source link

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

Closed sappelhoff closed 3 years ago

sappelhoff commented 3 years ago

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:

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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:

https://github.com/bids-standard/bids-examples/blob/a3057982f7170b23151151a966843cce56346e3c/ieeg_motorMiller2007/sub-fp/ses-01/ieeg/sub-fp_ses-01_space-Talairach_coordsystem.json#L1-L7

dorahermes commented 3 years ago

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 commented 3 years ago

ds000117

"Elekta/Neuromag" => "ElektaNeuromag" PR: https://github.com/bids-standard/bids-examples/pull/216

ds000246

"MEGCoordinateSystem":"ctf" Are all the fields case sensitive? If this is the problem: https://github.com/bids-standard/bids-examples/pull/217

ds000248

"ctf" => "CTF" PR: https://github.com/bids-standard/bids-examples/pull/218

ieeg_epilepsy / ieeg_epilepsy_ecog

Renamed files: space-MNI152 => MNI152NLin6Sym Fixed iEEGCoordinateSystem: "MNI152" => "Other" PR: https://github.com/bids-standard/bids-examples/pull/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 commented 3 years ago

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 commented 3 years ago

Please see https://github.com/bids-standard/bids-specification/pull/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 commented 3 years ago

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

Thanks everybody for your feedback!