[FIX] Revise template-generated coordinate systems#213
[FIX] Revise template-generated coordinate systems#213oesteban wants to merge 177 commits intobids-standard:derivativesfrom
Conversation
sappelhoff
left a comment
There was a problem hiding this comment.
all points in the overview you give sound good to me, but I am not familiar enough with the material to review the nitty gritty.
You need to fix the table pipes for Travis to pass
edickie
left a comment
There was a problem hiding this comment.
OK looks good. Two things:
-
I'm OM with deprecating all the density-specific tags BUT I do think we need to provide in this doc the correspondence between the "new"
den-values and many types of fsaverage (i.e. space-fsaverage5 --> fsaverage_den??) So that users do not become confused. -
One section (the introduction of the
volspace-<space>key) of the anatomical derivatives section still refers to separate "Surface" and "Volume" template tables. I remember expanding this section with more examples in another PR, so I would make sure you don't get conflicts down the road. Note: this key was introduced to capture the use case where the surface files (gifti surface coordinates) files where the surface points have been transformed correspond with the T1w imagespace-fsLR_den-32k_volspace-individual_pial.surf.giifrom the one where the points are in MNI-spacespace-fsLR_den-32k_volspace-MNI152Lin6Sym_pial.surf.gii
| | HCPMNIfsLR59k | Combination of `MNI152NLin6AsymConte69` subcortical volumetric space (with 1.6mm grid) and `fsLR59k` surface space resulting in 170494 data points per timepoint. Used in Human Connectom Project. [https://github.com/Washington-University/HCPpipelines/tree/master/global/templates/170494_Greyordinates](https://github.com/Washington-University/HCPpipelines/tree/master/global/templates/170494_Greyordinates) | | ||
| | Coordinate System | Description | | ||
| | ----------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | 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`. | |
There was a problem hiding this comment.
just clarifying that the individual anatomical space can come from any MR/PET modality.
also, i know we are doing fsnative, but how would we handle things like CIVET or other algorithms that generate surface spaces. is fsnative the default key for native space, independent of the algorithm used to produce it?
There was a problem hiding this comment.
It looks like fsnative is not in the current standard, so it is reasonable to me to change it, if people want. What about recon (short for individualsurface)?
Speaking of individual, is this a proposal, or was there previous discussion? Were there other options? It just feels long...
There was a problem hiding this comment.
yeah I also feel like "individual" is too long..but do we have a good alternative?
There was a problem hiding this comment.
@satra:
how would we handle things like CIVET or other algorithms that generate surface spaces
what you mean by "surface space"? how those operate differently w.r.t. FreeSurfer and fsnative/fsaverage?
There was a problem hiding this comment.
yes, they are different surfaces similar to fs32LR being different from fsnative/fsaverage
There was a problem hiding this comment.
I am probably being obtuse here, my apologies. But fsLR is a template space, for CIVET we would then add the corresponding ID for their template (for instance).
How that maps on to the individual space?
There was a problem hiding this comment.
i'm not a user, so it would be difficult for me to say (perhaps some of the folks north of the border can chime in). in terms of common templates, i would definitely say the freesurfer templates are much more widely used.
Sure, I agree. Thanks for the tip, will modify accordingly. BTW, what does "I'm OM" mean...?
That's my next step here, revising that other PR and make sure everything is consistent.
Agreed, I just reorganized what it was written already. But I'll have a second pass to state this.
This is a good question. I'm not very fond of |
|
..oops I meant to write I'm OK.. |
- this was discussed in the past and supposed to be already removed in version https://github.com/bids-standard/bids-specification/blob/master/src/CHANGES.md#111 but apparently slipped through - it is documented in the text properly, where it states "Note that if EEG is recorded with a separate amplifier, it should be stored separately under a new /eeg data type"
…ersAmplifierModelName [FIX] remove ManufacturersAmplifierModelName (again)
[INFRA] Update release protocol
| | HCPMNIfsLR59k | Combination of `MNI152NLin6AsymConte69` subcortical volumetric space (with 1.6mm grid) and `fsLR59k` surface space resulting in 170494 data points per timepoint. Used in Human Connectom Project. [https://github.com/Washington-University/HCPpipelines/tree/master/global/templates/170494_Greyordinates](https://github.com/Washington-University/HCPpipelines/tree/master/global/templates/170494_Greyordinates) | | ||
| | Coordinate System | Description | | ||
| | ----------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | 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`. | |
There was a problem hiding this comment.
It looks like fsnative is not in the current standard, so it is reasonable to me to change it, if people want. What about recon (short for individualsurface)?
Speaking of individual, is this a proposal, or was there previous discussion? Were there other options? It just feels long...
revert clone instructions; instead point out requirements
Co-Authored-By: Kirstie Whitaker <kw401@cam.ac.uk>
…date [ENH] BEP Update
[ENH] Clarify requirements in Release Protocol
[ENH] Adding Contributors and updating contributions
I've made several changes to the table of template identifiers:
- [x] Reordered template identifiers alphabetically
- [x] Removed the distinction between surface templates and volumetric
templates. After all, to properly apply a surface template one
should first spatially normalize to a particular volumetric
template. Although it is possible to create mixed templates
(e.g. using ``MNI152Lin`` to generate a mapping for volumetric
coordinates and then, say, ``fsLR`` for the surface after some
surface mapping process), this option should be discouraged.
Anyways, this discussion does not affect how the names of BIDS
entities should be specified, since one can always describe how
the template coordinates are generated with the information
described in this PR.
- [x] Removed ``MNI152NLin6AsymConte69`` because it does not define
any new coordinate system (see
bids-standard#207 (comment)),
- [x] Marked DEPRECATED ``fsaverage[3|4|5|6|sym]`
- [x] Added ``fsaverage`` and ``fsaverageSym`` corresponding to such
deprecations.
- [x] Removed ``FS305`` as it is now covered by ``fsaverage``.
- [x] DEPRECATED modifiers of ``UNCInfant``
- [x] Reorganized subsections in "standard" vs "nonstandard" spaces.
92803e5 to
e360607
Compare
|
@effigies, I've rebased to |
|
You were previously doing it against derivatives. I assumed that was intentional. |
|
Right. Okay, not intentional at all. Should I send a new PR? |
|
If it's strictly against things that aren't in derivatives, then I'd just rebase onto master and set master as the upstream. If it also touches on derivatives, I'd say set the upstream to common-derivatives. |
Unfolded from bids-standard#213. I'm splitting that PR into two: one here to the current spec, sorting out existing templates; and a second to the `common-derivatives` branch to make it consistent with this one and adding my suggestions regarding spaces.
This PR supersedes bids-standard#213, and should be rebased with master after bids-standard#306 is merged. W.r.t. master it only adds the table of _Nonstandard template identifiers_ W.r.t. `common-derivatives`, the changes are: - [x] Removed the distinction between surface templates and volumetric templates. After all, to properly apply a surface template one should first spatially normalize to a particular volumetric template. Although it is possible to create mixed templates (e.g. using ``MNI152Lin`` to generate a mapping for volumetric coordinates and then, say, ``fsLR`` for the surface after some surface mapping process), this option should be discouraged. Anyways, this discussion does not affect how the names of BIDS entities should be specified, since one can always describe how the template coordinates are generated with the information described in this PR. - [x] Removed ``MNI152NLin6AsymConte69`` because it does not define any new coordinate system (see bids-standard#207 (comment)), - [x] Removed ``FS305`` as it is now covered by ``fsaverage``. - [x] Reorganized subsections in "standard" vs "nonstandard" spaces.
|
I'm having real trouble rebasing. I separated out this in two PR:
|
* [FIX] Revise template-generated coordinate systems This PR supersedes #213, and should be rebased with master after #306 is merged. W.r.t. master it only adds the table of _Nonstandard template identifiers_ W.r.t. `common-derivatives`, the changes are: - [x] Removed the distinction between surface templates and volumetric templates. After all, to properly apply a surface template one should first spatially normalize to a particular volumetric template. Although it is possible to create mixed templates (e.g. using ``MNI152Lin`` to generate a mapping for volumetric coordinates and then, say, ``fsLR`` for the surface after some surface mapping process), this option should be discouraged. Anyways, this discussion does not affect how the names of BIDS entities should be specified, since one can always describe how the template coordinates are generated with the information described in this PR. - [x] Removed ``MNI152NLin6AsymConte69`` because it does not define any new coordinate system (see #207 (comment)), - [x] Removed ``FS305`` as it is now covered by ``fsaverage``. - [x] Reorganized subsections in "standard" vs "nonstandard" spaces. * fix: added the concept of ``scanner`` as an Image-based coordinate system - [x] Describes more clearly the situation when files do not have ``space-`` definition. - [x] Remove the ``ReferenceIndex`` as an index does not generate a coordinate system (the concept here is mixed up with that of image registration, which is just a process).
I've made several changes to the table of template identifiers:
templates. After all, to properly apply a surface template one
should first spatially normalize to a particular volumetric
template. Although it is possible to create mixed templates
(e.g. using
MNI152Linto generate a mapping for volumetriccoordinates and then, say,
fsLRfor the surface after somesurface mapping process), this option should be discouraged.
Anyways, this discussion does not affect how the names of BIDS
entities should be specified, since one can always describe how
the template coordinates are generated with the information
described in this PR.
MNI152NLin6AsymConte69because it does not defineany new coordinate system (see
[ENH] Derived (processed) MR data #207 (comment)),
fsaverage[3|4|5|6|sym]fsaverageandfsaverageSymcorresponding to suchdeprecations.
FS305as it is now covered byfsaverage.UNCInfant