♻️ refactoring anatomical_resampled bold masking by birajstha · Pull Request #2215 · FCP-INDI/C-PAC

birajstha · 2025-04-16T16:37:31Z

Fixes

Fixes #2209 by @birajstha
Related to #2047 @sgiavasis

Description

This PR refactors the bold_mask_anatomical_resampled nodeblock used in functional masking, aligning its functionality more closely with its original intent of producing template-space BOLD masks only, and removes its generalization for creating native-space masks, which was causing cropped mask outputs.

Technical details

Removed support for native-space masking via Anatomical_Resampled, including:
- The back-transformation of the mask to native space.
- Associated nodes and connections performing that step.
Removed Anatomical_Resampled from the func_masking options in pipeline configs to avoid misuse.
Created a new dedicated template_space_bold_masking section, where Anatomical_Resampled is now the only available method tobetter reflect its usage in the ABCD pipeline.
Cleaned up unused or misleading components within the nodeblock.
Updated the abcd_options preconfig to continue using Anatomical_Resampled for template-space masking only.

Tests

Run CPAC with abcd-options preconfig.

Screenshots

After the fix

Checklist

My pull request has a descriptive title (not a vague title like Update index.md).
My pull request targets the develop branch of the repository.
My commit messages follow best practices.
My code follows the established code style of the repository.
I added tests for the changes I made (if applicable).
I updated the changelog.
I added or updated documentation (if applicable).
I tried running the project locally and verified that there are no visible errors.

Developer Certificate of Origin

Developer Certificate of Origin
Version 1.1

Copyright (C) 2004, 2006 The Linux Foundation and its contributors.
1 Letterman Drive
Suite D4700
San Francisco, CA, 94129

Everyone is permitted to copy and distribute verbatim copies of this
license document, but changing it is not allowed.


Developer's Certificate of Origin 1.1

By making a contribution to this project, I certify that:

(a) The contribution was created in whole or in part by me and I
    have the right to submit it under the open source license
    indicated in the file; or

(b) The contribution is based upon previous work that, to the best
    of my knowledge, is covered under an appropriate open source
    license and I have the right under that license to submit that
    work with modifications, whether created in whole or in part
    by me, under the same open source license (unless I am
    permitted to submit under a different license), as indicated
    in the file; or

(c) The contribution was provided directly to me by some other
    person who certified (a), (b) or (c) and I have not modified
    it.

(d) I understand and agree that this project and the contribution
    are public and that a record of the contribution (including all
    personal information I submit with it, including my sign-off) is
    maintained indefinitely and may be redistributed consistent with
    this project or the open source license(s) involved.

…space masking block

for more information, see https://pre-commit.ci

shnizzedy · 2025-04-28T14:46:39Z

CPAC/func_preproc/func_motion.py

+    else:
+        automask = pe.Node(interface=afni.Automask(), name=f"automask_bold_{pipe_num}")
+        automask.inputs.dilate = 1
+        automask.inputs.outputtype = "NIFTI_GZ"
+
+        node, out = strat_pool.get_data("desc-preproc_bold")
+        wf.connect(node, out, automask, "in_file")
+        wf.connect(automask, "out_file", gen_motion_stats, "inputspec.mask")


In this case, do we want to put automask.out_file in the resource pool as "space-bold_desc-brain_mask" or do we not care to keep it beyond this one connection?

I am not sure about this one.
As far as I can tell, its not needed in the abcd-options pipeline as the bold is masked in template space.
I guess it won't hurt to push that into the resource pool anyway. I can check with @sgiavasis in the next meeting.

I would lean towards no, but, actually-
Does HCP-ABCD do anything with the BOLD in native space? If so, how do they handle it?
If not - the answer is probably that they just do all of this in template space.

I recommend reviewing their pipeline choices first.

What I'm getting at - it may be that we just need to make that jump to more flexible "native vs. template space" toggles for basically any single nodeblock that can interchangeably be done in any space.

The answer is probably "calculate motion stats in template space after masking in template space."
As always, my mandatory mention: the new engine could do this 😅

CPAC/func_preproc/func_preproc.py

CPAC/pipeline/cpac_pipeline.py

shnizzedy · 2025-04-28T16:30:11Z

CPAC/pipeline/schema.py

            },
+            "template_space_func_masking": {
+                "run": bool1_1,
+                "using": [In({"Anatomical_Resampled"})],


Why even have using if there's only one option?

This is an attempt to keep it open if we have another method in future, that we may add here.
But, I am open to just make it for "Anatomical_Resampled".

shnizzedy · 2025-04-28T16:35:21Z

CPAC/resources/configs/pipeline_config_default.yml

+  template_space_func_masking:
+    run: Off
+      # Anatomical_Resampled: Resample anatomical brain mask in standard space to get BOLD brain mask in standard space. Adapted from DCAN Lab's BOLD mask method from the ABCD pipeline. ("Create fMRI resolution standard space files for T1w image, wmparc, and brain mask […] don't use FLIRT to do spline interpolation with -applyisoxfm for the 2mm and 1mm cases because it doesn't know the peculiarities of the MNI template FOVs")
+    using: [Anatomical_Resampled]
+
+    apply_func_mask_in_template_space: Off


I (think I) get "run" is generating the mask and "apply_func_mask_in_template_space" is applying the generated mask, but I think we could be clearer if we add some comments in the default and blank configs and/or naming the keys something like "generate" and "apply"

I see what you menan. I think we could remove the switch apply_func_mask_in_template_space: Off, so that run will both generate and apply the mask. Its less confusing that way?

If we don't want / care to allow for generating-but-not-applying, then yeah, I think just one switch makes sense. Otherwise, I just think

it's not clear that run generates but doesn't apply, and

apply_func_mask_in_template_space is unnecessarily long.

But those are just my opinions. I think with some comments saying what the options do, these keys can work fine.

birajstha

Copilot changes

codecov · 2025-06-13T17:36:07Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 0.0%. Comparing base (b42fcd6) to head (eeb8915).
Report is 18 commits behind head on develop.

Additional details and impacted files

@@            Coverage Diff             @@
##           develop   #2215      +/-   ##
==========================================
- Coverage     28.2%       0   -28.2%     
==========================================
  Files          230       0     -230     
  Lines        25890       0   -25890     
  Branches      4069       0    -4069     
==========================================
- Hits          7294       0    -7294     
+ Misses       17969       0   -17969     
+ Partials       627       0     -627

see 230 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

birajstha added 5 commits April 16, 2025 11:28

removing master input as data is already aligned to desired orientation

52318ca

removing resampling of mask from template space to native space

ed1b214

changing the pre-configs adding anatomical_resampled to the template-…

c7136eb

…space masking block

adding template_space_func_masking to the schema

cfeaa01

adding the template_space_func_masking nodeblock

2df7b30

github-project-automation bot added this to C-PAC Development Apr 16, 2025

birajstha temporarily deployed to ACCESS April 16, 2025 16:37 — with GitHub Actions Inactive

birajstha marked this pull request as draft April 16, 2025 16:38

birajstha linked an issue Apr 16, 2025 that may be closed by this pull request

🐛 Resampled mask chipped off #2209

Closed

15 tasks

birajstha temporarily deployed to ACCESS April 18, 2025 13:59 — with GitHub Actions Inactive

adding quick bold masking with automask

9cca410

birajstha force-pushed the fix_chipped_mask branch from a8b3bdf to 9cca410 Compare April 25, 2025 04:25

tweaking the switch to match the new nodeblock

fbc6abf

birajstha force-pushed the fix_chipped_mask branch from b378709 to fbc6abf Compare April 25, 2025 05:09

pre-commit-ci bot and others added 2 commits April 25, 2025 05:09

[pre-commit.ci] auto fixes from pre-commit.com hooks

992e01c

for more information, see https://pre-commit.ci

Merge branch 'develop' into fix_chipped_mask

91eb67b

birajstha temporarily deployed to ACCESS April 25, 2025 05:18 — with GitHub Actions Inactive

using afni.Automask instead of MaskTool

3e95ba3

birajstha temporarily deployed to ACCESS April 25, 2025 17:20 — with GitHub Actions Inactive

birajstha and others added 2 commits April 25, 2025 15:10

Merge branch 'develop' into fix_chipped_mask

b64155a

adding to changelog

860cfde

birajstha marked this pull request as ready for review April 25, 2025 19:20

birajstha temporarily deployed to ACCESS April 25, 2025 19:20 — with GitHub Actions Inactive

birajstha requested review from a team and Copilot April 25, 2025 19:21

This comment was marked as off-topic.

Sign in to view

Reverting few custom changes used for testing

e371bd1

birajstha temporarily deployed to ACCESS April 25, 2025 19:27 — with GitHub Actions Inactive

FCP-INDI deleted a comment from Copilot AI Apr 25, 2025

birajstha requested a review from Copilot April 25, 2025 19:43

This comment was marked as duplicate.

Sign in to view

shnizzedy reviewed Apr 28, 2025

View reviewed changes

birajstha commented Apr 29, 2025

View reviewed changes

birajstha marked this pull request as draft April 29, 2025 16:18

birajstha added 2 commits April 29, 2025 12:59

changes made to the resolve confusing namings after jon's review

eb3cbbb

removing schema validation for removed switch

6d8da2b

birajstha marked this pull request as ready for review April 29, 2025 17:10

birajstha had a problem deploying to ACCESS April 29, 2025 17:10 — with GitHub Actions Failure

birajstha temporarily deployed to ACCESS April 29, 2025 17:16 — with GitHub Actions Inactive

Merge branch 'develop' into fix_chipped_mask

eeb8915

sgiavasis temporarily deployed to ACCESS June 12, 2025 17:21 — with GitHub Actions Inactive

sgiavasis merged commit e464b49 into develop Jun 13, 2025
29 checks passed

github-project-automation bot moved this to ✅ Done in C-PAC Development Jun 13, 2025

shnizzedy mentioned this pull request Jun 26, 2025

🐛 Create missing space-bold_desc-brain_mask on-demand #2256

Merged

8 tasks

Uh oh!

Comments

Conversation

birajstha commented Apr 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fixes

Description

Technical details

Tests

Screenshots

Checklist

Developer Certificate of Origin

Uh oh!

This comment was marked as off-topic.

Uh oh!

This comment was marked as duplicate.

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!

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!

birajstha left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Jun 13, 2025

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

birajstha commented Apr 16, 2025 •

edited

Loading