From 2bacc6d33242ebd9330fc738c8008a62c7ebab3b Mon Sep 17 00:00:00 2001 From: delavega4 Date: Wed, 24 Apr 2019 16:11:38 -0500 Subject: [PATCH 01/12] Add smoothing level to CLI, and pass down --- fitlins/cli/run.py | 5 +++-- fitlins/interfaces/nistats.py | 1 - fitlins/workflows/base.py | 10 +++++++--- 3 files changed, 10 insertions(+), 6 deletions(-) diff --git a/fitlins/cli/run.py b/fitlins/cli/run.py index 89e59fad..db9f78b3 100755 --- a/fitlins/cli/run.py +++ b/fitlins/cli/run.py @@ -96,10 +96,11 @@ def get_parser(): help="use BOLD files with the provided description label") g_prep = parser.add_argument_group('Options for preprocessing BOLD series') - g_prep.add_argument('-s', '--smoothing', action='store', metavar="TYPE:FWHM", + g_prep.add_argument('-s', '--smoothing', action='store', metavar="TYPE:FWHM:LEVEL", help="Smooth BOLD series with FWHM mm kernel prior to fitting. " "Valid types: iso (isotropic); " - "e.g. `--smothing iso:5` will use an isotropic 5mm FWHM kernel") + "e.g. `--smothing iso:5:1` will use an isotropic 5mm" + "FWHM at the first level.") g_perfm = parser.add_argument_group('Options to handle performance') g_perfm.add_argument('--n-cpus', action='store', default=0, type=int, diff --git a/fitlins/interfaces/nistats.py b/fitlins/interfaces/nistats.py index a8bcff4b..a1b49221 100644 --- a/fitlins/interfaces/nistats.py +++ b/fitlins/interfaces/nistats.py @@ -44,7 +44,6 @@ class FirstLevelModelInputSpec(BaseInterfaceInputSpec): smoothing_fwhm = traits.Float(desc='Full-width half max (FWHM) in mm for smoothing in mask') - class FirstLevelModelOutputSpec(TraitedSpec): contrast_maps = traits.List(File) contrast_metadata = traits.List(traits.Dict) diff --git a/fitlins/workflows/base.py b/fitlins/workflows/base.py index 19bceb54..1087fde6 100644 --- a/fitlins/workflows/base.py +++ b/fitlins/workflows/base.py @@ -57,17 +57,18 @@ def init_fitlins_wf(bids_dir, derivatives, out_dir, analysis_level, space, name='getter') if smoothing: - smoothing_params = smoothing.split(':', 1) + smoothing_params = smoothing.split(':', 2) if smoothing_params[0] != 'iso': raise ValueError(f"Unknown smoothing type {smoothing_params[0]}") smoothing_fwhm = float(smoothing_params[1]) + smoothing_level = int(smoothing_params[1]) + if smoothing_level < 0: + smoothing_level = len(model_dict) - smoothing_level l1_model = pe.MapNode( FirstLevelModel(), iterfield=['session_info', 'contrast_info', 'bold_file', 'mask_file'], name='l1_model') - if smoothing: - l1_model.inputs.smoothing_fwhm = smoothing_fwhm # Set up common patterns image_pattern = 'reports/[sub-{subject}/][ses-{session}/]figures/[run-{run}/]' \ @@ -159,6 +160,9 @@ def init_fitlins_wf(bids_dir, derivatives, out_dir, analysis_level, space, level = 'l{:d}'.format(ix + 1) + if smoothing and smoothing_level == ix: + model.inputs.smoothing_fwhm = smoothing_fwhm + # TODO: No longer used at higher level, suggesting we can simply return # entities from loader as a single list select_entities = pe.Node( From e808838bb83417c2a5fcfda74412f77ea7038a1c Mon Sep 17 00:00:00 2001 From: delavega4 Date: Wed, 24 Apr 2019 16:34:11 -0500 Subject: [PATCH 02/12] Pass smoothing kernel to nistats --- fitlins/interfaces/nistats.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fitlins/interfaces/nistats.py b/fitlins/interfaces/nistats.py index a1b49221..9ff4a615 100644 --- a/fitlins/interfaces/nistats.py +++ b/fitlins/interfaces/nistats.py @@ -126,6 +126,7 @@ class SecondLevelModelInputSpec(BaseInterfaceInputSpec): stat_files = traits.List(traits.List(File(exists=True)), mandatory=True) stat_metadata = traits.List(traits.List(traits.Dict), mandatory=True) contrast_info = traits.List(traits.Dict, mandatory=True) + smoothing_fwhm = traits.Float(desc='Full-width half max (FWHM) in mm for smoothing in mask') class SecondLevelModelOutputSpec(TraitedSpec): @@ -150,7 +151,11 @@ class SecondLevelModel(NistatsBaseInterface, SimpleInterface): output_spec = SecondLevelModelOutputSpec def _run_interface(self, runtime): - model = level2.SecondLevelModel() + smoothing_fwhm = self.inputs.smoothing_fwhm + if not isdefined(smoothing_fwhm): + smoothing_fwhm = None + + model = level2.SecondLevelModel(smoothing_fwhm=smoothing_fwhm) contrast_maps = [] contrast_metadata = [] From 305c6d622187c26c943d8b27b119ed3b057a18f0 Mon Sep 17 00:00:00 2001 From: Alejandro de la Vega Date: Wed, 24 Apr 2019 16:37:05 -0500 Subject: [PATCH 03/12] Fix typo --- fitlins/cli/run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fitlins/cli/run.py b/fitlins/cli/run.py index db9f78b3..1ffaebd5 100755 --- a/fitlins/cli/run.py +++ b/fitlins/cli/run.py @@ -99,7 +99,7 @@ def get_parser(): g_prep.add_argument('-s', '--smoothing', action='store', metavar="TYPE:FWHM:LEVEL", help="Smooth BOLD series with FWHM mm kernel prior to fitting. " "Valid types: iso (isotropic); " - "e.g. `--smothing iso:5:1` will use an isotropic 5mm" + "e.g. `--smoothing iso:5:1` will use an isotropic 5mm" "FWHM at the first level.") g_perfm = parser.add_argument_group('Options to handle performance') From 78e2f39a4178c023e9cdebf13c24770a63ad301b Mon Sep 17 00:00:00 2001 From: delavega4 Date: Wed, 24 Apr 2019 20:05:05 -0500 Subject: [PATCH 04/12] Don't use pep 517 --- Dockerfile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Dockerfile b/Dockerfile index c7b092ce..a68cb5ea 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,10 +1,10 @@ # Generated by Neurodocker version 0.4.3-6-g12a2b6f # Timestamp: 2019-02-28 22:10:25 UTC -# +# # Thank you for using Neurodocker. If you discover any issues # or ways to improve this software, please submit an issue or # pull request on our GitHub repository: -# +# # https://github.com/kaczmarj/neurodocker FROM neurodebian@sha256:5fbbad8c68525b588a459092254094436aae9dc1f3920f8d871a03053b10377c @@ -100,7 +100,7 @@ RUN bash -c "source activate neuro \ && sync RUN bash -c "source activate neuro \ - && pip install --no-cache-dir -e \ + && pip install --no-use-pep517 --no-cache-dir -e \ '/src/fitlins[all]'" \ && rm -rf ~/.cache/pip/* \ && sync \ From 257c08e5c51181b08cb06e13312b5ee50644d79b Mon Sep 17 00:00:00 2001 From: Alejandro de la Vega Date: Thu, 25 Apr 2019 10:26:01 -0500 Subject: [PATCH 05/12] Check number of steps for -1 smoothing level --- fitlins/workflows/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fitlins/workflows/base.py b/fitlins/workflows/base.py index 1087fde6..61dff2c5 100644 --- a/fitlins/workflows/base.py +++ b/fitlins/workflows/base.py @@ -63,7 +63,7 @@ def init_fitlins_wf(bids_dir, derivatives, out_dir, analysis_level, space, smoothing_fwhm = float(smoothing_params[1]) smoothing_level = int(smoothing_params[1]) if smoothing_level < 0: - smoothing_level = len(model_dict) - smoothing_level + smoothing_level = len(model_dict['Steps']) - smoothing_level l1_model = pe.MapNode( FirstLevelModel(), From 5551cefa33cf870256995d367472e7d18b944f66 Mon Sep 17 00:00:00 2001 From: delavega4 Date: Mon, 29 Apr 2019 16:20:56 -0500 Subject: [PATCH 06/12] Check out Dockerfile from master --- Dockerfile | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index 54c3d279..fe2bc877 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,11 +1,10 @@ # Generated by Neurodocker version 0.5.0 # Timestamp: 2019-04-29 18:39:28 UTC # - # Thank you for using Neurodocker. If you discover any issues # or ways to improve this software, please submit an issue or # pull request on our GitHub repository: -# +# # https://github.com/kaczmarj/neurodocker FROM neurodebian@sha256:5fbbad8c68525b588a459092254094436aae9dc1f3920f8d871a03053b10377c From dc5c4c540e5dc98e7b99098676275e5aa4343b72 Mon Sep 17 00:00:00 2001 From: delavega4 Date: Tue, 30 Apr 2019 17:18:51 -0500 Subject: [PATCH 07/12] Use step level string instead of integer --- fitlins/cli/run.py | 4 ++-- fitlins/workflows/base.py | 12 +++++------- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/fitlins/cli/run.py b/fitlins/cli/run.py index 1ffaebd5..9a7bcf7c 100755 --- a/fitlins/cli/run.py +++ b/fitlins/cli/run.py @@ -96,10 +96,10 @@ def get_parser(): help="use BOLD files with the provided description label") g_prep = parser.add_argument_group('Options for preprocessing BOLD series') - g_prep.add_argument('-s', '--smoothing', action='store', metavar="TYPE:FWHM:LEVEL", + g_prep.add_argument('-s', '--smoothing', action='store', metavar="LEVEL:TYPE:FWHM", help="Smooth BOLD series with FWHM mm kernel prior to fitting. " "Valid types: iso (isotropic); " - "e.g. `--smoothing iso:5:1` will use an isotropic 5mm" + "e.g. `--smoothing dataset:iso:5` will use an isotropic 5mm" "FWHM at the first level.") g_perfm = parser.add_argument_group('Options to handle performance') diff --git a/fitlins/workflows/base.py b/fitlins/workflows/base.py index 61dff2c5..7a9e77a4 100644 --- a/fitlins/workflows/base.py +++ b/fitlins/workflows/base.py @@ -58,12 +58,10 @@ def init_fitlins_wf(bids_dir, derivatives, out_dir, analysis_level, space, if smoothing: smoothing_params = smoothing.split(':', 2) - if smoothing_params[0] != 'iso': - raise ValueError(f"Unknown smoothing type {smoothing_params[0]}") - smoothing_fwhm = float(smoothing_params[1]) - smoothing_level = int(smoothing_params[1]) - if smoothing_level < 0: - smoothing_level = len(model_dict['Steps']) - smoothing_level + if smoothing_params[1] != 'iso': + raise ValueError(f"Unknown smoothing type {smoothing_params[1]}") + smoothing_fwhm = float(smoothing_params[2]) + smoothing_level = int(smoothing_params[0]) l1_model = pe.MapNode( FirstLevelModel(), @@ -160,7 +158,7 @@ def init_fitlins_wf(bids_dir, derivatives, out_dir, analysis_level, space, level = 'l{:d}'.format(ix + 1) - if smoothing and smoothing_level == ix: + if smoothing and smoothing_level == step: model.inputs.smoothing_fwhm = smoothing_fwhm # TODO: No longer used at higher level, suggesting we can simply return From 7762d901aadeeabf318d69f38f5c464916cb18b2 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Wed, 1 May 2019 08:59:45 -0600 Subject: [PATCH 08/12] Update fitlins/cli/run.py Co-Authored-By: adelavega --- fitlins/cli/run.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fitlins/cli/run.py b/fitlins/cli/run.py index 9a7bcf7c..efa4f460 100755 --- a/fitlins/cli/run.py +++ b/fitlins/cli/run.py @@ -100,7 +100,7 @@ def get_parser(): help="Smooth BOLD series with FWHM mm kernel prior to fitting. " "Valid types: iso (isotropic); " "e.g. `--smoothing dataset:iso:5` will use an isotropic 5mm" - "FWHM at the first level.") + "FWHM on subject-level maps, before evaluating the dataset level.") g_perfm = parser.add_argument_group('Options to handle performance') g_perfm.add_argument('--n-cpus', action='store', default=0, type=int, From db5c6b88214544ee90d86bd59d01c032b7a9d4e9 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Wed, 1 May 2019 09:00:15 -0600 Subject: [PATCH 09/12] Update fitlins/workflows/base.py Co-Authored-By: adelavega --- fitlins/workflows/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fitlins/workflows/base.py b/fitlins/workflows/base.py index a2a3eba2..133bb9c7 100644 --- a/fitlins/workflows/base.py +++ b/fitlins/workflows/base.py @@ -61,7 +61,7 @@ def init_fitlins_wf(bids_dir, derivatives, out_dir, analysis_level, space, if smoothing_params[1] != 'iso': raise ValueError(f"Unknown smoothing type {smoothing_params[1]}") smoothing_fwhm = float(smoothing_params[2]) - smoothing_level = int(smoothing_params[0]) + smoothing_level = smoothing_params[0] l1_model = pe.MapNode( FirstLevelModel(), From e1264f015cb46c7cec7d60cb65db8234f80078b0 Mon Sep 17 00:00:00 2001 From: delavega4 Date: Wed, 1 May 2019 12:25:01 -0500 Subject: [PATCH 10/12] Parse new smoothing options, wit backwards compatability --- fitlins/cli/run.py | 5 +++-- fitlins/workflows/base.py | 30 ++++++++++++++++++++++++++---- 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/fitlins/cli/run.py b/fitlins/cli/run.py index efa4f460..213bc12d 100755 --- a/fitlins/cli/run.py +++ b/fitlins/cli/run.py @@ -96,10 +96,11 @@ def get_parser(): help="use BOLD files with the provided description label") g_prep = parser.add_argument_group('Options for preprocessing BOLD series') - g_prep.add_argument('-s', '--smoothing', action='store', metavar="LEVEL:TYPE:FWHM", + g_prep.add_argument('-s', '--smoothing', action='store', metavar="FWHM:LEVEL:TYPE", help="Smooth BOLD series with FWHM mm kernel prior to fitting. " "Valid types: iso (isotropic); " - "e.g. `--smoothing dataset:iso:5` will use an isotropic 5mm" + "Defaults: LEVEL='subject' TYPE='iso';" + "e.g. `--smoothing 5:dataset:iso will use an isotropic 5mm" "FWHM on subject-level maps, before evaluating the dataset level.") g_perfm = parser.add_argument_group('Options to handle performance') diff --git a/fitlins/workflows/base.py b/fitlins/workflows/base.py index 133bb9c7..934c1ab9 100644 --- a/fitlins/workflows/base.py +++ b/fitlins/workflows/base.py @@ -1,4 +1,5 @@ from pathlib import Path +import warnings from nipype.pipeline import engine as pe from nipype.interfaces import utility as niu # from nipype.interfaces import fsl @@ -58,10 +59,31 @@ def init_fitlins_wf(bids_dir, derivatives, out_dir, analysis_level, space, if smoothing: smoothing_params = smoothing.split(':', 2) - if smoothing_params[1] != 'iso': - raise ValueError(f"Unknown smoothing type {smoothing_params[1]}") - smoothing_fwhm = float(smoothing_params[2]) - smoothing_level = smoothing_params[0] + smoothing_fwhm = float(smoothing_params[0]) + if smoothing_fwhm == 'iso': + warnings.warn( + "The order of arguments for smoothing will" + "change in the next release.", FutureWarning) + smoothing_level = 'l1' + smoothing_fwhm = smoothing_params[1] + else: + if len(smoothing_params) > 1: + smoothing_level = smoothing_params[1] + else: + smoothing_level = 'l1' + + if len(smoothing_params) > 2: + if smoothing_params[2] != 'iso': + raise ValueError( + f"Unknown smoothing type {smoothing_params[1]}") + + # Check that smmoothing level exists in model + if (smoothing_level.startswith("l") and + int(smoothing_level.strip("l")) > len(model_dict)): + raise ValueError("Invalid smoothing level {smoothing_level}") + elif (smoothing_level not in + [step['Level'] for step in model_dict['Steps']]): + raise ValueError(f"Invalid smoothing level {smoothing_level}") l1_model = pe.MapNode( FirstLevelModel(), From 6be8b4f2292a28655e3b6ba8af208ceec348cfa6 Mon Sep 17 00:00:00 2001 From: delavega4 Date: Wed, 1 May 2019 12:25:56 -0500 Subject: [PATCH 11/12] Check if smoothing_level equals step number --- fitlins/workflows/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fitlins/workflows/base.py b/fitlins/workflows/base.py index 934c1ab9..43485717 100644 --- a/fitlins/workflows/base.py +++ b/fitlins/workflows/base.py @@ -180,7 +180,7 @@ def init_fitlins_wf(bids_dir, derivatives, out_dir, analysis_level, space, level = 'l{:d}'.format(ix + 1) - if smoothing and smoothing_level == step: + if smoothing and (smoothing_level == step or smoothing_level == level): model.inputs.smoothing_fwhm = smoothing_fwhm # TODO: No longer used at higher level, suggesting we can simply return From e6ab2695c9a6a9a2362fd1fb699b2717b81cd86b Mon Sep 17 00:00:00 2001 From: delavega4 Date: Wed, 1 May 2019 14:04:30 -0500 Subject: [PATCH 12/12] Stylistic improvements to smoothing logic --- fitlins/cli/run.py | 13 +++++++------ fitlins/workflows/base.py | 35 +++++++++++++++++------------------ 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/fitlins/cli/run.py b/fitlins/cli/run.py index 213bc12d..24163e63 100755 --- a/fitlins/cli/run.py +++ b/fitlins/cli/run.py @@ -96,12 +96,13 @@ def get_parser(): help="use BOLD files with the provided description label") g_prep = parser.add_argument_group('Options for preprocessing BOLD series') - g_prep.add_argument('-s', '--smoothing', action='store', metavar="FWHM:LEVEL:TYPE", - help="Smooth BOLD series with FWHM mm kernel prior to fitting. " - "Valid types: iso (isotropic); " - "Defaults: LEVEL='subject' TYPE='iso';" - "e.g. `--smoothing 5:dataset:iso will use an isotropic 5mm" - "FWHM on subject-level maps, before evaluating the dataset level.") + g_prep.add_argument('-s', '--smoothing', action='store', metavar="FWHM[:LEVEL:[TYPE]]", + help="Smooth BOLD series with FWHM mm kernel prior to fitting at LEVEL. " + "Optional analysis LEVEL (default: l1) may be specified numerically " + "(e.g., `l1`) or by name (`run`, `subject`, `session` or `dataset`). " + "Optional smoothing TYPE (default: iso) must be one of: `iso` (isotropic). " + "e.g., `--smoothing 5:dataset:iso` will perform a 5mm FWHM isotropic " + "smoothing on subject-level maps, before evaluating the dataset level.") g_perfm = parser.add_argument_group('Options to handle performance') g_perfm.add_argument('--n-cpus', action='store', default=0, type=int, diff --git a/fitlins/workflows/base.py b/fitlins/workflows/base.py index c0e0b770..520e9d94 100644 --- a/fitlins/workflows/base.py +++ b/fitlins/workflows/base.py @@ -61,28 +61,27 @@ def init_fitlins_wf(bids_dir, derivatives, out_dir, analysis_level, space, if smoothing: smoothing_params = smoothing.split(':', 2) - smoothing_fwhm = float(smoothing_params[0]) - if smoothing_fwhm == 'iso': + # Convert old style and warn; this should turn into an (informative) error around 0.5.0 + if smoothing_params[0] == 'iso': + smoothing_params = (smoothing_params[1], 'l1', smoothing_params[0]) warnings.warn( - "The order of arguments for smoothing will" - "change in the next release.", FutureWarning) - smoothing_level = 'l1' - smoothing_fwhm = smoothing_params[1] - else: - if len(smoothing_params) > 1: - smoothing_level = smoothing_params[1] - else: - smoothing_level = 'l1' - - if len(smoothing_params) > 2: - if smoothing_params[2] != 'iso': - raise ValueError( - f"Unknown smoothing type {smoothing_params[1]}") + "The format for smoothing arguments has changed. Please use " + f"{':'.join(smoothing_params)} instead of {smoothing}.", FutureWarning) + # Add defaults to simplify later logic + if len(smoothing_params) == 1: + smoothing_params.extend(('l1', 'iso')) + elif len(smoothing_params) == 2: + smoothing_params.append('iso') + + smoothing_fwhm, smoothing_level, smoothing_type = smoothing_params + smoothing_fwhm = float(smoothing_fwhm) + if smoothing_type not in ('iso'): + raise ValueError(f"Unknown smoothing type {smoothing_type}") # Check that smmoothing level exists in model if (smoothing_level.startswith("l") and int(smoothing_level.strip("l")) > len(model_dict)): - raise ValueError("Invalid smoothing level {smoothing_level}") + raise ValueError(f"Invalid smoothing level {smoothing_level}") elif (smoothing_level not in [step['Level'] for step in model_dict['Steps']]): raise ValueError(f"Invalid smoothing level {smoothing_level}") @@ -182,7 +181,7 @@ def init_fitlins_wf(bids_dir, derivatives, out_dir, analysis_level, space, level = 'l{:d}'.format(ix + 1) - if smoothing and (smoothing_level == step or smoothing_level == level): + if smoothing and smoothing_level in (step, level): model.inputs.smoothing_fwhm = smoothing_fwhm # TODO: No longer used at higher level, suggesting we can simply return