ENH: Subcortical alignment workflow #72

mgxd · 2021-06-04T15:24:46Z

Closes #49.

A few missing pieces:

CiftiSeparate interface
- The one referenced in Piecewise subcortical alignment for CIFTI mapping #49 (comment) is missing the -volume-all flag, so I'll probably just copy and build on it here.
A workflow description that can be used in the boilerplate (perhaps @ericfeczko would be interested in doing this?)

Once this standalone workflow is finalized (and somewhat tested), I'll open a separate PR actually using it in nibabies.

pep8speaks · 2021-06-04T15:24:49Z

Hello @mgxd! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-07-26 19:02:33 UTC

codecov-commenter · 2021-06-04T20:58:39Z

Codecov Report

Merging #72 (9fe1243) into master (1cdf9da) will increase coverage by 1.97%.
The diff coverage is 68.10%.

@@            Coverage Diff             @@
##           master      #72      +/-   ##
==========================================
+ Coverage   31.53%   33.50%   +1.97%     
==========================================
  Files          42       44       +2     
  Lines        3257     3462     +205     
==========================================
+ Hits         1027     1160     +133     
- Misses       2230     2302      +72

Impacted Files	Coverage Δ
nibabies/config.py	`57.41% <0.00%> (-0.76%)`	⬇️
nibabies/utils/misc.py	`52.38% <ø> (ø)`
nibabies/workflows/bold/alignment.py	`27.08% <27.08%> (ø)`
nibabies/interfaces/workbench.py	`88.40% <70.37%> (-11.60%)`	⬇️
nibabies/interfaces/nibabel.py	`73.77% <97.05%> (+29.32%)`	⬆️
nibabies/conftest.py	`100.00% <100.00%> (ø)`
nibabies/interfaces/tests/test_nibabel.py	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1cdf9da...9fe1243. Read the comment docs.

mgxd · 2021-07-22T22:37:28Z

One of the biggest improvements this Nipype-ified workflow offers is parallelization. However, depending on the number of timepoints in the subject's functional, there are a few points where virtual memory can substantially peak:

Applying the transform of the ROI from the subjects functional to the template space

nibabies/nibabies/workflows/bold/alignment.py

Lines 91 to 96 in 559f7ea

    
           applyxfm_roi = pe.MapNode( 
        
               fsl.ApplyXFM(interp="spline"), 
        
               iterfield=["reference", "in_matrix_file"], 
        
               name='applyxfm_roi', 
        
               mem_gb=2, 
        
           )

I'm fairly sure this can be replaced by NiTransforms, though I don't know if that will help with the memory issue.

Aggregating all processed 4D ROI images into a dense timeseries file

nibabies/nibabies/workflows/bold/alignment.py

Lines 160 to 163 in 559f7ea

    
           create_dtseries_tmpl = pe.Node( 
        
               CiftiCreateDenseFromTemplate(series=True, series_step=repetition_time, series_start=0), 
        
               name='create_dtseries_tmpl', 
        
           )

wb_command -cifti-create-dense-from-template loads up all volumes simultaneously, which can lead to memory crashes. This has been addressed in the DCAN script, but at the cost of having to do things sequentially. Since the final result is just a 4D NIfTI of all the combined ROIs, an alternate approach can be to:

split each 4D ROI NIfTI into 3D series
combine the ROIs within each timepoint (essentially adding the images together)
recombine the now aggregated timepoints into one 4D image

cc @ericfeczko @effigies @oesteban

Using `wb_command -cifti-create-dense-from-template` would load all 4D ROIs into memory at once, causing a drastic bottleneck. Instead of creating a dtseries only to separate back into a volumetric NIfTI, we can simply add together the 4D ROIs and produce the same result, minimizing the number of files simultaneously loaded in memory'

mgxd · 2021-07-23T21:46:18Z

Okay, the two concerns above have been addressed. I'd love to have another set of eyes look over this before merging it in

effigies

I haven't looked through the workflow in detail, yet, but here's some early comments. Do we have docs that will show the workflow graph?

nibabies/workflows/bold/alignment.py

effigies · 2021-07-24T02:11:23Z

nibabies/workflows/bold/alignment.py

+    return rois[0], rois[1:], ("-add %s " * (len(rois) - 1)).strip()
+
+
+def merge_rois(in_files):


This feels like a significant enough function to write a test for. 3 (3,1,1,5) time series would exercise it plenty, and easy to modify to hit each error condition.

effigies · 2021-07-24T02:14:55Z

nibabies/workflows/bold/alignment.py

+def format_volume_rois(structs, rois):
+    """Format volume arguments for CiftiCreateDenseFromTemplate."""
+    return [(struct, roi) for struct, roi in zip(structs, rois)]


I think this is unused. Also, it's just list(zip(structs, rois))...

Yep, remnant of the previous implementation

Co-authored-by: Chris Markiewicz <[email protected]>

effigies · 2021-07-24T19:56:08Z

@mgxd Pushed a test to 9a89211, if you want to have a look then cherry-pick.

mgxd · 2021-07-24T20:51:11Z

@mgxd Pushed a test to 9a89211, if you want to have a look then cherry-pick.

haha, looks like we were both working on the same thing 👀

effigies

A few comments. It overall seems reasonable. A bit tough to be sure of the correctness without a comparison of subject with both workflows.

nibabies/workflows/bold/alignment.py

mgxd · 2021-07-26T19:11:32Z

Thanks for the review @effigies!

nibabies/workflows/bold/alignment.py

mgxd force-pushed the enh/subcortical-cifti branch 6 times, most recently from ffa7eee to 8748be7 Compare June 4, 2021 20:57

mgxd force-pushed the enh/subcortical-cifti branch from 92700af to 67e7684 Compare June 7, 2021 14:26

mgxd force-pushed the enh/subcortical-cifti branch from 67e7684 to 0be8a65 Compare July 20, 2021 01:03

mgxd added 20 commits July 22, 2021 11:51

WIP: Subcortical MNI alignment

c9db42a

RF: CiftiCreateDenseTimeseries optional argument

8f05c5e

WIP: Subcortical alignment

9e55548

RF: CreateDenseTimeseries default output

8bf6d50

RF: set a default for CiftiCreateLabel out_file

83184a0

WIP: Subcortical alignment

639658c

ENH: Finalize subcortical alignment workflow

c53b264

STY: Pep8

be0b05d

FIX: Use value rather than default_value

1ffdfa7

RF: Move CiftiDilateInputSpec

7c29e62

ENH: CiftiSeparate

c00848f

FIX: CreateDenseFromTemplate formatting

959f248

FIX: Set volume all file

6bfa64c

TST: Add simple test for label file parsing

90884a9

TST: Output formatting

99611d8

ENH: Set MNI Infant as the default output space

fb75f29

FIX: Syntax errors, connections, everything really

9cc6a80

ENH: Add helper script to test workflow

ad3cc2e

FIX: more faulty connections

4acaa31

ENH: Run in MultiProc, assign default nipype configuration

d0851fc

mgxd added 2 commits July 22, 2021 12:11

FIX: Doctests

147f42b

FIX: Improve out_file name

559f7ea

mgxd added 4 commits July 23, 2021 11:29

STY: PEP8

630d0f5

FIX: Cap ApplyXFM memory usage

422fe62

ENH: Allow setting nipype plugin via CLI

acf5450

mgxd requested review from effigies, ericfeczko and oesteban July 23, 2021 21:45

effigies reviewed Jul 24, 2021

View reviewed changes

mgxd and others added 3 commits July 23, 2021 23:21

Update nibabies/workflows/bold/alignment.py

d7eda9f

Co-authored-by: Chris Markiewicz <[email protected]>

Update nibabies/workflows/bold/alignment.py

72a7a69

Co-authored-by: Chris Markiewicz <[email protected]>

Update nibabies/workflows/bold/alignment.py

0c6000c

Co-authored-by: Chris Markiewicz <[email protected]>

mgxd added 2 commits July 24, 2021 16:48

RF: Create dedicated interface for ROI combination

e42c7d9

ENH: Add unit test for MergeROIs interface

08529f4

mgxd added 2 commits July 26, 2021 14:12

FIX: Cleanup unused parameter, docstring

1aed572

STY: Black

10f1f45

effigies reviewed Jul 26, 2021

View reviewed changes

nibabies/workflows/bold/alignment.py Outdated Show resolved Hide resolved

nibabies/workflows/bold/alignment.py Show resolved Hide resolved

nibabies/workflows/bold/alignment.py Show resolved Hide resolved

mgxd added 2 commits July 26, 2021 14:57

ENH: Use available transform, set volumetric sigma as parameter

bbeb265

FIX: Keep testing script updated

9fe1243

mgxd mentioned this pull request Jul 26, 2021

Verify default value for CIFTI subcortical volume smoothing #89

Open

mgxd merged commit 0e681c9 into nipreps:master Jul 26, 2021

mgxd deleted the enh/subcortical-cifti branch July 26, 2021 19:11

effigies reviewed Jul 26, 2021

View reviewed changes

nibabies/workflows/bold/alignment.py Show resolved Hide resolved

mgxd mentioned this pull request Sep 16, 2021

ENH: Incorporate subcortical CIFTI alignment to functional processing #102

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: Subcortical alignment workflow #72

ENH: Subcortical alignment workflow #72

mgxd commented Jun 4, 2021 •

edited

Loading

pep8speaks commented Jun 4, 2021 •

edited

Loading

codecov-commenter commented Jun 4, 2021 •

edited

Loading

mgxd commented Jul 22, 2021

mgxd commented Jul 23, 2021

effigies left a comment

effigies Jul 24, 2021

effigies Jul 24, 2021

mgxd Jul 24, 2021

effigies commented Jul 24, 2021

mgxd commented Jul 24, 2021

effigies left a comment

mgxd commented Jul 26, 2021

		return rois[0], rois[1:], ("-add %s " * (len(rois) - 1)).strip()


		def merge_rois(in_files):

ENH: Subcortical alignment workflow #72

ENH: Subcortical alignment workflow #72

Conversation

mgxd commented Jun 4, 2021 • edited Loading

pep8speaks commented Jun 4, 2021 • edited Loading

Comment last updated at 2021-07-26 19:02:33 UTC

codecov-commenter commented Jun 4, 2021 • edited Loading

Codecov Report

mgxd commented Jul 22, 2021

mgxd commented Jul 23, 2021

effigies left a comment

Choose a reason for hiding this comment

effigies Jul 24, 2021

Choose a reason for hiding this comment

effigies Jul 24, 2021

Choose a reason for hiding this comment

mgxd Jul 24, 2021

Choose a reason for hiding this comment

effigies commented Jul 24, 2021

mgxd commented Jul 24, 2021

effigies left a comment

Choose a reason for hiding this comment

mgxd commented Jul 26, 2021

mgxd commented Jun 4, 2021 •

edited

Loading

pep8speaks commented Jun 4, 2021 •

edited

Loading

codecov-commenter commented Jun 4, 2021 •

edited

Loading