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

"T1w" as keyword for <datatype>*CoordinateSystem #661

Open
sappelhoff opened this issue Jul 2, 2020 · 15 comments
Open

"T1w" as keyword for <datatype>*CoordinateSystem #661

sappelhoff opened this issue Jul 2, 2020 · 15 comments
Labels
bug Something isn't working consistency Spec is (potentially) inconsistent EEG Electroencephalography iEEG MEG Magnetoencephalography

Comments

@sappelhoff
Copy link
Member

sappelhoff commented Jul 2, 2020

https://github.com/bids-standard/bids-examples/blob/23ab390fdfb4d38b3e3eeabdbd663e2ac09a16bc/eeg_ds000117/sub-01/eeg/sub-01_coordsystem.json#L2-L11

the T1w keyword seems to indicate that all coordinates must be interpreted in the light of the T1 weighted MRI image that is supplied with the IntendedFor field.

However, such a convention is currently not documented in Appendix 8: https://bids-specification.readthedocs.io/en/latest/99-appendices/08-coordinate-systems.html#coordinate-systems-applicable-to-meg-eeg-and-ieeg

@sappelhoff sappelhoff changed the title "T1w" as keyword for <datatype>*Coordinatesystem "T1w" as keyword for <datatype>*CoordinateSystem Jul 2, 2020
@sappelhoff sappelhoff transferred this issue from bids-standard/bids-examples Nov 4, 2020
@sappelhoff sappelhoff added bug Something isn't working consistency Spec is (potentially) inconsistent EEG Electroencephalography iEEG MEG Magnetoencephalography labels Nov 4, 2020
@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!

@adam2392
Copy link
Member

adam2392 commented Dec 3, 2020

iEEG user here: I think it would be very helpful to have T1w as a keyword for the CoordinateSystem because you generally end up localizing electrodes and mapping them to a T1w image. The current keywords essentially only account for if you ACPC align your T1w image before mapping ieeg coordinates to the image.

However, there are cases where this description imo is insufficient. For example, if I want to map iEEG coordinates onto the T1w defined from FreeSurfer, it seems that a T1w keyword with IntendedFor = T1.mgz (from FreeSurfer) would be more sensible.

@sappelhoff
Copy link
Member Author

you generally end up localizing electrodes and mapping them to a T1w image.

so are the electrode coordinate units then in "voxels"? or still in "mm", but mapped on the T1w image?

Perhaps it'd make sense to do something like:

  • *CoordinateSystem can be T1w, if the electrode coordinates are aligned with the T1w image
  • but in that case, the IntendedFor field is required and must point to the corresponding T1w image
  • in that case, units MAY also be in voxels ... otherwise they must be in mm, cm, or m

also, let's not forget what @robertoostenveld said in his comment here: #520 (comment)

@robertoostenveld
Copy link
Collaborator

if it were expressed in voxels, it would also have to be made explicit whether voxel indices are counted with a 0-offset or with a 1-offset. I.e., is the first corner of the volume [0,0,0] or [1,1,1]? If it were expressed in mm, that would not be needed, as it is taken care of by the low-level NIFTI file definition.

@sappelhoff
Copy link
Member Author

if it were expressed in voxels, it would also have to be made explicit whether voxel indices are counted with a 0-offset or with a 1-offset. I.e., is the first corner of the volume [0,0,0] or [1,1,1]?

indeed, and for the MRI AnatomicalLandmarkCoordinates field, 0-indexing is prescribed: https://bids-specification.readthedocs.io/en/latest/04-modality-specific-files/01-magnetic-resonance-imaging-data.html#anatomical-landmarks

@dorahermes
Copy link
Member

We typically express coordinates in mm, but mapped onto a T1 image.

iEEGCoordinateUnits currently does not have the option for voxels.

@sappelhoff
Copy link
Member Author

Taken from @robertoostenveld's comment: #520 (comment)

Having said that: I don't see a problem in the specification as T1w. It could also be a CT or a T2w or whatever file the IntendedFor 3D scan is linking to.

I think I agree with this, but it also renders T1w (or T2w, CT, ...) non-helpful. I think instead it could/should be something generic like Image. and if Image is specified as *CoordinateSystem, then an IntendedFor MUST be specified that links to the image file that defines the reference of electrodes.tsv.

WDYT?

also, if you are reading this, please also check out bids-standard/bids-examples#215

@adam2392
Copy link
Member

adam2392 commented Dec 6, 2020

I think I agree with this, but it also renders T1w (or T2w, CT, ...) non-helpful. I think instead it could/should be something generic like Image. and if Image is specified as *CoordinateSystem, then an IntendedFor MUST be specified that links to the image file that defines the reference of electrodes.tsv.

WDYT?

This makes sense to me and imo is more informative then just "Other" which is currently how one encodes any arbitrary coordinate system for iEEG. Allowing for CT is also important since technically your ieeg coordinates are usually localized on that image.

@sappelhoff
Copy link
Member Author

Allowing for CT is also important since technically your ieeg coordinates are usually localized on that image.

With the proposal from #661 (comment) you would encode CT by supplying Image as the CoordinateSystem and then point to a CT file with IntendedFor. Of course, that CT file would have to be put into .bidsignore until CT support is officially part of BIDS.

Coming back to:

in that case, units MAY also be in voxels ... otherwise they must be in mm, cm, or m

That wouldn't work for CT would it? Or is CT also measured in voxels? I get the impression that we should not support voxels for this after all ... and perhaps deprecate the only place where it's currently allowed (MRI AnatomicalLandmarkCoordinates field)

@dorahermes
Copy link
Member

I think that allowing Image and IntendedFor and having coordinates in mm units would suffice for iEEG (also for a CT in .bidsignore).

In terms of processing, we get either mm coordinates or voxel indices from a CT.nii.gz and there is just 1 matrix transformation between these two. For consistency, it is not necessary to allow a new unit (voxels) for iEEG and mm/cm/m is sufficient.

@sappelhoff
Copy link
Member Author

I am planning to open a PR soon that will address (1) an Image keyword to be used in conjunction with IntendedFor, and (2) allowing the keywords like Talairach etc. from the table.

However, I have some questions/comments about the intro text to the table:

The transformation of the real world geometry to an artificial frame of reference is described in CoordinateSystem. Unless otherwise specified below, the origin is at the AC and the orientation of the axes is RAS. Unless specified explicitly in the sidecar file in the CoordinateUnits field, the units are assumed to be mm.

  1. The first sentence will need some clarification - in particular that the keywords listed in the table are valid entries to *CoordinateSystem.
  2. the second sentence says that "the origin is at the AC and the orientation of the axes is RAS" --> is the AC a point in space? I thought that was more of a "line"? --> furthermore are there entries in the table at all that do not specify origin and axes orientations? ... I think I'd prefer it if each of the listed coordinate systems would clearly define the origin and axes orientations.
  3. the third question states that the default value for CoordinateUnits is mm. However, given that CoordinateUnits is a mandatory field that only accepts mm, cm, and m, I think we can cut that sentence.

In terms of processing, we get either mm coordinates or voxel indices from a CT.nii.gz and there is just 1 matrix transformation between these two. For consistency, it is not necessary to allow a new unit (voxels) for iEEG and mm/cm/m is sufficient.

okay, +1 to not use voxels then, and I'll think about deprecating using "voxels" as a unit in the MRI AnatomicalLandmarkCoordinates for consistency in the spec.

@sappelhoff
Copy link
Member Author

see #700

@ftadel
Copy link
Collaborator

ftadel commented Feb 5, 2021

+1 for allowing space-Image, and enforcing IntendedFor in this case.

To avoid any ambiguity, we could limit this use case to the scanner-based coordinates, as defined in the qform matrix of the .nii file (eg. the coordinates we see in the title bar of MRIcron).
Since both the NII and MGZ file formats provide well documented solutions to define one unique 4x4 transformation matrix between these scanner coordinates in mm and the voxels, I'm not sure we need to add any additional flexibility.

"space-Other" + iEEGCoordinateSystemDescription would lead to a lot of inconsistencies across datasets, and interpretation problems.

Examples:
https://github.com/bids-standard/bids-examples/blob/master/ieeg_epilepsy_ecog/sub-ecog01/ses-postimp/ieeg/sub-ecog01_ses-postimp_space-Other_coordsystem.json
https://github.com/bids-standard/bids-examples/blob/master/ieeg_epilepsy/sub-01/ses-postimp/ieeg/sub-01_ses-postimp_space-Other_coordsystem.json

@robertoostenveld
Copy link
Collaborator

is the qform in the NII file always in scanner-based coordinates?

@ftadel
Copy link
Collaborator

ftadel commented Feb 5, 2021

is the qform in the NII file always in scanner-based coordinates?

Well, it can be anything because it can be edited after acquisition.
But we can call it "a scanner-based" coordinate system.
And it has the advantage of almost always being defined, and unique. It is proper to a single file, and not comparable with other files even in the same study, but it doesn't matter.

https://nifti.nimh.nih.gov/nifti-1/documentation/nifti1fields/nifti1fields_pages/qsform_brief_usage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working consistency Spec is (potentially) inconsistent EEG Electroencephalography iEEG MEG Magnetoencephalography
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants