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

[SCHEMA][ENH] Remove atlas entity and replace it with seg in prep of BEP038 #1579

Merged
merged 18 commits into from
Sep 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 13 additions & 17 deletions src/derivatives/imaging.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,9 @@ Template:

A binary (1 - inside, 0 - outside) mask in the space defined by the [`space` entity](../appendices/entities.md#space).
If no transformation has taken place, the value of `space` SHOULD be set to `orig`.
If the mask is an ROI mask derived from an atlas, then the [`label` entity](../appendices/entities.md#label)) SHOULD
be used to specify the masked structure
(see [Common image-derived labels](#common-image-derived-labels)),
and the `Atlas` metadata SHOULD be defined.
If the mask is an ROI mask derived from an atlas segmentation,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if there's a more extended discussion required here regarding "atlas" vs. "template" or "brain" vs. "structure".

Say one derives a brain mask by registering to a template image and then applying the inverse transformation to a brain mask defined with respect to the template image. This is an entirely valid ROI mask. Two issues however:

  1. The main precedent I'm aware of, fMRIPrep, uses _desc-brain rather than _label-brain.
  2. In my mind, there was no "atlas" used in this process. An "atlas", to me, requires a plurality of labels.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't see any problems with this passage. @Lestropie, I don't understand your example:

Say one derives a brain mask by registering to a template image and then applying the inverse transformation to a brain mask defined with respect to the template image. This is an entirely valid ROI mask.

This example entirely refers to a "brain mask" which to me is not an ROI, but an image defining brain/non-brain. Hence I would never call the input or output a "ROI mask".

I may be out of sync with this PR, but to me "ROI" implies a sub-"region" and excludes a brain mask.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If it's sufficiently unambiguous and unanimous that "ROI" = "subset of brain", then that is true. However I'm not 100% convinced that's the case; I don't see why the brain couldn't be considered a "region" of interest with respect to the whole image, just as could be eg. the skull or face.

If yours were to be an accepted definition, specifically in the context of a brain imaging data structure, especially if it were to instruct that different entities be used for ROI masks vs. not-ROI masks, perhaps that necessitates inclusion in the definition dictionary. The only relevant reference I can find is the "Type" metadata field, which lists "Brain" and "ROI" as being two separate things, but is fairly hidden away.


To put more explicitly the consequence of the point: is there a reason why the _label- entity "SHOULD" doesn't just apply to _mask. suffix data, regardless of whether it is a "ROI" derived from an "atlas" (over and above the potential ambiguity of those two terms)?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this thread may be getting a bit off-topic. The goal of this particular change is simply to use the phrase "atlas segmentation" instead of "atlas". If you would like to make a clarification that is consistent with the existing definition (modulo the removal of this narrowly defined atlas), that would be in-scope for this PR.

If you think the existing text is insufficiently clear, then I would respectfully ask that we postpone that discussion until this one is complete, or at least move it to a new issue/PR.

then the [`label` entity](../appendices/entities.md#label) SHOULD be used to specify the masked structure
(see [Common image-derived labels](#common-image-derived-labels)).

JSON metadata fields:

Expand All @@ -170,7 +169,6 @@ A guide for using macros can be found at
-->
{{ MACROS___make_sidecar_table([
"derivatives.common_derivatives.MaskDerivatives",
"derivatives.common_derivatives.MaskDerivativesAtlas",
"derivatives.common_derivatives.ImageDerivativeResEntity",
"derivatives.common_derivatives.ImageDerivativeDenEntity",
]) }}
Expand Down Expand Up @@ -229,10 +227,10 @@ structure may be concatenated in a single file.
Segmentations may be defined in a volume (labeled voxels), a surface (labeled
vertices) or a combined volume/surface space.

If the segmentation can be derived from different atlases,
the [`atlas` entity](../appendices/entities.md#atlas) MAY be used to
distinguish the different segmentations.
If so, the `Atlas` metadata SHOULD also be defined.
If the segmentation can be generated in different ways,
for example, following an atlas segmentation,
the [`seg` entity](../appendices/entities.md#segmentation) MAY be used to
distinguish the name of the segmentation used.

The following section describes discrete and probabilistic segmentations of
volumes, followed by discrete segmentations of surface/combined spaces.
Expand All @@ -250,7 +248,6 @@ A guide for using macros can be found at
-->
{{ MACROS___make_sidecar_table([
"derivatives.common_derivatives.SegmentationCommon",
"derivatives.common_derivatives.SegmentationCommonAtlas",
"derivatives.common_derivatives.ImageDerivativeResEntity",
"derivatives.common_derivatives.ImageDerivativeDenEntity",
]) }}
Expand All @@ -268,7 +265,7 @@ Template:
<pipeline_name>/
sub-<label>/
anat|func|dwi/
<source_entities>[_space-<space>][_atlas-<label>][_res-<label>][_den-<label>]_dseg.nii.gz
<source_entities>[_space-<space>][_seg-<label>][_res-<label>][_den-<label>]_dseg.nii.gz
```

Example:
Expand Down Expand Up @@ -296,8 +293,7 @@ In this case, the mask suffix MUST be used,
the [`label` entity](../appendices/entities.md#label)) SHOULD be used
to specify the masked structure
(see [Common image-derived labels](#common-image-derived-labels)),
the [`atlas` entity](../appendices/entities.md#atlas) and the
`Atlas` metadata SHOULD be defined.
and the [`seg` entity](../appendices/entities.md#segmentation) SHOULD be defined.

For example:

Expand All @@ -310,8 +306,8 @@ A guide for using macros can be found at
"pipeline": {
"sub-001": {
"anat": {
"sub-001_space-orig_atlas-Desikan_label-GM_mask.nii.gz": "",
"sub-001_space-orig_atlas-Desikan_label-GM_mask.json": "",
"sub-001_space-orig_seg-Desikan_label-GM_mask.nii.gz": "",
"sub-001_space-orig_seg-Desikan_label-GM_mask.json": "",
},
},
}
Expand All @@ -333,7 +329,7 @@ Template:
<pipeline_name>/
sub-<label>/
func|anat|dwi/
<source_entities>[_space-<space>][_atlas-<label>][_res-<label>][_den-<label>][_label-<label>]_probseg.nii.gz
<source_entities>[_space-<space>][_seg-<label>][_res-<label>][_den-<label>][_label-<label>]_probseg.nii.gz
```

Example:
Expand Down Expand Up @@ -407,7 +403,7 @@ Template:
<pipeline_name>/
sub-<label>/
anat/
<source_entities>[_hemi-{L|R}][_space-<space>][_atlas-<label>][_res-<label>][_den-<label>]_dseg.{label.gii|dlabel.nii}
<source_entities>[_hemi-{L|R}][_space-<space>][_seg-<label>][_res-<label>][_den-<label>]_dseg.{label.gii|dlabel.nii}
```

The [`hemi-<label>`](../appendices/entities.md#hemi) entity is REQUIRED for GIFTI files storing information about
Expand Down
20 changes: 10 additions & 10 deletions src/schema/objects/entities.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,16 +25,6 @@ acquisition:
remains at the discretion of the researcher.
type: string
format: label
atlas:
name: atlas
display_name: Atlas
description: |
The `atlas-<label>` key/value pair corresponds to a custom label the user
MAY use to distinguish a different atlas used for similar type of data.

This entity is only applicable to derivative data.
type: string
format: label
ceagent:
name: ce
display_name: Contrast Enhancing Agent
Expand Down Expand Up @@ -286,6 +276,16 @@ sample:
The label MUST be unique per subject and is RECOMMENDED to be unique throughout the dataset.
type: string
format: label
segmentation:
name: seg
display_name: Segmentation
description: |
The `seg-<label>` key/value pair corresponds to a custom label the user
MAY use to distinguish different segmentations.

This entity is only applicable to derivative data.
type: string
format: label
session:
name: ses
display_name: Session
Expand Down
6 changes: 0 additions & 6 deletions src/schema/objects/metadata.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -177,12 +177,6 @@ AssociatedEmptyRoom:
format: dataset_relative
- type: string
format: bids_uri
Atlas:
name: Atlas
display_name: Atlas
description: |
Which atlas (if any) was used to generate the mask.
type: string
AttenuationCorrection:
name: AttenuationCorrection
display_name: Attenuation Correction
Expand Down
2 changes: 1 addition & 1 deletion src/schema/rules/entities.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
- split
- recording
- chunk
- atlas
- segmentation
- resolution
- density
- label
Expand Down
20 changes: 0 additions & 20 deletions src/schema/rules/sidecars/derivatives/common_derivatives.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -38,33 +38,13 @@ MaskDerivatives:
Sources: recommended
RawSources: deprecated

MaskDerivativesAtlas:
selectors:
- dataset.dataset_description.DatasetType == "derivative"
- suffix == "mask"
- '"label" in entities'
fields:
Atlas:
level: recommended
level_addendum: if `label` entity is defined

SegmentationCommon:
selectors:
- dataset.dataset_description.DatasetType == "derivative"
- 'intersects([suffix], ["dseg", "probseg"])'
fields:
Manual: optional

SegmentationCommonAtlas:
selectors:
- dataset.dataset_description.DatasetType == "derivative"
- 'intersects([suffix], ["dseg", "probseg"])'
- '"atlas" in entities'
fields:
Atlas:
level: recommended
level_addendum: if `atlas` is present

# Derivatives -> Imaging data types
ImageDerivatives:
selectors:
Expand Down