adam2392 commented 3 years ago

Summary

In #734, @sappelhoff summarized the following issue that I'm facing:

He has an MRI file that he wants to store in the subject-specific freesurfer space (FreeSurfer t1w.mgz file)
apparently, this is known as fsnative space ... and in BIDS, the appropriate keyword might be individual
So he could place the T1w file in derivatives, using *_space-individual_T1w.nii with an accompanying *_space-individual_T1w.json
However, according to the spec, that T1w.json MUST contain the SpatialReference metadata --> but what would he need to put in that field as a value?

@effigies, your answer https://github.com/bids-standard/bids-specification/pull/734#issuecomment-787963573 does currently not help me to understand this :thinking:

Apart from that, it'd be more convenient for @adam2392 to actually store the rotated MRIs not in derivatives, but in raw ... and for that we'd need to:

allow the _space- entity for MRI data also in RAW (not just in derivatives)
add individual to the keywords permitted for MEG/EEG/iEEG "spaces" and "coordsystems"
(still resolve / clarify how/if to use SpatialReference for this case)
however, see @dorahermes's comment on this, that these "rotated" MRIs should be stored in derivatives, and not in RAW: https://github.com/bids-standard/bids-specification/pull/734#issuecomment-786085388

@adam2392 please chime in if I misrepresented one of your points.

Originally posted by @sappelhoff in https://github.com/bids-standard/bids-specification/issues/734#issuecomment-788038614

Example

As an example, I have the following dataset.

In *coordsystem.json the contents are:

{
    "IntendedFor": "sub-la02_ses-presurgery_space-individual_proc-freesurfer_T1w.nii",
    "iEEGCoordinateSystem": "Other",
    "iEEGCoordinateUnits": "mm",
    "iEEGCoordinateSystemDescription": "FreeSurfer Coordinate System derived from the CT, or T1 MRI scan.",
}

space-individual is "not allowed" currently? If it is, then I can submit a PR to bids-validator. This is the first low-hanging fruit.
IntendedFor is used to map this electrode coordinate back to an anatomical image for interpretation. Since this isn't back to the raw dicom NIFTI, then per @sappelhoff 's comments above, I presume this must be mapped to a FreeSurfer T1w image stored as a "derivative". Is the space-individual there then correct? Or should it be something else to be compliant? Individual doesn't have much information in the BIDS appendix. For example, i) This coordinate system requires specifying an additional, participant-specific file to be fully defined. is not clear to me exactly and ii) the idea of a SpatialReference is not clear to me where it should go. Does it go with the image sidecar json, or *coordsystem.json, or somewhere else?

Outlook

From my perspective, rn the spec works real well if I just want coordinates in MNI/fsaverage template space, cuz then everything works out how it should with space being a standard template identifier.

When things are not standard, the question then arises, what's the correct thing to do with regards to space, IntendedFor and SpatialReference?

In Nonstandard Template identifiers, it says to specify a SpatialReference, which then seems... like IntendedFor?
And does SpatialReference go in the *coordsystem.json file then, or somewhere else?

adam2392 commented 3 years ago

Apologies if perhaps I'm being overly pedantic, I'm just trying to get my data stored the right explicit way the first time, so I don't have to go back and do it again later on (possibly multiple times) :p.

dorahermes commented 3 years ago

Several thoughts here related to prior bids examples:

1) When you have an MRI, rotate it, deface it, etc in order to share it, can you put this MRI in the raw data, assuming that there is no naming conflict? This is the case in some of the BIDS-iEEG examples where the T1 is in ACPC space: https://github.com/bids-standard/bids-examples/tree/master/ieeg_motorMiller2007

2) In case 1) creates a naming conflict, it seems like this should be followed: https://bids-specification.readthedocs.io/en/stable/05-derivatives/03-imaging.html#preprocessed-coregistered-andor-resampled-volumes Question: does this issue in the bids-validator apply to this case?

3) For an individual T1 in freesurfer space, when there is no naming conflict, previous examples have used space-Other: 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 If space-individual is accepted, should these space-Other cases in the future be replaced by space-individual?

tagging @robertoostenveld

adam2392 commented 3 years ago

Restating the Goal

Almost all iEEG ppl that I know of localized electrode coordinates on CT coregistered onto a T1w image. If you coregistere onto the T1.mgz from FreeSurfer, then your coordinates are then downstream interpretable based on the atlas segmentation. If you coregister to just the "Raw" T1.nii.gz file, then you don't get much. Even if you ACPC align it, you're still lacking downstream information you would probably like.

Therefore, I presume the dominating workflow of iEEG coordinates is:

create raw T1w
FreeSurfer transforms this to the T1.mgz file of FreeSurfer
CT coregistraiton to T1.mgz
use some program to localize iEEG electrodes
save everything to BIDS

Response to @dorahermes

So if I understand correctly:

Any sort of "non-pipeline" transformations done on the raw dicom images (no naming conflict): should and can be stored in raw data. E.g. with acpcdetect, then one would store the iEEG metadata as: sub-01_ses-01_space-ACPC_coordsystem.json and then it seems the T1w image could be stored in derivatives/acpcdeface/sub-01/ses-01/anat/sub-01_ses-01_space-ACPC_T1w.nii.gz.
Based on this link you posted, FreeSurfer recon-all would fall under the categorization of a "Discrete segmentation" right? So something like the following is correct?

freesurfer/
    sub-001/
        anat/
            sub-001_space-orig_dseg.nii.gz  (these are the annotated volumes based on an atlas from FreeSurfer)
            sub-001_space-orig_dseg.json
            sub-001_space-orig_T1w.nii.gz    (this is the T1.mgz from FreeSurfer)
            etc. (hypothetically, all FreeSurfer output files, could be reorganized and renamed in this fashion)

Questions Based on the Above

is the space-orig correct, cuz it's not in https://bids-specification.readthedocs.io/en/stable/99-appendices/08-coordinate-systems.html#image-based-coordinate-systems? If it's not space-orig, then should it be space-individual? If not space-individual, is space-Other the only way? It seems weird to say the FreeSurfer T1.mgz file is derivatives/freesurfer/sub-01/ses-01/anat/sub-01_ses-01_space-Other_T1w.nii, when we know it's just a "resampled, re-oriented, and normalized" version of the original T1w image. It makes sense to allow then space-individual in both MRI and iEEG.

Assuming the above is correct, then what should the *coordsystem.json file look like?

{
    "IntendedFor": "derivatives/freesurfer/sub-01/ses-01/anat/sub-01_ses-01_space-<orig/individual>_T1w.nii",  (is htis right?)
    "SpatialReference": (is this needed then since "individual" is a non-standard template identifier and the spec says so? What should it be?)
    "iEEGCoordinateSystem": "<Other, or individual>",
    "iEEGCoordinateUnits": "mm",
    "iEEGCoordinateSystemDescription": "FreeSurfer Coordinate System derived from the CT, or T1 MRI scan.",
}

what is the space entity to use here? individual, Other, orig, something else? I'm leaning towards individual, since that is the most explicit.
what should IntendedFor be here?
what should SpatialReference be here?

robertoostenveld commented 3 years ago

When writing the EEG and iEEG specification, we discussed that electrode positions for EEG are usually based on a raw measurement, e.g. using a Polhemus, but that electrode positions for iEEG are always based on some (MRI/CT) data processing. Therefore, it is dubious to qualify iEEG electrode positions as "raw data". However, since derivatives did not yet exist, and interpretation of iEEG really needs the electrode positions, and determining electrode positions requires a lot of work that is valuable to share, it was decided (jointly by the iEEG paper authors) to store electrode positions with the raw data. I see the problem that now surfaces as a consequence of this.

However, the problem is pressing for iEEG, but not unique to iEEG. While preparing the 204 subject "MOUS" dataset we wanted to share defaced anatomical MRIs in device coordinates and in CTF coordinates (i.e. rotated and translated). We did this using the names sub-A2002_T1w.nii and sub-A2002_space-CTF_T1w.nii, and added the space-CTF nii and son files to .bidsignore.

At the time of preparing the MOUS dataset in 2018, there was nothing defined yet for BIDS derivatives. However, now that we do have derivatives for imaging and the space keyword is defined, I am actually not sure whether I would use it - in case I had to recreate the MOUS dataset. It is not clear to me how to use SpatialReference and/or IntendedFor. It is also not clear how to link between separate BIDS datasets, e.g. when I place the derivatives next to the raw dataset (which I tend to make read-only as fast as I can), instead of inside the raw dataset. E.g. if you look at this standardized research folder structure figure 1, right column, I would put both sourcedata and bidsdata in 01_primary_data and I would put different processing pipeline outcomes in 02_derived_data.

adam2392 commented 3 years ago

However, the problem is pressing for iEEG, but not unique to iEEG. While preparing the 204 subject "MOUS" dataset we wanted to share defaced anatomical MRIs in device coordinates and in CTF coordinates (i.e. rotated and translated). We did this using the names sub-A2002_T1w.nii and sub-A2002_space-CTF_T1w.nii, and added the space-CTF nii and son files to .bidsignore.

For electrodes at least, CTF is an accepted keyword for MEG electrodes.tsv, which I presume is subject-specific. The current issue with the iEEG-BIDS spec is that there is no subject-specific entity.

Is it safe to say most people are okay with adding individual as an acceptable keyword? Perhaps we can even further specify individualfs for individual derived from FreeSurfer? Therefore, there may be future additions to the specification based on alternative workflows?

It seems that adding space to the imaging modalities perhaps should be another issue altogether. I can open up one rn. I have relatively less expertise there.

WDYT @robertoostenveld ?

robertoostenveld commented 3 years ago

CTF is indeed subject specific, i.e., one subject may have his nasion at [+10 0 0], and another subject with a larger head has his nasion at [+11.5 0 0], but the nasion is always guaranteed to be along the positive x-axis.

What you are referring to with "individual" or "fsnative" as a keyword corresponds to column 5 in this table.

Just "individual" does not provide enough information to identify the coordinate system (hence the need for the other columns), but if you were to use "fsnative" it might become well-characterized, as that also specifies the location of the origin and the direction of the axes. However, I was under the impression that FS uses multiple coordinate systems, depending on which stage of the processing the volumetric or surface based data is expressed. Is "fsnative" then the last representation?

adam2392 commented 3 years ago

fsnative from my understanding is the "last representation" in the sense that if one localizes electrodes using CT coregistered to FreeSurfer's T1.mgz, then all the downstream image volumes can be overlaid, such as the a2009.aparc.mgz (Desikan and Destrieux atlas labeled image volumes), wm_aparc.mgz (white matter Desikan atlas labeled image volumes), brain_mask.mgz (brain mask), etc.

Perhaps things might be a bit confusing if one wants say the "blown up" spherical representation of the FreeSurfer brain? Otw, fsnative seems pretty good to me.

Other "coordinate systems" might be tkras, which is just the "FreeSurfer" surface RAS coordinates, rather then the RAS coordinates. Where fieldtrip refers to this. We could add fsnative and fstkras as both acceptable coordinate frames to iEEG?

schoffelen commented 3 years ago

just chiming in here, since Robert asked me to think along a bit: I think it would be good to be explicit about the 'fsnative' in the designation of the coordinate system. I would assume that this is indeed the last representation in which the electrodes' coordinates are expressed, although this would I think depend on the way their locations have been identified.

If they are identified based on the surfaces (in the freesurfer derived surf directory) AND if the implicit additional transformation -as required by the tkvox2ras vs. vox2ras discrepancy in the corresponding volumetric image's header- is not taken into account, then the coordinate system's origin will be the somewhat arbitrary anatomical location that coincides with the centre-of-volume of the anatomical image that has been used for the generation of the meshes. In that case, one could indeed call it 'tkras', or 'fstkras'.

adam2392 commented 3 years ago

Tagging @alexrockhill and @libertyh and @thebrainchain

adam2392 commented 3 years ago

It seems we now at least converged(?) to agreement that a Freesurfer volume and possibly a freesurfer surface coordinate system should be allowed for IEEG subject specific coordinate systems.

Name tbd: is fsvolume and fstkras okay? (Fsnative seems unpopular) I can make a PR unless someone objects? Maybe fsras and fstkras?

To have the T1w FS Preprocessed images in Derivatives seems fine with me. As long as the IEEG electrodes coords can be defined with eg. 'space-fsvolume'.

The other question then was what do we do with "intended for" and "spatial reference"? For both coordsystem.json and the derivative t1w?

alexrockhill commented 3 years ago

How about just t1w to make it crystal clear which space it's in? Also that doesn't really have to do with freesurfer, it's just the coordinate system of the scanner.

sappelhoff commented 3 years ago

The other question then was what do we do with "intended for" and "spatial reference"? For both coordsystem.json and the derivative t1w?

If we define some coordinates in electrodes.tsv with respect to a derivative (e.g., a FS T1.mgz), we need a way to unambiguously point to that file (as you mention, using IntendedFor, and/or SpatialReference)

That opens up another issue / current shortcoming: How to point to other datasets. derivatives/ can be placed outside of a BIDS dataset, and in that case just putting a relative path (with respect to root) into IntendedFor/SpatialReference to point to a T1.mgz does not work (because derivatives may be somewhere else locally or on the Internet).

So we also need a way to point to such "remote" datasets. Yesterday @effigies had a few suggestions such as a JSON dictionary with short "identifier" keys for the remote dataset names and their values being URIs.

Then in IntendedFor/SpatialReference one could do something like key:://relative-path/goes/here so that people know how to find the file (look up key, get that dataset, then use the relative path within that dataset).

These ideas are clearly not fully developed, but they point into the direction we need to think as well.

adam2392 commented 3 years ago

How about just t1w to make it crystal clear which space it's in? Also that doesn't really have to do with freesurfer, it's just the coordinate system of the scanner.

How would we differentiate then ras coordinates vs tkras coordinates since tkras only makes sense for Freesurfer?

For regular T1 images, I rn BIDS has ACPC (caveat is it must be the T1 but in ACPC).

adam2392 commented 3 years ago

@sappelhoff sounds like a can of worms... :( What if key::// is not uploaded? Would it just be relative to whatever bids_root then?

any suggestions on how to move forward then?

edit by SA: xref -> https://github.com/mne-tools/mne-bids/issues/737

sappelhoff commented 3 years ago

What if key::// is not uploaded? Would it just be relative to whatever bids_root then?

well that'd be the same case as if you upload a dataset with missing files, that's also something that can happen by accident and that can utterly destroy the dataset (e.g., if you forget to upload all JSON files) --> in that case the dataset is simply broken I think.

robertoostenveld commented 3 years ago

I don't think that fsvolume is appropriate. FS has multiple volumetric data representations, and this does not distinguish between them. It is also not used elsewhere, except in the FS code where it has a different meaning. fsnative is used (see google), as is fsaverage

But in any case: you should be able to define relative to the anatomy of the subject (or template) (not relative to the binary files of some "random" software) how the coordinate system is defined. Where in the subject (or template) is the origin, what are the direction of the axes, what are the units? Like MNI and TT, which are relative to the AC. That is an anatomical structure that you carry around in your head, even if there is no anatomical MRI available to point it out.

adam2392 commented 3 years ago

I don't think that fsvolume is appropriate. FS has multiple volumetric data representations, and this does not distinguish between them. It is also not used elsewhere, except in the FS code where it has a different meaning. fsnative is used (see google), as is fsaverage

Okay that makes sense. Should it be then fsnative and fsnativetkras (or fsnativesurface)?

Re orientation: It seems safe to say both are RAS, right?

Re origin: Sorry I'm a bit noob here perhaps. @robertoostenveld would you be able to explain the origin difference between fsnative (just applying the vox2ras transform) vs fsnativetkras (applying the vox2tkr transform)? In fieldtrip, it says:

center of isotropic 1 mm 256x256x256 volume

^ does this correspond to regular RAS, or surface RAS? Just trying to clarify in explicit detail to maybe make the spec describe it as explicitly as possible.

robertoostenveld commented 3 years ago

With "in fieldtrip" you probably mean "on this page of the fieldtrip website", right?

The FieldTrip code does currently not use the string freesurfer, fsnative or tkras for any coordinate system indication. It does use fsaverage at some places.

I am also a noob wrt FreeSurfer. It would be good to elaborate (first) on the FieldTrip website (or another public place) how these specific coordinate systems are defined relative to the subject (not to the binary data on disk, but the actual head; imagine sticking skewers in someone's head).

adam2392 commented 3 years ago

With "in fieldtrip" you probably mean "on this page of the fieldtrip website", right?

yep!

I am also a noob wrt FreeSurfer. It would be good to elaborate (first) on the FieldTrip website (or another public place) how these specific coordinate systems are defined relative to the subject (not to the binary data on disk, but the actual head; imagine sticking skewers in someone's head).

Okay, yeah I think to address the origin problem, I posted in Neurostars. Hopefully ppl have answers :p. Otw, the orientation is RAS I believe for everything, so once that is solved, we can move forward with the PR here.

adam2392 commented 3 years ago

Since #820 is on track to be merged, is it possible to revive this issue?

tldr: how can we store iEEG coordinates in subject-specific manner?

Summary of Issues So Far

Issues with acpc: Not everyone has data stored with ACPC alignment. Ppl in general would not want to re-run their FreeSurfer recon-all just to make sure their data is ACPC aligned. In addition, it is near "impossible" to check if one has data aligned to the ACPC by the validator. Unless there becomes a community driven algorithm that is extremely easy to use and provides consistent algorithmic alignments to the ACPC, then the ACPC alignment is also very subjective and will be error-prone if say someone provides the original dicoms and they need to align the ACPC again.

Issues with fsaverage, or any other template names: People may prefer sharing individualized coordinate brains instead of a template version.

Issues with Other: It is not descriptive enough and is just a hack to make coordinates work. This results in a tremendous uncertainty in downstream tools that want to deal with electrode coordinates in BIDS. Moreover, it's weird that we don't support a common use-case in the iEEG community.

Summary of Solutions So Far

Keep acpc as RECOMMENDED for iEEG, but specify additional things one must store in the metadata, such as the explicit AC and PC landmarks (just like NAS, LAS, etc. are for the BIDS-EEG).

Additional keywords for iEEG storage:

fsnative, or fstkras, or something along those lines: FreeSurfer specific
scanras: seems to have surfaced as a general purpose solution for this case, where it is subject-specific, encompasses FreeSurfer (but is more general), and REQUIRES one to have a pointer in their metadata to the corresponding T1 image associated with the coordinates (enabled by #820). A related discussion occurred in mne-bids

Scanner RAS for iEEG would be defined as:

origin: origin of the T1 image associated
orientation: RAS

Are there any issues with scanras for iEEG?

cc: @alexrockhill @sappelhoff @jasmainak @agramfort @larsoner @robertoostenveld

alexrockhill commented 3 years ago

Since #820 is on track to be merged, is it possible to revive this issue?

tldr: how can we store iEEG coordinates in subject-specific manner?

Summary of Issues So Far

Issues with acpc: Not everyone has data stored with ACPC alignment. Ppl in general would not want to re-run their FreeSurfer recon-all just to make sure their data is ACPC aligned. In addition, it is near "impossible" to check if one has data aligned to the ACPC by the validator. Unless there becomes a community driven algorithm that is extremely easy to use and provides consistent algorithmic alignments to the ACPC, then the ACPC alignment is also very subjective and will be error-prone if say someone provides the original dicoms and they need to align the ACPC again.

Issues with fsaverage, or any other template names: People may prefer sharing individualized coordinate brains instead of a template version.

Issues with Other: It is not descriptive enough and is just a hack to make coordinates work. This results in a tremendous uncertainty in downstream tools that want to deal with electrode coordinates in BIDS. Moreover, it's weird that we don't support a common use-case in the iEEG community.

Summary of Solutions So Far

Keep acpc as RECOMMENDED for iEEG, but specify additional things one must store in the metadata, such as the explicit AC and PC landmarks (just like NAS, LAS, etc. are for the BIDS-EEG).

Additional keywords for iEEG storage:

fsnative, or fstkras, or something along those lines: FreeSurfer specific

scanras: seems to have surfaced as a general purpose solution for this case, where it is subject-specific, encompasses FreeSurfer (but is more general), and REQUIRES one to have a pointer in their metadata to the corresponding T1 image associated with the coordinates (enabled by [ENH] Major update to pointing to files within, outside of, or remote to a current BIDS dataset #820). A related discussion occurred in [mne-bids](https://github.com/mne-tools/mne-bids/pull/859#discussion_r698690644)

Scanner RAS for iEEG would be defined as:

origin: origin of the T1 image associated

orientation: RAS

Are there any issues with scanras for iEEG?

cc: @alexrockhill @sappelhoff @jasmainak @agramfort @larsoner @robertoostenveld

Sounds good except that specifying AC and PC coordinates leaves the rotation about the axis of the AC-PC line unspecified and so there would have to be a third coordinate.

Also scanras sounds great and I think the file pointer is a really great addition; it would be ambiguous if the CT were shared as well if it were the MR or the CT scan.

dorahermes commented 3 years ago

In my experience the above definition of scanras is the most common when reconstructing iEEG electrode positions (and matches workflows in most electrode reconstruction packages). Other plusses are that it requires an IntendedFor image, and is more specific than Other.

While ACPC seems more specific, it should also continue to require an IntendedFor image in ACPC coordinates. This is because there can be discussion about the ACPC definition. While I have typically defined ACPC with the AC as origin (as in e.g. Fieldtrip https://www.fieldtriptoolbox.org/faq/acpc/) one of my neurosurgical colleagues insists that the origin is the midcommissural point. Even the location of the AC and PC may differ in workflow (see Figure 1 here https://www.lead-dbs.org/dbs-targets-in-mni-space/). Some explicit details on what BIDS defines as ACPC may be helpful.

jasmainak commented 3 years ago

I would also endorse scanras as it's what seems to be least ambiguous, automatic and lead to the simplest analysis pipeline in the tooling

adam2392 commented 3 years ago

Sounds good except that specifying AC and PC coordinates leaves the rotation about the axis of the AC-PC line unspecified and so there would have to be a third coordinate.

Any thoughts as to what this 3rd point would have to be? @alexrockhill @larsoner?

alexrockhill commented 3 years ago

Sounds good except that specifying AC and PC coordinates leaves the rotation about the axis of the AC-PC line unspecified and so there would have to be a third coordinate.

Any thoughts as to what this 3rd point would have to be? @alexrockhill @larsoner?

I think according to the coordinate frame definition, it's a point superior to the AC on the midline but there are not really specific that I have seen about it other than that (i.e. magnitude unspecified as far as I know).

robertoostenveld commented 3 years ago

Scanras sound like a good extension of one use case that now has to be dealt with the catch-all Other solution. Explicit machine-readable specifications are better than implicit specifications that require a human to type the description for other, leading to potential confusion as to that definition.

Could someone contribute a definition of scanras on https://www.fieldtriptoolbox.org/faq/how_are_the_different_head_and_mri_coordinate_systems_defined/ ? See here on github.

bids-standard / bids-specification

Storing iEEG electrodes with locations from FreeSurfer image #747

Summary

Example

Outlook

Restating the Goal

Response to @dorahermes

Questions Based on the Above

Summary of Issues So Far

Summary of Solutions So Far

Summary of Issues So Far

Summary of Solutions So Far