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

[melanieganz]

During the BIDS derivative meeting in June, the atlas BEP0038 was thoroughly discussed. You can see a summary of the discussions here.

The outcome of this discussion was that the concept of an atlas is broader than what the current atlas entity defines it to be, so we would like to replace the current atlas entity which was basically representing a segmentation of a derived dataset with the entity "seg" instead. Currently, we define what segmentation is in the spec but do not use segmentation as an entity.

This opens up the possibility for the atlas BEP038 to re-define the atlas entity in a broader sense and to allow it to e.g. be used like a subject in itself.

Examples of the use of the seg entity in a derivative would then be:

<dataset>/derivatives/<pipeline_name>/
sub-01/
	anat/		
             sub-01_space-<label>_seg-<label>_[dseg|probseg|mask].[nii.gz|dscalar.nii|dlabel.nii|label.gii|tsv]

The label would then identify which segmentation was used, e.g. DKT, HarvardOxford, etc.

Note also two things of relevance to this discussion:

1. "atlas" is defined twice in the specification, once as an entity and once as a metadata concept. This will also be necessarily updated by BEP038 in a separate issue.

2. In the derivatives spec we define parcellations explicitly as discrete surface segmentations, so while originally the use of "parc" as an entity was discussed as well "seg" seems more appropriate and broader.

---

The following screenshots summarize the changes to the specification, as compared with the current latest:

Before	After

[effigies]

I would suggest that for now we get rid of the Atlas metadata field altogether. It's under-specified, and BIDS permits unknown terms.

I will submit a PR against your branch to edit the spec text.

[CPernet]

I have too much involvement here to validate anything -- just FYI I updated the derivatives guidelines and BEP38 with 'seg' and propose for future ref atlas (Atlas is defined as per Merrian-Webster, a bound collection of maps (i.e. labelled brain regions) and metadata (tables, or textual matter))

[effigies]

Pr here:

• ENH: Remove Atlas metadata, update imaging derivatives text around seg- melanieganz/bids-specification#1

[PeerHerholz]

Hi folks,

thanks @melanieganz for starting the process and @effigies and @CPernet for comments!

BEP038 is fully supportive of the proposed changes and the current state of the BEP wouldn't change that much. @CPernet and I updated the respective sections of the BEP and examples.

Just to add how an atlas would be represented at the BIDS root, e.g., if shared as is and/or not applied to data:

<dataset>/atlas/<label>/
        atlas-<label>_[dseg|probseg|mask].[nii.gz|dscalar.nii|dlabel.nii|label.gii|tsv]
        atlas-<label>_[dseg|probseg|mask].json
        atlas-<label>_[dseg|probseg|mask].tsv (if `atlas` file is not .tsv)

[Remi-Gau]

will to get more green on the CI side

[codecov]

Codecov Report

Patch and project coverage have no change.

Comparison is base (18837af) 87.83% compared to head (ca28036) 87.83%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #1579   +/-   ##
=======================================
  Coverage   87.83%   87.83%           
=======================================
  Files          16       16           
  Lines        1356     1356           
=======================================
  Hits         1191     1191           
  Misses        165      165           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[melanieganz]

Thank you @effigies and @Remi-Gau for helping with the code coverage checks.

[robertoostenveld]

LGTM

[mnoergaard]

This also looks good to me!

[sappelhoff]

don't know much about atlases but the process and the general arguments make sense to me.

[Lestropie]

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.

[nicholst]

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.

[Lestropie]

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)?

[effigies]

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.

[Lestropie]

This is currently doing an outright erasure of the atlas entity. There are examples of suffices and metadata fields that have been explicitly flagged as deprecated in the specification, and presumably the validator would then have the capability of flagging erroneous use of such. Was there an upstream decision to not do such in this case?

In the derivatives spec we define parcellations explicitly as discrete surface segmentations, so while originally the use of "parc" as an entity was discussed as well "seg" seems more appropriate and broader.

1. Personally I dislike this one. I tend to use "parcellation" and "segmentation" fairly interchangeably due to an absence of robust definition, but I certainly don't do it based on surface vs. voxels. Discrete labels can be assigned to voxels or to vertices (or indeed to any other discrete brain imaging element), and I don't think making "parcellation" apply only to the vertex case provides clarity or utility. This may simply be a carry-over from FreeSurfer's output file naming (aparc*.mgz). So personally I'd revise that common derivatives text. But if there's any merit to that argument that needs to be moved to a different thread.

2. On-topic, a factor to consider here is that for two of the three use cases, "seg" already appears in the suffix:
   sub-<label>_seg-<label>_(d|prob)seg.nii
   I encountered a similar issue at one point with BEP016 where I wanted a model entity to state what the voxel-wise diffusion model is, as well as a model suffix to indicate that the file contains data pertaining to one of the parameters of the model.
   If there were somehow a consensus reached that, for instance, atlases define one or more "parcellations" in a template space, and once transformed to subject space they "segment" the structures in that subject, then perhaps it would be reasonable that "seg" be in the suffix and "parc" be an entity. But I suspect we're miles away from such.
   So for now I just want to point out the string duplication and leave it at that.

[CPernet]

we actually did consider parc but finally opted for seg because dseg and probseg suffixes already exist which bring more consistency - but it is true it is makes anat files with both seg- entity and _dseg or probseg suffixes and a little ugly. On the other hand it makes other files nicer bringing that consistency

derivatives/sub-01/anat/sub-01_seg-DKT_label-GM_dseg.nii
derivatives/sub-01/pet/sub-01-seg_DKT_label-GM_stat_mean_meas-BPND.nii

As you mentioned, parcellation is defined as discrete surface segmentation, which is also why we thought use seg as general segmentation (probabilistic in volume for instance) which follows the current definition.

PS: I am happy that we discuss parc vs seg but removing atlas as currently defined :-) and because we did discuss this, we not totally against it either

[nicholst]

I'm supportive of this change. Personally, I always viewed "atlas", without qualification, as referring to a template space (e.g. MNI, fslaverage_lr), and took care to use "ROI atlas" if referring to an image with integer values that defined regions.

[melanieganz]

This is currently doing an outright erasure of the atlas entity. There are examples of suffices and metadata fields that have been explicitly flagged as deprecated in the specification, and presumably the validator would then have the capability of flagging erroneous use of such. Was there an upstream decision to not do such in this case?

Thanks for this comment @Lestropie; we tried to make this clear above, but I can see it isn't.
The current definition of atlas is not in line with what we believe it should be and this has been discussed especially within the current atlas BEP838 as well as at the derivatives meeting in June. But before confusing everyone with a new definition, we though it was cleaner to actually remove the atlas definition as it is now since it's confusing (mentioned twice in the spec) and used almost interchangeably with what we often would call a segmentation. We want to ensure that this can be done, before we then can move forward with the atlas BEP and discuss the suggestions on how to define it there.

[effigies]

Just a little historical digging: atlas- was added in #997, and has only been in for one BIDS release (just under a year old). It went in with very little community comment (two thumbs ups from maintainers), and seemed harmless at the time as it conformed to an early version of the BIDS Derivatives proposal. It was also being proposed by a BEP17 contributor, and BEP17 will be shifting to the consensus definition of atlases before being merged.

In retrospect, adding a new entity without much broader comment from a BEP that was not in the process of being finalized was a serious mistake. I personally think it would be a shame if BIDS was held up by my mistake, as the merging maintainer.

As to deprecation, this would require new features in either the legacy validator or the upcoming schema validator. Part of the purpose of this is to establish whether this would be a disruptive change; if nobody is using this feature yet, or everybody has been treating it as experimental pending BEP17, then the benefit of that additional work is questionable. And if it is highly disruptive, then we may need to consider more significant changes such as pushing forward with a backwards-incompatible BIDS 2.0 as quickly as possible.

[shnizzedy]

if nobody is using this feature yet, or everybody has been treating it as experimental pending BEP17, then the benefit of that additional work is questionable.

We've been treating the feature as experimental pending BEP17

[danlurie]

Just to clarify, would this proposal also (eventually) change atlas to seg in BEP 012 (Functional MRI derivatives)?

For example BEP 012 specifies the following for ROI time series:
<source_entities>[_atlas-<label>][_desc-<label>]_timeseries.<tsv|nii[.gz]>

I currently make heavy use of atlas in this way, but I have no particularly strong opposition to replacing it with seg going forward as long as the new entity can support the same use case (i.e. specifying and clearly labeling the parcellation/atlas used to create a derivative)

[effigies]

Just to clarify, would this proposal also (eventually) change atlas to seg in BEP 012 (Functional MRI derivatives)?

Yes.

[melanieganz]

Dear all, are there any more comments regarding this or do you have any more objections before we merge this?

[effigies]

@Lestropie Have your questions/points been adequately addressed?

[Lestropie]

@effigies In the middle of a workshop so can't do a huge response, but basically yes:

1. label entity use with mask suffix warrants separate consideration; I commented in the code here as it appeared in the changelog under review but agree it's technically out of PR scope.
2. Deprecation: I've no particular issue with a lack of investment in handling of deprecated entities if this is the first instance of such, as long as it's documented here that the situation has been acknowledged and an explicit decision made.
3. "seg" duplication: As long as it's not an exact duplication between entity and suffix I think it's probably OK.
4. "Segmentation" vs "parcellation": Don't like my chances of switching the consensus so unlikely to try.

[effigies]

In the absence of objection, let's go ahead.

[melanieganz]

I will go ahead with this then!