From e64860d83d8898a0047047a889207af6f40edcc6 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Wed, 12 Dec 2018 12:07:04 -0500 Subject: [PATCH 01/13] PIN: effigies/nipype feature branch --- docs/environment.yml | 1 + fmriprep/__about__.py | 2 ++ requirements.txt | 1 + 3 files changed, 4 insertions(+) diff --git a/docs/environment.yml b/docs/environment.yml index 3d770429b..9f7fac6c5 100644 --- a/docs/environment.yml +++ b/docs/environment.yml @@ -31,3 +31,4 @@ dependencies: - nilearn - niworkflows>=0.5.1,<0.5.2 - tedana>=0.0.5 + - git+https://github.com/effigies/nipype.git@16af71ec3ca85ba55893f6b74cb5366ec8aa7fef#egg=nipype diff --git a/fmriprep/__about__.py b/fmriprep/__about__.py index f4527549a..095569356 100644 --- a/fmriprep/__about__.py +++ b/fmriprep/__about__.py @@ -102,6 +102,8 @@ LINKS_REQUIRES = [ + 'git+https://github.com/effigies/nipype.git@' + '16af71ec3ca85ba55893f6b74cb5366ec8aa7fef#egg=nipype', ] TESTS_REQUIRES = [ diff --git a/requirements.txt b/requirements.txt index 500c7874e..2163c533a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,3 +2,4 @@ niworkflows>=0.5.1,<0.5.2 grabbit==0.2.3 pybids==0.6.5 versioneer +git+https://github.com/effigies/nipype.git@16af71ec3ca85ba55893f6b74cb5366ec8aa7fef#egg=nipype From 15dd37f59012eb03e5a5b0b2ce98f1847620bd4f Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Wed, 12 Dec 2018 12:08:39 -0500 Subject: [PATCH 02/13] FIX: Output NaN columns for CompCor on failure --- fmriprep/workflows/bold/confounds.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fmriprep/workflows/bold/confounds.py b/fmriprep/workflows/bold/confounds.py index 57f7235af..ecd88e767 100755 --- a/fmriprep/workflows/bold/confounds.py +++ b/fmriprep/workflows/bold/confounds.py @@ -180,12 +180,12 @@ def init_bold_confs_wf(mem_gb, metadata, name="bold_confs_wf"): # a/t-CompCor tcompcor = pe.Node( TCompCor(components_file='tcompcor.tsv', header_prefix='t_comp_cor_', pre_filter='cosine', - save_pre_filter=True, percentile_threshold=.05), + save_pre_filter=True, percentile_threshold=.05, failure_mode='NaN'), name="tcompcor", mem_gb=mem_gb) acompcor = pe.Node( ACompCor(components_file='acompcor.tsv', header_prefix='a_comp_cor_', pre_filter='cosine', - save_pre_filter=True), + save_pre_filter=True, failure_mode='NaN'), name="acompcor", mem_gb=mem_gb) # Set TR if present From 339c46e63721dec358ce1225b6eca6cfa215b2f8 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Fri, 14 Dec 2018 12:31:25 -0500 Subject: [PATCH 03/13] RF: Add warning snippet when CompCor fails --- fmriprep/interfaces/patches.py | 117 +++++++++++++++++++++++++-------- 1 file changed, 89 insertions(+), 28 deletions(-) diff --git a/fmriprep/interfaces/patches.py b/fmriprep/interfaces/patches.py index 1790efbaf..281fd17a1 100644 --- a/fmriprep/interfaces/patches.py +++ b/fmriprep/interfaces/patches.py @@ -12,49 +12,110 @@ from numpy.linalg.linalg import LinAlgError from nipype.algorithms import confounds as nac +from nipype.interfaces.base import (File, traits) +from nipype.interfaces.mixins import reporting -class RobustACompCor(nac.ACompCor): - """ - Runs aCompCor several times if it suddenly fails with - https://github.com/poldracklab/fmriprep/issues/776 +class RetryCompCorInputSpecMixin(reporting.ReportCapableInputSpec): + out_report = File('report.html', usedefault=True, hash_files=False, + desc='filename for warning HTML snippet') + # 'NaN` by default + failure_mode = traits.Enum( + 'NaN', 'error', + usedefault=True, + desc='When no components are found or convergence fails, raise an error ' + 'or silently return columns of NaNs.') - """ +class RetryCompCorMixin(reporting.ReportCapableInterface): def _run_interface(self, runtime): + warn = self.inputs.failure_mode == 'NaN' + failures = 0 + save_exc = None while True: + success = True + # Identifiy success/failure in both error and NaN mode try: - runtime = super(RobustACompCor, self)._run_interface(runtime) + runtime = super()._run_interface(runtime) + if warn and self._check_nans(): + success = False + except LinAlgError as exc: + success = False + save_exc = exc + + if success: break - except LinAlgError: - failures += 1 - if failures > 10: - raise - start = (failures - 1) * 10 - sleep(randint(start + 4, start + 10)) + + failures += 1 + if failures > 10: + if warn: + break + raise save_exc + start = (failures - 1) * 10 + sleep(randint(start + 4, start + 10)) return runtime + def _check_nans(self): + import numpy as np + outputs = self._list_outputs() + components = np.loadtxt(outputs['components_file'], skiprows=1) + return np.isnan(components).all() + + def _generate_report(self): + snippet = '' + if self._check_nans(): + snippet = '''\ +

+ Warning: {} components could not be estimated, due to a linear algebra error. + While not definitive, this may be an indication of a poor mask. + Please inspect the {} contours above to ensure that they are located + in the white matter/CSF. +

+'''.format(self._header, 'magenta' if self._header[0] == 'a' else 'blue') + + with open(self._out_report, 'w') as fobj: + fobj.write(snippet) -class RobustTCompCor(nac.TCompCor): + +class RobustACompCorInputSpec(RetryCompCorInputSpecMixin, nac.ACompCorInputSpec): + pass + + +class RobustACompCorOutputSpec(reporting.ReportCapableOutputSpec, nac.ACompCorOutputSpec): + pass + + +class RobustACompCor(RetryCompCorMixin, nac.ACompCor): """ - Runs tCompCor several times if it suddenly fails with - https://github.com/poldracklab/fmriprep/issues/940 + Runs aCompCor several times if it suddenly fails with + https://github.com/poldracklab/fmriprep/issues/776 + + Warns by default, rather than failing, on linear algebra errors. + https://github.com/poldracklab/fmriprep/issues/1433 """ + input_spec = RobustACompCorInputSpec + output_spec = RobustACompCorOutputSpec - def _run_interface(self, runtime): - failures = 0 - while True: - try: - runtime = super(RobustTCompCor, self)._run_interface(runtime) - break - except LinAlgError: - failures += 1 - if failures > 10: - raise - start = (failures - 1) * 10 - sleep(randint(start + 4, start + 10)) - return runtime +class RobustTCompCorInputSpec(RetryCompCorInputSpecMixin, nac.ACompCorInputSpec): + pass + + +class RobustTCompCorOutputSpec(reporting.ReportCapableOutputSpec, nac.TCompCorOutputSpec): + pass + + +class RobustTCompCor(RetryCompCorMixin, nac.ACompCor): + """ + Runs tCompCor several times if it suddenly fails with + https://github.com/poldracklab/fmriprep/issues/776 + + Warns by default, rather than failing, on linear algebra errors. + https://github.com/poldracklab/fmriprep/issues/1433 + + """ + input_spec = RobustTCompCorInputSpec + output_spec = RobustTCompCorOutputSpec From ba43f412f7f7ba315340fea49f02bafc431a06dc Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Fri, 14 Dec 2018 12:41:05 -0500 Subject: [PATCH 04/13] RPT: Add warnings to report --- fmriprep/viz/config.json | 10 ++++++++++ fmriprep/workflows/bold/confounds.py | 12 ++++++++++++ 2 files changed, 22 insertions(+) diff --git a/fmriprep/viz/config.json b/fmriprep/viz/config.json index 235ab5ce7..72eafdea0 100644 --- a/fmriprep/viz/config.json +++ b/fmriprep/viz/config.json @@ -98,6 +98,16 @@ "title": "ROIs in BOLD space", "description": "Brain mask calculated on the BOLD signal (red contour), along with the masks used for a/tCompCor.
The aCompCor mask (magenta contour) is a conservative CSF and white-matter mask for extracting physiological and movement confounds.
The fCompCor mask (blue contour) contains the top 5% most variable voxels within a heavily-eroded brain-mask." }, + { + "name": "epi/warn_tcompcor", + "file_pattern": "func/.*_acompcor\\.", + "raw": true + }, + { + "name": "epi/warn_tcompcor", + "file_pattern": "func/.*_tcompcor\\.", + "raw": true + }, { "name": "epi_mean_t1_registration/flirt", "file_pattern": "func/.*_flirtnobbr", diff --git a/fmriprep/workflows/bold/confounds.py b/fmriprep/workflows/bold/confounds.py index ecd88e767..59c1ff661 100755 --- a/fmriprep/workflows/bold/confounds.py +++ b/fmriprep/workflows/bold/confounds.py @@ -220,6 +220,16 @@ def init_bold_confs_wf(mem_gb, metadata, name="bold_confs_wf"): name='ds_report_bold_rois', run_without_submitting=True, mem_gb=DEFAULT_MEMORY_MIN_GB) + ds_warn_acompcor = pe.Node( + DerivativesDataSink(suffix='acompcor'), + name='ds_warn_acompcor', run_without_submitting=True, + mem_gb=DEFAULT_MEMORY_MIN_GB) + + ds_warn_tcompcor = pe.Node( + DerivativesDataSink(suffix='tcompcor'), + name='ds_warn_tcompcor', run_without_submitting=True, + mem_gb=DEFAULT_MEMORY_MIN_GB) + def _pick_csf(files): return files[0] @@ -300,6 +310,8 @@ def _pick_wm(files): (acc_msk, mrg_compcor, [('out', 'in2')]), (mrg_compcor, rois_plot, [('out', 'in_rois')]), (rois_plot, ds_report_bold_rois, [('out_report', 'in_file')]), + (acompcor, ds_warn_acompcor, [('out_report', 'in_file')]), + (tcompcor, ds_warn_tcompcor, [('out_report', 'in_file')]), ]) return workflow From 58968ee7347d7f4c3f1640a6e480b212a9fbf61a Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Fri, 14 Dec 2018 12:43:27 -0500 Subject: [PATCH 05/13] RPT: Add comment so source can be checked in tests --- fmriprep/interfaces/patches.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fmriprep/interfaces/patches.py b/fmriprep/interfaces/patches.py index 281fd17a1..0f4e078d5 100644 --- a/fmriprep/interfaces/patches.py +++ b/fmriprep/interfaces/patches.py @@ -64,7 +64,7 @@ def _check_nans(self): return np.isnan(components).all() def _generate_report(self): - snippet = '' + snippet = ''.format(self._header) if self._check_nans(): snippet = '''\

From da0a79bf4945ecb5bfe42446ee3874327dba9294 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Fri, 14 Dec 2018 17:27:09 -0500 Subject: [PATCH 06/13] FIX: Bad superclasses --- fmriprep/interfaces/patches.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fmriprep/interfaces/patches.py b/fmriprep/interfaces/patches.py index 0f4e078d5..711b0b96f 100644 --- a/fmriprep/interfaces/patches.py +++ b/fmriprep/interfaces/patches.py @@ -79,11 +79,11 @@ def _generate_report(self): fobj.write(snippet) -class RobustACompCorInputSpec(RetryCompCorInputSpecMixin, nac.ACompCorInputSpec): +class RobustACompCorInputSpec(RetryCompCorInputSpecMixin, nac.CompCorInputSpec): pass -class RobustACompCorOutputSpec(reporting.ReportCapableOutputSpec, nac.ACompCorOutputSpec): +class RobustACompCorOutputSpec(reporting.ReportCapableOutputSpec, nac.CompCorOutputSpec): pass @@ -100,7 +100,7 @@ class RobustACompCor(RetryCompCorMixin, nac.ACompCor): output_spec = RobustACompCorOutputSpec -class RobustTCompCorInputSpec(RetryCompCorInputSpecMixin, nac.ACompCorInputSpec): +class RobustTCompCorInputSpec(RetryCompCorInputSpecMixin, nac.TCompCorInputSpec): pass From 84c51d1626128855a67df4e190aa2841a7103f21 Mon Sep 17 00:00:00 2001 From: Oscar Esteban Date: Sat, 15 Dec 2018 07:34:27 -0500 Subject: [PATCH 07/13] STY: _check_nans -> _is_allnans Co-Authored-By: effigies --- fmriprep/interfaces/patches.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fmriprep/interfaces/patches.py b/fmriprep/interfaces/patches.py index 711b0b96f..f52a2f301 100644 --- a/fmriprep/interfaces/patches.py +++ b/fmriprep/interfaces/patches.py @@ -38,7 +38,7 @@ def _run_interface(self, runtime): # Identifiy success/failure in both error and NaN mode try: runtime = super()._run_interface(runtime) - if warn and self._check_nans(): + if warn and self._is_allnans(): success = False except LinAlgError as exc: success = False @@ -57,7 +57,7 @@ def _run_interface(self, runtime): return runtime - def _check_nans(self): + def _is_allnans(self): import numpy as np outputs = self._list_outputs() components = np.loadtxt(outputs['components_file'], skiprows=1) @@ -65,7 +65,7 @@ def _check_nans(self): def _generate_report(self): snippet = ''.format(self._header) - if self._check_nans(): + if self._is_allnans(): snippet = '''\

Warning: {} components could not be estimated, due to a linear algebra error. From 31da5ac8a8c476fbcf17e5dacf14c6195fb58315 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Sat, 15 Dec 2018 08:00:42 -0500 Subject: [PATCH 08/13] RF: Drop failure_mode default reordering --- fmriprep/interfaces/patches.py | 6 ------ 1 file changed, 6 deletions(-) diff --git a/fmriprep/interfaces/patches.py b/fmriprep/interfaces/patches.py index f52a2f301..ed96fd9f3 100644 --- a/fmriprep/interfaces/patches.py +++ b/fmriprep/interfaces/patches.py @@ -19,12 +19,6 @@ class RetryCompCorInputSpecMixin(reporting.ReportCapableInputSpec): out_report = File('report.html', usedefault=True, hash_files=False, desc='filename for warning HTML snippet') - # 'NaN` by default - failure_mode = traits.Enum( - 'NaN', 'error', - usedefault=True, - desc='When no components are found or convergence fails, raise an error ' - 'or silently return columns of NaNs.') class RetryCompCorMixin(reporting.ReportCapableInterface): From bc52a78eb7ea2c31cff905b8aacbd273ec5b60a2 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Sat, 15 Dec 2018 08:00:58 -0500 Subject: [PATCH 09/13] FIX: Generate CompCor reports --- fmriprep/workflows/bold/confounds.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fmriprep/workflows/bold/confounds.py b/fmriprep/workflows/bold/confounds.py index 59c1ff661..99934f437 100755 --- a/fmriprep/workflows/bold/confounds.py +++ b/fmriprep/workflows/bold/confounds.py @@ -180,12 +180,13 @@ def init_bold_confs_wf(mem_gb, metadata, name="bold_confs_wf"): # a/t-CompCor tcompcor = pe.Node( TCompCor(components_file='tcompcor.tsv', header_prefix='t_comp_cor_', pre_filter='cosine', - save_pre_filter=True, percentile_threshold=.05, failure_mode='NaN'), + save_pre_filter=True, percentile_threshold=.05, failure_mode='NaN', + generate_report=True), name="tcompcor", mem_gb=mem_gb) acompcor = pe.Node( ACompCor(components_file='acompcor.tsv', header_prefix='a_comp_cor_', pre_filter='cosine', - save_pre_filter=True, failure_mode='NaN'), + save_pre_filter=True, failure_mode='NaN', generate_report=True), name="acompcor", mem_gb=mem_gb) # Set TR if present From c083051c1d0b4483a328ae28d66e500c99ded750 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Sat, 15 Dec 2018 08:29:43 -0500 Subject: [PATCH 10/13] STY: Unused import [skip ci] --- fmriprep/interfaces/patches.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fmriprep/interfaces/patches.py b/fmriprep/interfaces/patches.py index ed96fd9f3..c6655b941 100644 --- a/fmriprep/interfaces/patches.py +++ b/fmriprep/interfaces/patches.py @@ -12,7 +12,7 @@ from numpy.linalg.linalg import LinAlgError from nipype.algorithms import confounds as nac -from nipype.interfaces.base import (File, traits) +from nipype.interfaces.base import File from nipype.interfaces.mixins import reporting From 5b20a2819690e45a1230c7032d8aebdc21d2e8e8 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Sat, 15 Dec 2018 14:36:09 -0500 Subject: [PATCH 11/13] RF: Rename datasinks so source file magic applies --- fmriprep/workflows/bold/confounds.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/fmriprep/workflows/bold/confounds.py b/fmriprep/workflows/bold/confounds.py index 99934f437..281c52c1d 100755 --- a/fmriprep/workflows/bold/confounds.py +++ b/fmriprep/workflows/bold/confounds.py @@ -221,14 +221,14 @@ def init_bold_confs_wf(mem_gb, metadata, name="bold_confs_wf"): name='ds_report_bold_rois', run_without_submitting=True, mem_gb=DEFAULT_MEMORY_MIN_GB) - ds_warn_acompcor = pe.Node( + ds_report_warn_acompcor = pe.Node( DerivativesDataSink(suffix='acompcor'), - name='ds_warn_acompcor', run_without_submitting=True, + name='ds_report_warn_acompcor', run_without_submitting=True, mem_gb=DEFAULT_MEMORY_MIN_GB) - ds_warn_tcompcor = pe.Node( + ds_report_warn_tcompcor = pe.Node( DerivativesDataSink(suffix='tcompcor'), - name='ds_warn_tcompcor', run_without_submitting=True, + name='ds_report_warn_tcompcor', run_without_submitting=True, mem_gb=DEFAULT_MEMORY_MIN_GB) def _pick_csf(files): @@ -311,8 +311,8 @@ def _pick_wm(files): (acc_msk, mrg_compcor, [('out', 'in2')]), (mrg_compcor, rois_plot, [('out', 'in_rois')]), (rois_plot, ds_report_bold_rois, [('out_report', 'in_file')]), - (acompcor, ds_warn_acompcor, [('out_report', 'in_file')]), - (tcompcor, ds_warn_tcompcor, [('out_report', 'in_file')]), + (acompcor, ds_report_warn_acompcor, [('out_report', 'in_file')]), + (tcompcor, ds_report_warn_tcompcor, [('out_report', 'in_file')]), ]) return workflow From ea4f9ba156d722a2b6dd1b111f185bb6aef7cb4c Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Mon, 17 Dec 2018 11:04:38 -0500 Subject: [PATCH 12/13] FIX: Wrong superclass (again) --- fmriprep/interfaces/patches.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fmriprep/interfaces/patches.py b/fmriprep/interfaces/patches.py index c6655b941..64f1090ab 100644 --- a/fmriprep/interfaces/patches.py +++ b/fmriprep/interfaces/patches.py @@ -102,7 +102,7 @@ class RobustTCompCorOutputSpec(reporting.ReportCapableOutputSpec, nac.TCompCorOu pass -class RobustTCompCor(RetryCompCorMixin, nac.ACompCor): +class RobustTCompCor(RetryCompCorMixin, nac.TCompCor): """ Runs tCompCor several times if it suddenly fails with https://github.com/poldracklab/fmriprep/issues/776 From 74371b375b2b362b2517a51aef5b164e97488254 Mon Sep 17 00:00:00 2001 From: "Christopher J. Markiewicz" Date: Mon, 17 Dec 2018 11:05:35 -0500 Subject: [PATCH 13/13] PIN: nipype 1.1.7 --- docs/environment.yml | 3 +-- fmriprep/__about__.py | 4 +--- requirements.txt | 1 - 3 files changed, 2 insertions(+), 6 deletions(-) diff --git a/docs/environment.yml b/docs/environment.yml index 9f7fac6c5..a90962fd5 100644 --- a/docs/environment.yml +++ b/docs/environment.yml @@ -21,7 +21,7 @@ dependencies: - python-dateutil - pydot>=1.2.3 - cython -- nipype>=1.1.6 +- nipype>=1.1.7 - pip: - sphinx-argparse @@ -31,4 +31,3 @@ dependencies: - nilearn - niworkflows>=0.5.1,<0.5.2 - tedana>=0.0.5 - - git+https://github.com/effigies/nipype.git@16af71ec3ca85ba55893f6b74cb5366ec8aa7fef#egg=nipype diff --git a/fmriprep/__about__.py b/fmriprep/__about__.py index 095569356..2a52a7033 100644 --- a/fmriprep/__about__.py +++ b/fmriprep/__about__.py @@ -88,7 +88,7 @@ 'indexed_gzip>=0.8.8', 'nibabel>=2.2.1', 'nilearn', - 'nipype>=1.1.6', + 'nipype>=1.1.7', 'nitime', 'niworkflows>=0.5.1,<0.5.2', 'numpy', @@ -102,8 +102,6 @@ LINKS_REQUIRES = [ - 'git+https://github.com/effigies/nipype.git@' - '16af71ec3ca85ba55893f6b74cb5366ec8aa7fef#egg=nipype', ] TESTS_REQUIRES = [ diff --git a/requirements.txt b/requirements.txt index 2163c533a..500c7874e 100644 --- a/requirements.txt +++ b/requirements.txt @@ -2,4 +2,3 @@ niworkflows>=0.5.1,<0.5.2 grabbit==0.2.3 pybids==0.6.5 versioneer -git+https://github.com/effigies/nipype.git@16af71ec3ca85ba55893f6b74cb5366ec8aa7fef#egg=nipype