[ENH] Efforts towards keeping memory low #807

oesteban · 2017-10-31T23:13:17Z

This PR reduces memory with two principal changes:

Improved and more granular memory annotations
Run confounds in native space:
- Added a new workflow that resamples the BOLD in native space (will be useful for [WIP] ENH: Add --func-only option to skip anatomical processing #808)
- Adapted the confounds workflows to operate in native space: masks are converted into ROIs in the original T1w space (meaning, the erosions that Chris balanced are still happening at that resolution). A minor change is that the WM+CSF mask is now calculated from the TPMs (instead of calculating separate ROIs and then merging).
- Added the CSF ROI to the _confounds.tsv file.

For reviewing this PR, please have a detailed look into the new CompCor masks.

…-revisit

oesteban · 2017-11-02T06:14:17Z

Edited the PR initial message.

effigies

Couple mostly stylistic issues (questions, really), but I think the new ROI resampling strategy still needs to mask in the BOLD space.

effigies · 2017-11-02T13:12:04Z

fmriprep/interfaces/utils.py


+        for idx in indexes[1:]:
+            data += nb.load(in_files[idx]).get_data()


The goal here is to avoid keeping N files in memory?

Not really, I was just avoiding the use of fsl.maths. This interface is just to add tissue probability maps which are small files by definition. Maybe I don't get where you want to get at...

Just kind of wondering about the whole goal of this.

I see. If you're not trying to save memory and you really want the sum and not the logical OR (the docstring needs fixing, btw), you can replace a lot of this logic (up to this point):

def _run_interface(self, runtime): indices = self.inputs.indices or slice(None) in_files = np.array(self.inputs.in_files)[indices] if len(in_files) == 1: self._results['out_file'] = in_files[0] return runtime im = nb.concat_images(in_files) data = im.get_data().sum(axis=3)

effigies · 2017-11-02T18:02:38Z

fmriprep/interfaces/utils.py

-    input_spec = ConcatROIsInputSpec
-    output_spec = ConcatROIsOutputSpec
+        out_file = fname_presuffix(first_fname, suffix='_tpmsum')
+        im.__class__(data, im.affine, im.header).to_filename(out_file)


I believe this is boolean? Might be worth making sure that it's saved as np.uint8.

Ok, I'm double checking. That should be a float. This interface works directly on the TPMs, and it should not change the original dtype. I think it is safe to leave it this way.

effigies · 2017-11-02T18:05:35Z

fmriprep/workflows/bold/base.py

@@ -549,9 +562,60 @@ def init_func_preproc_wf(bold_file, ignore, freesurfer,
                    ('outputnode.bold_mask', 'inputnode.bold_mask')]),
            ])

+    # Apply transforms in 1 shot
+    # Only use uncompressed output if AROMA is to be run
+    bold_bold_trans_wf = init_bold_preproc_trans_wf(


Why bold_bold on this and next workflow?

well, its kind of bold-to-bold resampling. Since other are called bold_mni_ and bold_t1_ ...

Okay, I'm going to need to reread, because I did not catch that this wasn't bold->T1w. So is this so that we have an output if it's --func-only?

There are several targets with this PR:

Effectively, working on BOLD space will make it a lot easier to implement this functionality with --func-only. However, as it is now we still need segmentations from T1. Now it is just easier to replace them with other (e.g. coming from an EPI atlas).

Targets memory fingerprint, since the BOLD files we use to compute the confounds are as small as the original BOLD files. Resampled BOLD on T1 space were generally at least one order of magnitude bigger.

The idea of the new reportlet (std before and after resampling) we add some means to the reports to show if HMC worked well. You were right both on that the report I showed you mixes up all corrections (STC, HMC and SDC) but I'd expect that the largest contributor to differences in that plot is HMC.

Why would resampled BOLD on T1 be bigger? I thought the reduced FoV was supposed to resolve that? (We don't upsample.)

And I agree that the vast majority of the change will be HMC.

That's the thing: reduced FoV optimizes the tilt of the axial plane to minimize the number of slices you acquire, right.

If you bring that reduced FoV to the T1w, which is usually acquired to have the AC PC axis more or less aligned with the axial plane, then the projections of the brain mask to the planes are about the same in X and Y, but a lot larger in Z. This is because the axial plane of BOLD will be very tilted w.r.t. the axial plane of the T1w.

As a result: you have an image of the same resolution of the original BOLD, with about the same number of pixels in X and also in Y, but a lot more in Z. If the time-series is long, then the size of the image grows quickly.

Finally, an as I commented below w.r.t. the mask, the problems we were having with the limited FoV were derived from mapping those partial FoV to T1 and the poor coverage of the T1 brain mask. If you keep things in BOLD space, then you don't suffer from that problem.

effigies · 2017-11-02T18:41:08Z

fmriprep/workflows/bold/confounds.py

-            BOLD series mask in T1w space
+        t1_bold_xform
+            Affine matrix that maps the T1w space into alignment with
+            the native BOLD space


I think you're going to re-introduce issues with partial-FoV BOLD series if you're not masking with the BOLD mask after resampling the masks created in T1w space. (I think it's fine to use this transform. But I don't think it replaces masking.)

Oh, I see. I'll make sure masks do not go off limits with partial-FoV in BOLD space.

Actually, I'm thinking this doesn't make any difference: we work on BOLD space (so it is still partial if it was partial in first place). Nothing will be considered if it is off FoV.

Yeah, that seems correct. I suppose I was thinking we were in bold_space-T1w still.

oesteban · 2017-11-02T18:56:19Z

I checked on the resulting _confounds.tsv files and compare to the former version. Everything is quite close to the originals, except for the vx-stdDVARS which are very different.

Since now we are working on native space, I'm inclined to think that the new values of the voxelwise standardized dvars are more trustworthy.

oesteban · 2017-11-02T19:17:46Z

Confirmed this fixes #776.

effigies · 2017-11-02T19:38:14Z

fmriprep/interfaces/utils.py

    in_files = InputMultiPath(File(exists=True), mandatory=True, desc='input list of ROIs')
-    ref_header = File(exists=True, mandatory=True,
-                      desc='reference NIfTI file with desired output header/affine')
+    indexes = traits.List(traits.Int, desc='select specific maps')


Grammar nitpick: indices

effigies · 2017-11-02T19:51:40Z

fmriprep/interfaces/utils.py


+        for idx in indexes[1:]:
+            data += nb.load(in_files[idx]).get_data()


I see. If you're not trying to save memory and you really want the sum and not the logical OR (the docstring needs fixing, btw), you can replace a lot of this logic (up to this point):

def _run_interface(self, runtime): indices = self.inputs.indices or slice(None) in_files = np.array(self.inputs.in_files)[indices] if len(in_files) == 1: self._results['out_file'] = in_files[0] return runtime im = nb.concat_images(in_files) data = im.get_data().sum(axis=3)

effigies · 2017-11-02T19:57:19Z

fmriprep/workflows/bold/base.py

@@ -549,9 +562,60 @@ def init_func_preproc_wf(bold_file, ignore, freesurfer,
                    ('outputnode.bold_mask', 'inputnode.bold_mask')]),
            ])

+    # Apply transforms in 1 shot
+    # Only use uncompressed output if AROMA is to be run
+    bold_bold_trans_wf = init_bold_preproc_trans_wf(


Why would resampled BOLD on T1 be bigger? I thought the reduced FoV was supposed to resolve that? (We don't upsample.)

And I agree that the vast majority of the change will be HMC.

oesteban added 13 commits October 31, 2017 15:33

revise memory annotations adding granularity

d2d1dab

annotations revision

61a6feb

fix error

0cfffcf

fix failing workflows in docs [docs only]

4073b0c

add a new bold resampling workflow

84caba8

add a new bold resampling workflow (missing change)

ca3ffdc

move confounds calculation to native space

3d56ac5

fix connection

87366f1

new workflow should generate a new reference

37bc9d9

fix merge output

d140328

get docs to build

08d807d

fixing documentation issues

ff78055

Merge remote-tracking branch 'upstream/master' into fix/memory-issues…

e03cfb0

…-revisit

oesteban changed the title ~~[ENH] Efforts towards keeping memory low~~ [WIP,ENH] Efforts towards keeping memory low Nov 1, 2017

oesteban added 3 commits November 1, 2017 16:35

add a new reportlet

fc25c04

massage confounds masks in T1w space, then resample

4cc627e

[skip ci] pacify flakes8

b8b5696

oesteban changed the title ~~[WIP,ENH] Efforts towards keeping memory low~~ [ENH] Efforts towards keeping memory low Nov 2, 2017

add CSF columns to _confounds.tsv - close nipreps#702

74de82a

oesteban mentioned this pull request Nov 2, 2017

aCompCor - numpy.linalg.linalg.LinAlgError: SVD did not converge #776

Closed

effigies reviewed Nov 2, 2017

View reviewed changes

oesteban added 3 commits November 2, 2017 12:44

cleanup of unused functions

64ce662

mask confounds ROIs with the bold mask

e6bc9af

[skip ci] pacify flake8

e7b8357

effigies reviewed Nov 2, 2017

View reviewed changes

effigies mentioned this pull request Nov 2, 2017

workflow error: node acompcor failed to run on host [fmriprep] #812

Closed

final comments

6d5167f

oesteban merged commit 3166647 into nipreps:master Nov 2, 2017

oesteban deleted the fix/memory-issues-revisit branch November 2, 2017 22:08

effigies mentioned this pull request Nov 8, 2017

FIX: Immediately fix headers of AFNI outputs #818

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[ENH] Efforts towards keeping memory low #807

[ENH] Efforts towards keeping memory low #807

oesteban commented Oct 31, 2017 •

edited

oesteban commented Nov 2, 2017

effigies left a comment •

edited

effigies Nov 2, 2017

oesteban Nov 2, 2017

effigies Nov 2, 2017

effigies Nov 2, 2017 •

edited

effigies Nov 2, 2017

oesteban Nov 2, 2017

effigies Nov 2, 2017

oesteban Nov 2, 2017

effigies Nov 2, 2017

oesteban Nov 2, 2017

effigies Nov 2, 2017

oesteban Nov 2, 2017

effigies Nov 2, 2017

oesteban Nov 2, 2017

oesteban Nov 2, 2017

oesteban Nov 2, 2017

effigies Nov 2, 2017

oesteban commented Nov 2, 2017

oesteban commented Nov 2, 2017

effigies Nov 2, 2017

effigies Nov 2, 2017 •

edited

effigies Nov 2, 2017


		for idx in indexes[1:]:
		data += nb.load(in_files[idx]).get_data()

[ENH] Efforts towards keeping memory low #807

[ENH] Efforts towards keeping memory low #807

Conversation

oesteban commented Oct 31, 2017 • edited

oesteban commented Nov 2, 2017

effigies left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

effigies Nov 2, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oesteban commented Nov 2, 2017

oesteban commented Nov 2, 2017

Choose a reason for hiding this comment

effigies Nov 2, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oesteban commented Oct 31, 2017 •

edited

effigies left a comment •

edited

effigies Nov 2, 2017 •

edited

effigies Nov 2, 2017 •

edited