Review of the atlas proposal #2

pwighton · 2025-02-21T23:18:28Z

This review of oesteban/bep038-review uses the following working definitions:

template: An aggregation of continuous- or discreet- valued data that MAY serve as the authoritative definition of a space
atlas: A collection of related templates

Additionaly, any reference to atlas in the text (e.g. "Let's complete the above example by adding two new atlases") has been interpreted to mean segmentation where reasonable.

My takeaway from reviewing the prosal under these assumptions is there is very little motivation for the atlas- entity that exists purely to group related templates.

I've itemized the major changes I've made to help kickoff the discussion

removed references to provenance, which I believe should be considered out of scope for this BEP
removed cohort- under entities that do not aggregate data over more than one subject but I do not think we should have the schema enforce this
removed [_atlas-<label>] under "The filename pattern for subject-level derivatives follows the general BIDS-Derivatives pattern". I can't think of case where [_seg-<label>] wouldn't suffice
removed the statement "The cohort-<label> directory and entity are dual in terms of usage to BIDS raw's session-<label>."
removed the statement "Similarly, deriving from an existing template and atlases is also a higher-than-first-level analysis as it builds on a previous analysis.". FreeSurfer uses many atlases in it's recon-all pipeline, but most people would consider recon-all to be a first-level or subject-level analysis.
removed the ps13rev2034 use case until it can be clarified. There seems to be some confusion over this use case. This pipeline does not generate segmentations, but uses existing segmentations to compute averages over ROIs. Also, if the goal was to add new segmentations to the dataset, why can't seg- be used for this purpose? If the goal is to update the outputs of the pipeline, why can't the outputs simply be changed from tpl-ps13 to tpl-ps13rev2034?
the entity label- is used often, but never mentioned in the text. Switched to seg-
It's not clear what seg-17n refers to in the Buckner2011 example, but that has been repalced by seg-Buckner2011n17

Making the following assumptions: - template: An aggregation of continuous- or discreet- valued data that MAY serve as the authoritative definition of a space - atlas: A collection of related templates - any reference to atlas in the text (e.g. "Let's complete the above example by adding two new atlases") has been interpreted to mean segmentation where reasonable.

oesteban

@pwighton may I ask you to annotate your changes with comments where you describe what's your goal with each change? I've had to stop the review at the moment I realized that I'm not understanding the rationale behind the changes. I worry these changes substantially blur the lines between template and atlas and, more worryingly, between seg- and atlas-, when seg is a concept already existing in BIDS-Derivatives which I don't think we should change (or misuse without explicitly changing).

Happy to go over the suggestions on a call.

src/derivatives/atlas.md

oesteban · 2025-02-24T10:30:30Z

src/derivatives/atlas.md

+In BIDS, a template is considered any aggregation of continuous- or discreet- 
+valued data.  Some templates also serve as the authoritative definition of a
+space and are used to bring other imaging data into alignment so that it can be 
+aggregated.
+
+In BIDS, an atlas is considered a collection of related templates.


These definitions were moved into the common principles, and a link is found at the beginning. I would avoid this re-definition here.

I'd be okay with a more explicit mention that template and atlas are defined in the common principles, either here or (probably better) at the very beginning.

Suggested change

In BIDS, a template is considered any aggregation of continuous- or discreet-

valued data. Some templates also serve as the authoritative definition of a

space and are used to bring other imaging data into alignment so that it can be

aggregated.

In BIDS, an atlas is considered a collection of related templates.

I understand this perspective but disagree. When I'm reading a spec, I don't want to have to jump around to from page to page to understand it. But if that's the way BIDS does things, then fine. I think we have larger issues to align on first.

oesteban · 2025-02-24T10:35:13Z

src/derivatives/atlas.md

+For templates that do not aggregate data over more than one subject, the
+organization follows the standards for BIDS raw and derivatives.  The following


I would need more context to understand this change, I cannot grasp what is the nuance it is trying to introduce.

The original sentence wants to say: if you are working within each individual's space, and, for example, apply a segmentation at a given scale from an atlas, then you use the general BIDS raw and derivatives conventions which is basically to say, this part of derivatives is no different to others, so we will not redefine those entities (e.g., seg-).

It was not my intent to change the meaning of this sentence,but rather to clarify based on our last meeting. Do you think this change changes the meaning?

I've also removed the mention of "provenance" as I think that should be considered out of scope

oesteban · 2025-02-24T10:36:25Z

src/derivatives/atlas.md

+organization follows the standards for BIDS raw and derivatives.  The following
+entities MAY be employed to specify template-derived results:


Please use semantic line breaks:

Suggested change

organization follows the standards for BIDS raw and derivatives. The following

entities MAY be employed to specify template-derived results:

organization follows the standards for BIDS raw and derivatives.

The following entities MAY be employed to specify template-derived results:

That said, like for the previous sentence, I'm unsure I understand why removing atlas-derived from here.

For this review we agreed to use the definition of atlas as "A collection of related templates".

oesteban · 2025-02-24T10:37:58Z

src/derivatives/atlas.md

-            <source_entities>[_space-<space>][_cohort-<label>][_atlas-<label>][seg-<label>][_scale-<label>][_res-<label>][_den-<label>][_desc-<label>]_<suffix>.<extension>
+            <source_entities>[_space-<space>][_cohort-<label>][seg-<label>][_scale-<label>][_res-<label>][_den-<label>][_desc-<label>]_<suffix>.<extension>


I don't understand this change, it is conflicting with the changes above. First, the pattern continues to use cohort when it has not been defined earlier.

oesteban · 2025-02-24T10:41:18Z

src/derivatives/atlas.md

-The [`cohort-<label>` directory and entity](../glossary.md#cohort-entities) are dual in terms of usage to BIDS raw's
-[`session-<label>`](../glossary.md#session-entities).
-Both subject-level and template-level results can coexist in a single pipeline directory:
+
+Both subject-level and group-level results can coexist in a single pipeline directory:


It is not clear what "template-level" means. Subject-level and group-level are widely understood concepts. Is FreeSurfer's recon-all subject-level or template-level? It's both, right? Since it's an analysis that occurs at the subject-level, yet uses templates.

oesteban · 2025-02-24T10:44:45Z

src/derivatives/atlas.md

+            "sub-01_space-T1w_atlas-AAL_label-brain_mask.nii.gz": "",
+            "sub-01_space-T1w_atlas-AAL_label-head_mask.nii.gz": "",
+            "sub-01_space-T1w_atlas-AAL_T1w.nii.gz": "",
+            "sub-01_space-T1w_atlas-AAL_T1w.json": "",


What does this mean? This example wants to show that you can combine results derived from an atlas (AAL, in this case) and results derived from, e.g., applying FSL FAST and extracting the brain and the head mask.

Adding atlas-AAL here is confusing and defeats the purpose of them in the example. Happy to include explanations to make the point clearer.

src/derivatives/atlas.md

oesteban · 2025-02-24T10:45:59Z

src/derivatives/atlas.md

-            "tpl-PS13_atlas-Economo1916_desc-nopvc_dseg.nii.gz": "",
-            "tpl-PS13_atlas-Economo1916_desc-pvc_dseg.nii.gz": "",
-            "tpl-PS13_atlas-Economo1916_dseg.json": "",
-            "tpl-PS13_atlas-Economo1916_dseg.tsv": "",
-            "tpl-PS13_atlas-RamonCajal1908_desc-nopvc_dseg.nii.gz": "",
-            "tpl-PS13_atlas-RamonCajal1908_desc-pvc_dseg.nii.gz": "",
-            "tpl-PS13_atlas-RamonCajal1908_dseg.json": "",
-            "tpl-PS13_atlas-RamonCajal1908_dseg.tsv": "",


oesteban · 2025-02-24T10:47:03Z

src/derivatives/atlas.md

 }}

-where the `atlas-RamonCajal1908` and `atlas-Economo1916` hypothetically define
+where the `seg-RamonCajal1908` and `seg-Economo1916` hypothetically define


This conflates seg with atlas. Seg is not anymore a segmentation, but a family of annotated knowledge of the brain.

Co-authored-by: Oscar Esteban <[email protected]>

pwighton · 2025-03-01T01:16:32Z

@pwighton may I ask you to annotate your changes with comments where you describe what's your goal with each change?

Yes, @oesteban absolutely. I struggled for a while trying to find a decent way to do this in github's PR system. I eventually gave up and transcribed your proposal into a google doc, made my changes there and described the motivations.

The transcription of the proposal is here:
https://docs.google.com/document/d/1FVXLQ_Fy35KzLXLTQI5Cw3ZqmAj-JviGfsJ6x0ADMLM/edit?usp=sharing

And my edits with motivations is here:
https://docs.google.com/document/d/1OX5c6uVccMZ9iBrBgX4aRwBU1yRnxEHEW3SCxBDirB8/edit?usp=sharing

I can give you (and anyone else interested) write access to both these documents. Just reply here (or email me) with the email address I should send the invite to.

There might be slight differences between the edits in this PR and in the google doc. I've glossed over the ps13 example for now, because I believe there are some issues there, but those issues are not related to the fundamental concepts we need to align on first.

I worry these changes substantially blur the lines between template and atlas and, more worryingly, between seg- and atlas-, when seg is a concept already existing in BIDS-Derivatives which I don't think we should change (or misuse without explicitly changing).

I think this gets to the root of the issues I've been wrestling with for a while. I appreciate your patience as I try to understand the differences between tpl- and atlas- and seg- and atlas-. I will note that this proposal uses the seg- entity sparingly (e.g. tpl-SUIT_atlas-Buckner2011_seg-17n_dseg.nii.gz) so I'm not sure why it can't be used more broadly.

This proposal also uses the suffixes _dseg and _probseg quite often. If the spec got to a state where there are cases when the suffixes _dseg and _probseg apply but the entity seg- does not. I think that would be unfortunate as it would likely confuse users. If there is motivation for this however, it should be described in the specification.

Thanks. Looking forward to discussing this on a call.

effigies · 2025-03-06T19:10:53Z

@pwighton Can you give me comment and/or write permission on those docs? I can't see the edits/comments without.

effigies · 2025-03-06T19:14:29Z

Also, @pwighton @oesteban can we set up a weekly/biweekly call? I'm happy to send around a whenisgood if that works for you.

pwighton · 2025-03-06T19:39:16Z

@effigies thanks for pointing this out, I had no idea! You should have an invite for editor access.

Everyone else should be able to see the comments using this link
https://docs.google.com/document/d/1OX5c6uVccMZ9iBrBgX4aRwBU1yRnxEHEW3SCxBDirB8/edit?usp=sharing

But I'm happy to give editor access to anyone else who reaches out.

pwighton · 2025-03-06T19:40:11Z

Also, @pwighton @oesteban can we set up a weekly/biweekly call? I'm happy to send around a whenisgood if that works for you.

@effigies, yes! I think this is a good idea and the best way to make forward progress on this

pwighton mentioned this pull request Feb 21, 2025

Alternative proposal to BEP038 bids-standard/bids-specification#1856

Merged

oesteban reviewed Feb 24, 2025

View reviewed changes

pwighton and others added 2 commits February 26, 2025 14:15

Update src/derivatives/atlas.md

92a6fdb

Co-authored-by: Oscar Esteban <[email protected]>

Update src/derivatives/atlas.md

3bfcf1c

Co-authored-by: Oscar Esteban <[email protected]>

		For templates that do not aggregate data over more than one subject, the
		organization follows the standards for BIDS raw and derivatives. The following

		organization follows the standards for BIDS raw and derivatives. The following
		entities MAY be employed to specify template-derived results:

		<source_entities>[_space-<space>][_cohort-<label>][_atlas-<label>][seg-<label>][_scale-<label>][_res-<label>][_den-<label>][_desc-<label>]_<suffix>.<extension>
		<source_entities>[_space-<space>][_cohort-<label>][seg-<label>][_scale-<label>][_res-<label>][_den-<label>][_desc-<label>]_<suffix>.<extension>

Review of the atlas proposal #2

Are you sure you want to change the base?

Review of the atlas proposal #2

Uh oh!

Conversation

pwighton commented Feb 21, 2025

Uh oh!

oesteban left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pwighton Feb 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pwighton commented Mar 1, 2025

Uh oh!

effigies commented Mar 6, 2025

Uh oh!

effigies commented Mar 6, 2025

Uh oh!

pwighton commented Mar 6, 2025

Uh oh!

pwighton commented Mar 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pwighton Feb 28, 2025 •

edited

Loading