Skip to content

[FIX] Normalization of template-generated standard spaces#306

Merged
effigies merged 10 commits intobids-standard:masterfrom
oesteban:fix/standard-spaces
Nov 4, 2019
Merged

[FIX] Normalization of template-generated standard spaces#306
effigies merged 10 commits intobids-standard:masterfrom
oesteban:fix/standard-spaces

Conversation

@oesteban
Copy link
Copy Markdown
Collaborator

@oesteban oesteban commented Aug 15, 2019

Unfolded from #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.

  • Reordered template identifiers alphabetically
  • Preempts future distinctions between surface templates and volumetric
    templates (see [FIX] Revise template-generated coordinate systems #213).
  • Marked fsaverage[3|4|5|6|sym] as DISCOURAGED
  • Added fsaverage and fsaverageSym corresponding to such
    "deprecations".
  • DISCOURAGED modifiers of UNCInfant
  • Added fsLR

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.
@oesteban oesteban force-pushed the fix/standard-spaces branch from 0d596ab to aec4090 Compare August 15, 2019 22:41
oesteban added a commit to oesteban/bids-specification that referenced this pull request Aug 15, 2019
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.
@oesteban
Copy link
Copy Markdown
Collaborator Author

This should be ready for review. cc/ @effigies @edickie @satra

Copy link
Copy Markdown
Collaborator

@satra satra left a comment

Choose a reason for hiding this comment

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

Left some comments on naming consistency. I would choose one or both styles

  1. list every separate option as a separate name
  2. list stems with optional components

| MNI152Lin | Also known as ICBM (version with linear coregistration) [http://www.bic.mni.mcgill.ca/ServicesAtlases/ICBM152Lin](http://www.bic.mni.mcgill.ca/ServicesAtlases/ICBM152Lin) |
| MNI152NLin2009\[a-c\]\[Sym|Asym\] | Also known as ICBM (non-linear coregistration with 40 iterations, released in 2009). It comes in either three different flavours each in symmetric or asymmetric version. [http://www.bic.mni.mcgill.ca/ServicesAtlases/ICBM152NLin2009](http://www.bic.mni.mcgill.ca/ServicesAtlases/ICBM152NLin2009) |
| MNI152NLin6Sym | Also known as symmetric ICBM 6th generation (non-linear coregistration). Used by SPM99 - SPM8. [http://www.bic.mni.mcgill.ca/ServicesAtlases/ICBM152NLin6](http://www.bic.mni.mcgill.ca/ServicesAtlases/ICBM152NLin6) |
| MNI152NLin6ASym | A variation of `MNI152NLin6Sym` built by A. Janke that is released as the *MNI template* of FSL. Volumetric templates included with [HCP-Pipelines](https://github.com/Washington-University/HCPpipelines/tree/master/global/templates) correspond to this template too. See [10.1016/j.neuroimage.2012.01.024](https://doi.org/10.1016/j.neuroimage.2012.01.024). |
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

should this be compressed with the one above just like MNI152NLin2009...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Not sure about this one - everyone refers to this template as "MNI" because it comes as such within FSL, but you can't find it in the official repository. (Will correct the ASym bit to be Asym though).

| OASIS30AntsOASISAnts | [https://figshare.com/articles/ANTs_ANTsR_Brain_Templates/915436](https://figshare.com/articles/ANTs_ANTsR_Brain_Templates/915436) |
| OASIS30Atropos | [https://mindboggle.info/data.html](https://mindboggle.info/data.html) |
| Talairach | Piecewise linear scaling of the brain is implemented as described in TT88. [http://www.talairach.org/](http://www.talairach.org/) |
| UNCInfant | Infant Brain Atlases from Neonates to 1- and 2-year-olds. [https://www.nitrc.org/projects/pediatricatlas](https://www.nitrc.org/projects/pediatricatlas) |
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

how will the temporal qualifiers be noted for this atlas and for NIHPD?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

In TemplateFlow we are using a cohort- entity for that. However, I don't think age ranges should be considered as generators of new spaces.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this may boil down to the question whether a custom template is a new space?

i think here space is more related to image landmarks being different rather than whether coordinate axes and units are aligned.

so from that perspective age-ranges could create different landmarks.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I wouldn't go down that road. Coordinates across all age ranges are the same (i.e., the origin of coordinates is always the same and one mm is always the same length too).

For the purpose of registration, you will always have the cohort- present in the ReferenceVolume (or whatever keyword we end up choosing in the derivatives PR), so the reference will be univocal and you'll be always aware of what range was used.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

i'm generally ok, but the image coordinates are different from the coordinate system. under coordinate system, location of origin, units, and angle of axes all the MNI spaces are essentially the same.

i think we just need to be careful in what we call landmarks and coordinate systems. i think robert and i had an extensive discussion on this that led to the updated language for coordinates/landmarks/etc.,..

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

i'm generally ok, but the image coordinates are different from the coordinate system.

Not sure how this applies to this PR, could you clarify?

under coordinate system, location of origin, units, and angle of axes all the MNI spaces are essentially the same.

Agreed.

i think we just need to be careful in what we call landmarks and coordinate systems.

Agree - but this statement makes me believe you are not completely sure about it in the context of this PR, is that correct? could you clarify where you're getting at?

think robert and i had an extensive discussion on this that led to the updated language for coordinates/landmarks/etc.,..

hehehe I witnessed part of that :D

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

but this statement makes me believe you are not completely sure about it in the context of this PR, is that correct? could you clarify where you're getting at?

since we are talking about Template Based Coordinate Systems the image landmarks matter and hence just like fsaverage3/4/5/6, we would have to have some notion for the pediatric templates that are based on different age groups.

Copy link
Copy Markdown
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.

Overall, I'm fine with this. I think we could probably clean up the appearance and clarity of the "DISCOURAGED" statements by making them a new table, e.g.:

The following template identifiers are retained for backwards compatibility
of BIDS implementations.
Their use is NOT RECOMMENDED for new BIDS datasets and tooling, but their
presence MUST NOT produce a validation error.

| Coordinate System | Equivalent to |
|-------------------|---------------|
| fsaverage[...]    | ...           |
| UNCInfant[...]    | ...           |

| ICBM452Warp5Space | Reference space defined by the "average of 452 T1-weighted MRIs of normal young adult brains" "based on a 5th order polynomial transformation into the atlas space" [https://www.loni.usc.edu/research/atlases](https://www.loni.usc.edu/research/atlases) |
| IXI549Space | Reference space defined by the average of the "549 (...) subjects from the IXI dataset" linearly transformed to ICBM MNI 452.Used by SPM12. [http://www.brain-development.org/](http://www.brain-development.org/) |
| fsaverage\[3|4|5|6|sym\] | DISCOURAGED, please use `fsaverage` without modifiers. Images were sampled to the FreeSurfer surface reconstructed from the subject’s T1w image, and registered to an fsaverage template |
| fsaverage\[3|Sym\] | The `fsaverage` is a **dual template** providing both volumetric and surface coordinates references. The volumetric template corresponds to a FreeSurfer variant of `MNI305` space. The `fsaverage` atlas also defines a surface reference system (formerly described as fsaverage\[3|4|5|6|sym\]). The `fsaverageSym` is the symmetric counterpart of `fsaverage`. |
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Presumably you didn't mean to keep fsaverage3 in?

Suggested change
| fsaverage\[3|Sym\] | The `fsaverage` is a **dual template** providing both volumetric and surface coordinates references. The volumetric template corresponds to a FreeSurfer variant of `MNI305` space. The `fsaverage` atlas also defines a surface reference system (formerly described as fsaverage\[3|4|5|6|sym\]). The `fsaverageSym` is the symmetric counterpart of `fsaverage`. |
| fsaverage\[Sym\]. | The `fsaverage` is a **dual template** providing both volumetric and surface coordinates references. The volumetric template corresponds to a FreeSurfer variant of `MNI305` space. The `fsaverage` atlas also defines a surface reference system (formerly described as fsaverage\[3|4|5|6|sym\]). The `fsaverageSym` is the symmetric counterpart of `fsaverage`. |

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Oh right, but I meant to keep the pipe symbol |.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The square brackets should indicate optional already, if that's what a | and empty string are supposed to mean.

Copy link
Copy Markdown
Collaborator Author

@oesteban oesteban Sep 6, 2019

Choose a reason for hiding this comment

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

MNI152NLin2009 is then allowed?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yeah, I guess we're kind of making up these rules as we go. Whatever. As long as it's unambiguous, I can live with it.

Copy link
Copy Markdown
Collaborator

@edickie edickie Sep 6, 2019

Choose a reason for hiding this comment

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

I think it's almost ready to go...but I think this table should also contain the mapping of the original fsaverage[3,4,5,6] to their respective recommended den- values..(or at least a link to text section where those values are specified)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

There is no concept of resolution nor density in the BIDS spec at this point. Those keywords will be added in #307. Please write a comment there so I don't forget to update the table with your suggestion.

@oesteban
Copy link
Copy Markdown
Collaborator Author

oesteban commented Sep 4, 2019 via email

effigies
effigies previously approved these changes Sep 6, 2019
@oesteban
Copy link
Copy Markdown
Collaborator Author

oesteban commented Sep 6, 2019

Copy link
Copy Markdown
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.

Reading the rendered doc, I noticed a typo that we might as well clean up while we're here.

Co-Authored-By: Chris Markiewicz <effigies@gmail.com>
@oesteban
Copy link
Copy Markdown
Collaborator Author

bumping up! cc @edickie @satra

@satra
Copy link
Copy Markdown
Collaborator

satra commented Sep 11, 2019

looks ok to me.

@effigies effigies removed this from the Common Derivatives milestone Sep 26, 2019
@effigies effigies added this to the 1.3.0 milestone Sep 26, 2019
@effigies
Copy link
Copy Markdown
Collaborator

Moved this to the 1.3.0 milestone, because it goes into master, not common-derivatives. It does nonetheless need to be in before common-derivatives can be finalized.

Are there current open issues to address? I'm having trouble determining if @satra's and @edickie's latest comments require changes.

@effigies
Copy link
Copy Markdown
Collaborator

effigies commented Oct 9, 2019

Bump to all concerned.

@edickie @satra @oesteban What do you each feel is the current status?

Also, @bids-standard/raw in general, if you have thoughts about coordinate systems and templates.

@oesteban
Copy link
Copy Markdown
Collaborator Author

Bump to all concerned.

@edickie @satra @oesteban What do you each feel is the current status?

Also, @bids-standard/raw in general, if you have thoughts about coordinate systems and templates.

Bump again :)

@edickie
Copy link
Copy Markdown
Collaborator

edickie commented Oct 22, 2019 via email

@effigies
Copy link
Copy Markdown
Collaborator

@satra @edickie If you're happy with this, could one or both of you give an approving review? Need two to merge.

@effigies
Copy link
Copy Markdown
Collaborator

effigies commented Nov 1, 2019

I plan to merge this on Monday unless someone objects.

@effigies effigies merged commit e7af029 into bids-standard:master Nov 4, 2019
effigies pushed a commit that referenced this pull request Dec 5, 2019
* [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).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants