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

[FIX] 1) Clarify appropriate labels for space entity, 2) Clarify channels+electrodes do not have to match #734

Merged
merged 8 commits into from
Mar 5, 2021

Conversation

sappelhoff
Copy link
Member

@sappelhoff sappelhoff commented Feb 13, 2021

minor clarifications concerning iEEG, MEG, EEG:

  1. make it more explicit, that labels for the _space-<label> entity MUST come from a restricted list of keywords
    1. clarify where this list can be found
  2. clarify/explain that EEG channels.tsv and electrodes.tsv do not have to correspond
    1. edits to the examples for channels.tsv and electrodes.tsv to make them "talk about the same data" and serve to make the previous point obvious

NOTE: Also contains a large discussion on the space entity, and its lack of support in "raw BIDS" MRI data (supported only for "derivatives" MRI data currently), and more ...

@adam2392
Copy link
Member

adam2392 commented Feb 15, 2021

One question I have is that if space must be taken from a list of the accepted keywords and might be tied to an anatomical image via the IntendedFor key, then how can one differentiate different "spaces" of T1w images?

Say I have iEEG data in space-fsnative, that is IntendedFor for a T1w image that is aligned to FreeSurfer (via recon-all; the T1.mgz essentially). However, there are other T1w files in other spaces, say just from the original raw dicoms (possibly ACPC aligned). If one wants to differentiate various *_electrodes.tsv files for which image that are coordinates in these two different image spaces, then:

  1. how does one proceed to annotate and name the electrodes/coordsystem files?
  2. How does one differentiate the T1w images?

Related https://github.com/bids-standard/bids-validator/issues/1059#event-4325876812

@sappelhoff
Copy link
Member Author

This is a good question that I stumbled over as well. I don't know how this was envisioned when the space entity was introduced, cc @dorahermes. Please all tag other people that you want to weigh in on this ... I feel like this is an important issue to sort out.

However, there are other T1w files in other spaces, say just from the original raw dicoms (possibly ACPC aligned).

so you have the same scan twice, but just in two different coordinate systems?

how does one proceed to annotate and name the electrodes/coordsystem files?

It is indeed inconvenient that *_T1w.nii anatomical scans do not allow for the space entity 🤔 (https://github.com/bids-standard/bids-validator/issues/1059#event-4325876812)

Otherwise it'd be simply

  • *_space-ACPC_electrodes.tsv
  • *_space-ACPC_coordsystem.json with IntendedFor pointing to *_space-ACPC_T1w.nii

and for your fsnative

  • *_space-Other_electrodes.tsv
  • *_space-Other_coordsystem.json with IntendedFor pointing to *_space-Other_T1w.nii

NOTES

  • I don't know if fsnative is actually not supported in the appendix VIII ... or if it's there just under a different name
  • if it's not supported, you'd have to use Other, as I do in the example
  • BUT that reveals a new issue: imagine that for the first files you don't have the valid keyword ACPC, but some other non-supported coordsystem like fsnativeX ... then you would have to define that as Other too, which makes two Other files that would then have to be distinguished using some other entity like acq --> and that can soon become hacky.

Ideas

  • if fsnative is (i) often used, (ii) truly not supported currently, THEN --> let's make it supported (i.e., add it to the keywords in appendix VIII), if viable (I cannot judge)
  • make the space keyword permissible for anatomical scans ... otherwise we need to use acq in some hacky way (or am I missing something in my example?)
  • For the last issue under my NOTES section (the case with two Other spaces) ... I guess this occurs rarely to never --> and if it does, we can always discuss adding a new valid keyword to appendix VIII. so it's okay. WDYT?

@adam2392
Copy link
Member

adam2392 commented Feb 15, 2021

However, there are other T1w files in other spaces, say just from the original raw dicoms (possibly ACPC aligned).

so you have the same scan twice, but just in two different coordinate systems?

Yes, I think this is common and possible. It is for me at least. My workflow would entail i) receiving dicoms from clinicians and converting to nifti, (original "scanner" space if you will) ii) do some preprocessing (robustfov via FSL and acpcdetect perhaps now in the ACPC space), and then iii) do some reconstruction, say via FreeSurfer (now fsnative space). Ideally I have all 3 nifti's in my dataset for full transparency of the workflow. In addition, I might be interested in obtaining now electrode coordinates in the ACPC and fsnative space.

Now, rises our problem :/.

NOTES

  • I don't know if fsnative is actually not supported in the appendix VIII ... or if it's there just under a different name
  • if it's not supported, you'd have to use Other, as I do in the example
  • BUT that reveals a new issue: imagine that for the first files you don't have the valid keyword ACPC, but some other non-supported coordsystem like fsnativeX ... then you would have to define that as Other too, which makes two Other files that would then have to be distinguished using some other entity like acq --> and that can soon become hacky.

To my reading of the BIDS spec, it is not included. It is mentioned in the individual keyword. However, tbh I have no idea what this means (perhaps someone w/ more T1 exp can help). It seems to imply that this is a coordinate system for the specific image for a subject? If so, how is it different from scanner space? In addition, why does it mention fsnative? Not all participant specific anatomical spaces are rendered through FreeSurfer?

individual | Participant specific anatomical space (for example derived from T1w and/or T2w images). This coordinate system requires specifying an additional, participant-specific file to be fully defined. In context of surfaces this space has been refered to as fsnative.

Ideas

  • if fsnative is (i) often used, (ii) truly not supported currently, THEN --> let's make it supported (i.e., add it to the keywords in appendix VIII), if viable (I cannot judge)

+1 if not in there.

  • make the space keyword permissible for anatomical scans ... otherwise we need to use acq in some hacky way (or am I missing something in my example?)

+1 unless there is an alternative fix for this kind of thing. Then perhaps, it'd be useful to add to the spec for BIDS-iEEG?

  • For the last issue under my NOTES section (the case with two Other spaces) ... I guess this occurs rarely to never --> and if it does, we can always discuss adding a new valid keyword to appendix VIII. so it's okay. WDYT?

Yes agreed. I would be surprised if I ever needed two space-Other files.

@sappelhoff
Copy link
Member Author

Questions:

  1. is fsnative an "individual" anatomical space? If yes, we cannot add it under standard templates, right?
  2. The individual space seems interesting, but somewhat tied to "derivatives", as it references the SpatialReference field, which is in the derivatives spec
  3. How do you achieve your workflow under current constraints?

@adam2392
Copy link
Member

  1. is fsnative an "individual" anatomical space? If yes, we cannot add it under standard templates, right?

I suppose yes then... I suppose then the FreeSurfer "derived" files of an individual subject T1w scan is considered a derivative. But to be fair, if I instead use the FreeSurfer MNI template (i.e. fsaverage), then technically, it's still a derivative image that you use to localize the ieeg electrodes.

  1. The individual space seems interesting, but somewhat tied to "derivatives", as it references the SpatialReference field, which is in the derivatives spec

Yep, not sure what the right answer here is...

  1. How do you achieve your workflow under current constraints?

I basically "hack" acquisition entity since "space" is not allowed, or I add space, but I use .bidsignore to get by the bids-validator (which I suppose is bad practice :p).

@adam2392
Copy link
Member

Current Summary

I think the issues are the following:

  1. is FreeSurfer derived files (e.g. the T1.mgz file) considered derivatives, or raw data? It's rotated and converted to a 256^3 cube, but the data itself remains the same. Can it be stored with the raw data?
  2. How can one reference the T1w file in the FreeSurfer space in the *_coordsystem.json for electrodes.tsv and also correctly specify it in the name?

Old solution

For coordsystem.sjon/electrodes.tsv use space-Other, and coordsystem='Other' and specify IntendedFor=<fpath to image>. This runs into a clash when you have multiple images (e.g. ACPC aligned, and then coordinates in the T1w FreeSurfer space).

For the actual anat/*_T1w.nii files, this cannot be resolved to my knowledge

Proposed solutions

  1. expand allowed usage of space entity
  2. expand allowed keywords of coordinatesystems (e.g. fsnative), or clarify usage of individual keyword for this workflow.

@sappelhoff
Copy link
Member Author

@adam2392 given that your https://github.com/bids-standard/bids-validator/pull/1190 was merged and the validator now properly enforces that the label for _space-<label> comes from the list of restricted keywords, I think it'd be good to get this small clarification PR merged.

Your issue is still not resolved, but I think it may be good if you could write brief a summary and open a new issue, and then tag all people who have to say something about this in order to move forward.

The alternative is that you keep hacking acq, or start using "Other" or /derivatives more ... (but please no .bidsignore 🤣 )

@sappelhoff sappelhoff requested a review from adam2392 February 25, 2021 16:00
@sappelhoff
Copy link
Member Author

@hoechenberger see also this discussion

@dorahermes
Copy link
Member

I typically consider transformations to be derivatives, like in fMRI. For my studies, my raw data have only one _electrodes.tsv file in T1w space with the T1 scan indicated in the IntendedFor field (and maybe an MNI space one).

When I convert the electrode coordinates to several other T1w spaces (acpc alligned/fsnative etc), I consider this derived data (similar as fMRI coregistration/reslicing is a derivative).

@adam2392
Copy link
Member

Thanks for clarifying @dorahermes !

So for the sake of analyzing iEEG data, for most of our purposes, there is no reason to have electrode coordinates in the original scanner space, because we can't do much w/ it. Overlaying it in the fsnative (FreeSurfer T1w.mgz for that subject) is much more useful. In this case, I should have:

To allow fully traceable interpretation of the electrode coordinates that is BIDS-compliant? Am I missing anything?

I'm just having a hard time fitting our lab's workflow into BIDS in this current context and I can't decipher how to best proceed based on the spec :/

@sappelhoff
Copy link
Member Author

an *_space-other_electrodes.tsv and coordsystem.json file in the BIDS dataset along w/ my raw iEEG data

this is sensitive to capitalization, so it'd have to be space-Other, not space-other

in the coordsystem.json, I should have IntendedFor: derivatives/freesurfer/sub-/mri/T1.mgzandiEEGCoordinateSystem: 'individual'`?

individual is currently not a valid keyword ... but if individual truly corresponds to fsnative (FreeSurfer T1w.mgz for that subject), then we should add individual as a valid keyword

note that in that case, your space-Other would have to be space-individual, because the space and CoordinateSystem fields should correspond

where does one store the SpatialReference spoken about here?

I don't really know what this does. @effigies can you help us?

@effigies
Copy link
Collaborator

effigies commented Mar 1, 2021

where does one store the SpatialReference spoken about here?

I don't really know what this does. @effigies can you help us?

SpatialReference is metadata for the image that is in the space of another image. So you might have sub-01/func/sub-01_space-anat_bold.json in your root that says:

{
    "SpatialReference": "/sub-01/anat/sub-01_T1w.nii.gz"
}

(Or similar. I didn't check if I did that properly...)

Is that helpful, or am I misunderstanding the question?

Copy link
Collaborator

@effigies effigies left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM. I think there's a typo though.

sappelhoff and others added 2 commits March 1, 2021 15:35
Co-authored-by: Chris Markiewicz <effigies@gmail.com>
@sappelhoff
Copy link
Member Author

Okay, this only leaves us to resolve @adam2392's problem. Let me try to summarize:

  • 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 #734 (comment) does currently not help me to understand this 🤔


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:

  1. allow the _space- entity for MRI data also in RAW (not just in derivatives)
  2. add individual to the keywords permitted for MEG/EEG/iEEG "spaces" and "coordsystems"
  3. (still resolve / clarify how/if to use SpatialReference for this case)
  4. however, see @dorahermes's comment on this, that these "rotated" MRIs should be stored in derivatives, and not in RAW: [FIX] 1) Clarify appropriate labels for space entity, 2) Clarify channels+electrodes do not have to match #734 (comment)

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

@sappelhoff
Copy link
Member Author

notes from maintainers meeting:

  • @adam2392 can you share a directory tree / dataset that people can play around with? to gain some practical insight into the problem?
  • "spatial reference" field should point to "T1mgz" file for the _space-individual case
  • All MRIs with a space need to be placed in derivatives

@effigies effigies added this to the 1.6.0 milestone Mar 2, 2021
@sappelhoff
Copy link
Member Author

I'll interpret Chris' review from #734 (review) as an approval, merging.

@adam2392 if you wanna solve your issue in some way, please open a targeted issue 👍

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

Successfully merging this pull request may close these issues.

EEG - confusion between channels.tsv and electrodes.tsv
4 participants