From 4c2e1024fbeabd5bc2029c5740f6354e54584740 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Tue, 18 Aug 2020 17:33:28 -0400 Subject: [PATCH 1/5] STY: black --- fmriprep/config.py | 253 ++++++++++++++++++++++++--------------------- 1 file changed, 138 insertions(+), 115 deletions(-) diff --git a/fmriprep/config.py b/fmriprep/config.py index 20b6d0ebf..cd92b6548 100644 --- a/fmriprep/config.py +++ b/fmriprep/config.py @@ -71,12 +71,14 @@ from multiprocessing import set_start_method # Disable NiPype etelemetry always -_disable_et = bool(os.getenv("NO_ET") is not None or os.getenv("NIPYPE_NO_ET") is not None) +_disable_et = bool( + os.getenv("NO_ET") is not None or os.getenv("NIPYPE_NO_ET") is not None +) os.environ["NIPYPE_NO_ET"] = "1" os.environ["NO_ET"] = "1" try: - set_start_method('forkserver') + set_start_method("forkserver") except RuntimeError: pass # context has been already set finally: @@ -95,12 +97,15 @@ if not hasattr(sys, "_is_pytest_session"): sys._is_pytest_session = False # Trick to avoid sklearn's FutureWarnings # Disable all warnings in main and children processes only on production versions -if not any(( - "+" in __version__, - __version__.endswith(".dirty"), - os.getenv("FMRIPREP_DEV", "0").lower() in ("1", "on", "true", "y", "yes") -)): +if not any( + ( + "+" in __version__, + __version__.endswith(".dirty"), + os.getenv("FMRIPREP_DEV", "0").lower() in ("1", "on", "true", "y", "yes"), + ) +): from ._warnings import logging + os.environ["PYTHONWARNINGS"] = "ignore" elif os.getenv("FMRIPREP_WARNINGS", "0").lower() in ("1", "on", "true", "y", "yes"): # allow disabling warnings on development versions @@ -109,8 +114,8 @@ else: import logging -logging.addLevelName(25, 'IMPORTANT') # Add a new level between INFO and WARNING -logging.addLevelName(15, 'VERBOSE') # Add a new level between INFO and DEBUG +logging.addLevelName(25, "IMPORTANT") # Add a new level between INFO and WARNING +logging.addLevelName(15, "VERBOSE") # Add a new level between INFO and DEBUG DEFAULT_MEMORY_MIN_GB = 0.01 @@ -120,6 +125,7 @@ # Just get so analytics track one hit from contextlib import suppress from requests import get as _get_url, ConnectionError, ReadTimeout + with suppress((ConnectionError, ReadTimeout)): _get_url("https://rig.mit.edu/et/projects/nipy/nipype", timeout=0.05) @@ -127,48 +133,53 @@ _exec_env = os.name _docker_ver = None # special variable set in the container -if os.getenv('IS_DOCKER_8395080871'): - _exec_env = 'singularity' - _cgroup = Path('/proc/1/cgroup') - if _cgroup.exists() and 'docker' in _cgroup.read_text(): - _docker_ver = os.getenv('DOCKER_VERSION_8395080871') - _exec_env = 'fmriprep-docker' if _docker_ver else 'docker' +if os.getenv("IS_DOCKER_8395080871"): + _exec_env = "singularity" + _cgroup = Path("/proc/1/cgroup") + if _cgroup.exists() and "docker" in _cgroup.read_text(): + _docker_ver = os.getenv("DOCKER_VERSION_8395080871") + _exec_env = "fmriprep-docker" if _docker_ver else "docker" del _cgroup -_fs_license = os.getenv('FS_LICENSE') -if not _fs_license and os.getenv('FREESURFER_HOME'): - _fs_home = os.getenv('FREESURFER_HOME') +_fs_license = os.getenv("FS_LICENSE") +if not _fs_license and os.getenv("FREESURFER_HOME"): + _fs_home = os.getenv("FREESURFER_HOME") if _fs_home and (Path(_fs_home) / "license.txt").is_file(): _fs_license = str(Path(_fs_home) / "license.txt") del _fs_home -_templateflow_home = Path(os.getenv( - 'TEMPLATEFLOW_HOME', - os.path.join(os.getenv('HOME'), '.cache', 'templateflow')) +_templateflow_home = Path( + os.getenv( + "TEMPLATEFLOW_HOME", os.path.join(os.getenv("HOME"), ".cache", "templateflow") + ) ) try: from psutil import virtual_memory - _free_mem_at_start = round(virtual_memory().free / 1024**3, 1) + + _free_mem_at_start = round(virtual_memory().free / 1024 ** 3, 1) except Exception: _free_mem_at_start = None -_oc_limit = 'n/a' -_oc_policy = 'n/a' +_oc_limit = "n/a" +_oc_policy = "n/a" try: # Memory policy may have a large effect on types of errors experienced - _proc_oc_path = Path('/proc/sys/vm/overcommit_memory') + _proc_oc_path = Path("/proc/sys/vm/overcommit_memory") if _proc_oc_path.exists(): - _oc_policy = { - '0': 'heuristic', '1': 'always', '2': 'never' - }.get(_proc_oc_path.read_text().strip(), 'unknown') - if _oc_policy != 'never': - _proc_oc_kbytes = Path('/proc/sys/vm/overcommit_kbytes') + _oc_policy = {"0": "heuristic", "1": "always", "2": "never"}.get( + _proc_oc_path.read_text().strip(), "unknown" + ) + if _oc_policy != "never": + _proc_oc_kbytes = Path("/proc/sys/vm/overcommit_kbytes") if _proc_oc_kbytes.exists(): _oc_limit = _proc_oc_kbytes.read_text().strip() - if _oc_limit in ('0', 'n/a') and Path('/proc/sys/vm/overcommit_ratio').exists(): - _oc_limit = '{}%'.format( - Path('/proc/sys/vm/overcommit_ratio').read_text().strip() + if ( + _oc_limit in ("0", "n/a") + and Path("/proc/sys/vm/overcommit_ratio").exists() + ): + _oc_limit = "{}%".format( + Path("/proc/sys/vm/overcommit_ratio").read_text().strip() ) except Exception: pass @@ -181,7 +192,7 @@ class _Config: def __init__(self): """Avert instantiation.""" - raise RuntimeError('Configuration type is not instantiable.') + raise RuntimeError("Configuration type is not instantiable.") @classmethod def load(cls, settings, init=True): @@ -208,7 +219,7 @@ def get(cls): out = {} for k, v in cls.__dict__.items(): - if k.startswith('_') or v is None: + if k.startswith("_") or v is None: continue if callable(getattr(cls, k)): continue @@ -259,7 +270,7 @@ class environment(_Config): class nipype(_Config): """Nipype settings.""" - crashfile_format = 'txt' + crashfile_format = "txt" """The file format for crashfiles, either text or pickle.""" get_linked_libs = False """Run NiPype's tool to enlist linked libraries for every interface.""" @@ -269,11 +280,11 @@ class nipype(_Config): """Number of processes (compute tasks) that can be run in parallel (multiprocessing only).""" omp_nthreads = None """Number of CPUs a single process can access for multithreaded execution.""" - plugin = 'MultiProc' + plugin = "MultiProc" """NiPype's execution plugin.""" plugin_args = { - 'maxtasksperchild': 1, - 'raise_insufficient': False, + "maxtasksperchild": 1, + "raise_insufficient": False, } """Settings for NiPype's execution plugin.""" resource_monitor = False @@ -285,13 +296,13 @@ class nipype(_Config): def get_plugin(cls): """Format a dictionary for Nipype consumption.""" out = { - 'plugin': cls.plugin, - 'plugin_args': cls.plugin_args, + "plugin": cls.plugin, + "plugin_args": cls.plugin_args, } - if cls.plugin in ('MultiProc', 'LegacyMultiProc'): - out['plugin_args']['n_procs'] = int(cls.nprocs) + if cls.plugin in ("MultiProc", "LegacyMultiProc"): + out["plugin_args"]["n_procs"] = int(cls.nprocs) if cls.memory_gb: - out['plugin_args']['memory_gb'] = float(cls.memory_gb) + out["plugin_args"]["memory_gb"] = float(cls.memory_gb) return out @classmethod @@ -301,28 +312,34 @@ def init(cls): # Configure resource_monitor if cls.resource_monitor: - ncfg.update_config({ - 'monitoring': { - 'enabled': cls.resource_monitor, - 'sample_frequency': '0.5', - 'summary_append': True, + ncfg.update_config( + { + "monitoring": { + "enabled": cls.resource_monitor, + "sample_frequency": "0.5", + "summary_append": True, + } } - }) + ) ncfg.enable_resource_monitor() # Nipype config (logs and execution) - ncfg.update_config({ - 'execution': { - 'crashdump_dir': str(execution.log_dir), - 'crashfile_format': cls.crashfile_format, - 'get_linked_libs': cls.get_linked_libs, - 'stop_on_first_crash': cls.stop_on_first_crash, - 'check_version': False, # disable future telemetry + ncfg.update_config( + { + "execution": { + "crashdump_dir": str(execution.log_dir), + "crashfile_format": cls.crashfile_format, + "get_linked_libs": cls.get_linked_libs, + "stop_on_first_crash": cls.stop_on_first_crash, + "check_version": False, # disable future telemetry + } } - }) + ) if cls.omp_nthreads is None: - cls.omp_nthreads = min(cls.nprocs - 1 if cls.nprocs > 1 else os.cpu_count(), 8) + cls.omp_nthreads = min( + cls.nprocs - 1 if cls.nprocs > 1 else os.cpu_count(), 8 + ) class execution(_Config): @@ -367,7 +384,7 @@ class execution(_Config): the command line) as spatial references for outputs.""" reports_only = False """Only build the reports, based on the reportlets found in a cached working directory.""" - run_uuid = '%s_%s' % (strftime('%Y%m%d-%H%M%S'), uuid4()) + run_uuid = "%s_%s" % (strftime("%Y%m%d-%H%M%S"), uuid4()) """Unique identifier of this particular run.""" participant_label = None """List of participant identifiers that are to be preprocessed.""" @@ -375,7 +392,7 @@ class execution(_Config): """Select a particular task from all available in the dataset.""" templateflow_home = _templateflow_home """The root folder of the TemplateFlow client.""" - work_dir = Path('work').absolute() + work_dir = Path("work").absolute() """Path to a working directory where intermediate results will be available.""" write_graph = False """Write out the computational graph corresponding to the planned preprocessing.""" @@ -383,16 +400,16 @@ class execution(_Config): _layout = None _paths = ( - 'anat_derivatives', - 'bids_dir', - 'bids_database_dir', - 'fs_license_file', - 'fs_subjects_dir', - 'layout', - 'log_dir', - 'output_dir', - 'templateflow_home', - 'work_dir', + "anat_derivatives", + "bids_dir", + "bids_database_dir", + "fs_license_file", + "fs_subjects_dir", + "layout", + "log_dir", + "output_dir", + "templateflow_home", + "work_dir", ) @classmethod @@ -404,24 +421,38 @@ def init(cls): if cls._layout is None: import re from bids.layout import BIDSLayout - _db_path = cls.bids_database_dir or (cls.work_dir / cls.run_uuid / 'bids_db') + + _db_path = cls.bids_database_dir or ( + cls.work_dir / cls.run_uuid / "bids_db" + ) _db_path.mkdir(exist_ok=True, parents=True) cls._layout = BIDSLayout( str(cls.bids_dir), validate=False, database_path=_db_path, reset_database=cls.bids_database_dir is None, - ignore=("code", "stimuli", "sourcedata", "models", - "derivatives", re.compile(r'^\.'))) + ignore=( + "code", + "stimuli", + "sourcedata", + "models", + "derivatives", + re.compile(r"^\."), + ), + ) cls.bids_database_dir = _db_path cls.layout = cls._layout if cls.bids_filters: from bids.layout import Query + # unserialize pybids Query enum values for acq, filters in cls.bids_filters.items(): cls.bids_filters[acq] = { - k: getattr(Query, v[7:-4]) if not isinstance(v, Query) and 'Query' in v else v - for k, v in filters.items()} + k: getattr(Query, v[7:-4]) + if not isinstance(v, Query) and "Query" in v + else v + for k, v in filters.items() + } # These variables are not necessary anymore @@ -447,7 +478,7 @@ class workflow(_Config): (positive = exact, negative = maximum).""" bold2t1w_dof = None """Degrees of freedom of the BOLD-to-T1w registration steps.""" - bold2t1w_init = 'register' + bold2t1w_init = "register" """Whether to use standard coregistration ('register') or to initialize coregistration from the BOLD image-header ('header').""" cifti_output = None @@ -505,13 +536,13 @@ class loggers: default = logging.getLogger() """The root logger.""" - cli = logging.getLogger('cli') + cli = logging.getLogger("cli") """Command-line interface logging.""" - workflow = nlogging.getLogger('nipype.workflow') + workflow = nlogging.getLogger("nipype.workflow") """NiPype's workflow logger.""" - interface = nlogging.getLogger('nipype.interface') + interface = nlogging.getLogger("nipype.interface") """NiPype's interface logger.""" - utils = nlogging.getLogger('nipype.utils') + utils = nlogging.getLogger("nipype.utils") """NiPype's utils logger.""" @classmethod @@ -525,22 +556,18 @@ def init(cls): """ from nipype import config as ncfg + _handler = logging.StreamHandler(stream=sys.stdout) - _handler.setFormatter( - logging.Formatter(fmt=cls._fmt, datefmt=cls._datefmt) - ) + _handler.setFormatter(logging.Formatter(fmt=cls._fmt, datefmt=cls._datefmt)) cls.cli.addHandler(_handler) cls.default.setLevel(execution.log_level) cls.cli.setLevel(execution.log_level) cls.interface.setLevel(execution.log_level) cls.workflow.setLevel(execution.log_level) cls.utils.setLevel(execution.log_level) - ncfg.update_config({ - 'logging': { - 'log_directory': str(execution.log_dir), - 'log_to_file': True - }, - }) + ncfg.update_config( + {"logging": {"log_directory": str(execution.log_dir), "log_to_file": True},} + ) class seeds(_Config): @@ -565,7 +592,7 @@ def init(cls): def _set_ants_seed(): """Fix random seed for antsRegistration, antsAI, antsMotionCorr""" val = random.randint(1, 65536) - os.environ['ANTS_RANDOM_SEED'] = str(val) + os.environ["ANTS_RANDOM_SEED"] = str(val) return val @@ -581,10 +608,11 @@ def from_dict(settings): def load(filename): """Load settings from file.""" from toml import loads + filename = Path(filename) settings = loads(filename.read_text()) for sectionname, configs in settings.items(): - if sectionname != 'environment': + if sectionname != "environment": section = getattr(sys.modules[__name__], sectionname) section.load(configs) init_spaces() @@ -593,23 +621,26 @@ def load(filename): def get(flat=False): """Get config as a dict.""" settings = { - 'environment': environment.get(), - 'execution': execution.get(), - 'workflow': workflow.get(), - 'nipype': nipype.get(), - 'seeds': seeds.get(), + "environment": environment.get(), + "execution": execution.get(), + "workflow": workflow.get(), + "nipype": nipype.get(), + "seeds": seeds.get(), } if not flat: return settings - return {'.'.join((section, k)): v - for section, configs in settings.items() - for k, v in configs.items()} + return { + ".".join((section, k)): v + for section, configs in settings.items() + for k, v in configs.items() + } def dumps(): """Format config into toml.""" from toml import dumps + return dumps(get()) @@ -622,11 +653,11 @@ def to_filename(filename): def init_spaces(checkpoint=True): """Initialize the :attr:`~workflow.spaces` setting.""" from niworkflows.utils.spaces import Reference, SpatialReferences + spaces = execution.output_spaces or SpatialReferences() if not isinstance(spaces, SpatialReferences): spaces = SpatialReferences( - [ref for s in spaces.split(' ') - for ref in Reference.from_string(s)] + [ref for s in spaces.split(" ") for ref in Reference.from_string(s)] ) if checkpoint and not spaces.is_cached(): @@ -634,29 +665,21 @@ def init_spaces(checkpoint=True): # Add the default standard space if not already present (required by several sub-workflows) if "MNI152NLin2009cAsym" not in spaces.get_spaces(nonstandard=False, dim=(3,)): - spaces.add( - Reference("MNI152NLin2009cAsym", {}) - ) + spaces.add(Reference("MNI152NLin2009cAsym", {})) # Ensure user-defined spatial references for outputs are correctly parsed. # Certain options require normalization to a space not explicitly defined by users. # These spaces will not be included in the final outputs. if workflow.use_aroma: # Make sure there's a normalization to FSL for AROMA to use. - spaces.add( - Reference("MNI152NLin6Asym", {"res": "2"}) - ) + spaces.add(Reference("MNI152NLin6Asym", {"res": "2"})) cifti_output = workflow.cifti_output if cifti_output: # CIFTI grayordinates to corresponding FSL-MNI resolutions. - vol_res = '2' if cifti_output == '91k' else '1' - spaces.add( - Reference("fsaverage", {"den": "164k"}) - ) - spaces.add( - Reference("MNI152NLin6Asym", {"res": vol_res}) - ) + vol_res = "2" if cifti_output == "91k" else "1" + spaces.add(Reference("fsaverage", {"den": "164k"})) + spaces.add(Reference("MNI152NLin6Asym", {"res": vol_res})) # Make the SpatialReferences object available workflow.spaces = spaces From 0e2fb76e0495dc1fc8373e64d3f9a5dcbaa924ca Mon Sep 17 00:00:00 2001 From: mathiasg Date: Tue, 18 Aug 2020 16:30:09 -0400 Subject: [PATCH 2/5] ENH: Reuse existing config Squashed from gh-2107 and rebased onto current master --- .circleci/config.yml | 2 +- fmriprep/cli/parser.py | 20 +++++++---- fmriprep/config.py | 79 ++++++++++++++++++++++++++++++++---------- 3 files changed, 74 insertions(+), 27 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index b20c4c29f..e51eadc40 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -666,7 +666,7 @@ jobs: /tmp/data/${DATASET} /tmp/${DATASET}/derivatives participant \ --fs-no-reconall --sloppy --write-graph \ --output-spaces MNI152NLin2009cAsym:res-2 anat func \ - --reports-only --run-uuid $UUID + --reports-only RET=$? set -e [[ "$RET" -eq "1" ]] diff --git a/fmriprep/cli/parser.py b/fmriprep/cli/parser.py index def28a774..f2b6a7416 100644 --- a/fmriprep/cli/parser.py +++ b/fmriprep/cli/parser.py @@ -318,6 +318,7 @@ def _bids_filter(value): ) g_conf.add_argument( "--random-seed", + dest="_random_seed", action="store", type=int, default=None, @@ -504,13 +505,6 @@ def _bids_filter(value): help="only generate reports, don't run workflows. This will only rerun report " "aggregation, not reportlet generation for specific nodes.", ) - g_other.add_argument( - "--run-uuid", - action="store", - default=None, - help="Specify UUID of previous run, to include error logs in report. " - "No effect without --reports-only.", - ) g_other.add_argument( "--write-graph", action="store_true", @@ -570,11 +564,23 @@ def _bids_filter(value): def parse_args(args=None, namespace=None): """Parse args and run further checks on the command line.""" + import os import logging from niworkflows.utils.spaces import Reference, SpatialReferences parser = _build_parser() opts = parser.parse_args(args, namespace) + + new_config = os.getenv('FMRIPREP_NO_REUSE_CONFIG') + if not new_config: + # attempt to reuse previous config + config.load_previous_config( + opts.output_dir, + opts.work_dir, + participant=opts.participant_label, + new_uuid=not opts.reports_only, + ) + config.execution.log_level = int(max(25 - 5 * opts.verbose_count, logging.DEBUG)) config.from_dict(vars(opts)) diff --git a/fmriprep/config.py b/fmriprep/config.py index cd92b6548..a01730b5a 100644 --- a/fmriprep/config.py +++ b/fmriprep/config.py @@ -77,6 +77,8 @@ os.environ["NIPYPE_NO_ET"] = "1" os.environ["NO_ET"] = "1" +CONFIG_FILENAME = "fmriprep.toml" + try: set_start_method("forkserver") except RuntimeError: @@ -86,11 +88,11 @@ # ignoring the most annoying warnings import sys import random - from uuid import uuid4 - from pathlib import Path from time import strftime - from nipype import logging as nlogging, __version__ as _nipype_ver + + from pathlib import Path + from nipype import __version__ as _nipype_ver from templateflow import __version__ as _tf_ver from . import __version__ @@ -195,15 +197,15 @@ def __init__(self): raise RuntimeError("Configuration type is not instantiable.") @classmethod - def load(cls, settings, init=True): + def load(cls, settings, init=True, ignore=None): """Store settings from a dictionary.""" + ignore = ignore or {} for k, v in settings.items(): - if v is None: + if k in ignore or v is None: continue if k in cls._paths: setattr(cls, k, Path(v).absolute()) - continue - if hasattr(cls, k): + elif hasattr(cls, k): setattr(cls, k, v) if init: @@ -384,7 +386,7 @@ class execution(_Config): the command line) as spatial references for outputs.""" reports_only = False """Only build the reports, based on the reportlets found in a cached working directory.""" - run_uuid = "%s_%s" % (strftime("%Y%m%d-%H%M%S"), uuid4()) + run_uuid = f"{strftime('%Y%m%d-%H%M%S')}_{uuid4()}" """Unique identifier of this particular run.""" participant_label = None """List of participant identifiers that are to be preprocessed.""" @@ -497,8 +499,6 @@ class workflow(_Config): """Ignore particular steps for *fMRIPrep*.""" longitudinal = False """Run FreeSurfer ``recon-all`` with the ``-logitudinal`` flag.""" - random_seed = None - """Master random seed to initialize the Pseudorandom Number Generator (PRNG)""" medial_surface_nan = None """Fill medial surface with :abbr:`NaNs (not-a-number)` when sampling.""" regressors_all_comps = None @@ -538,11 +538,11 @@ class loggers: """The root logger.""" cli = logging.getLogger("cli") """Command-line interface logging.""" - workflow = nlogging.getLogger("nipype.workflow") + workflow = logging.getLogger("nipype.workflow") """NiPype's workflow logger.""" - interface = nlogging.getLogger("nipype.interface") + interface = logging.getLogger("nipype.interface") """NiPype's interface logger.""" - utils = nlogging.getLogger("nipype.utils") + utils = logging.getLogger("nipype.utils") """NiPype's utils logger.""" @classmethod @@ -573,18 +573,19 @@ def init(cls): class seeds(_Config): """Initialize the PRNG and track random seed assignments""" + _random_seed = None master = None - """Master seed used to generate all other tracked seeds""" + """Master random seed to initialize the Pseudorandom Number Generator (PRNG)""" ants = None """Seed used for antsRegistration, antsAI, antsMotionCorr""" @classmethod def init(cls): - cls.master = workflow.random_seed + if cls._random_seed is not None: + cls.master = cls._random_seed if cls.master is None: cls.master = random.randint(1, 65536) random.seed(cls.master) # initialize the PRNG - # functions to set program specific seeds cls.ants = _set_ants_seed() @@ -601,20 +602,22 @@ def from_dict(settings): nipype.load(settings) execution.load(settings) workflow.load(settings) - seeds.init() + seeds.load(settings) loggers.init() -def load(filename): +def load(filename, skip=None): """Load settings from file.""" from toml import loads + skip = skip or {} filename = Path(filename) settings = loads(filename.read_text()) for sectionname, configs in settings.items(): if sectionname != "environment": section = getattr(sys.modules[__name__], sectionname) - section.load(configs) + ignore = skip.get(sectionname) + section.load(configs, ignore=ignore) init_spaces() @@ -683,3 +686,41 @@ def init_spaces(checkpoint=True): # Make the SpatialReferences object available workflow.spaces = spaces + + +def load_previous_config(output_dir, work_dir, participant=None, new_uuid=True): + """ + Search for existing config file, and carry over previous options. + If a participant label is specified, the output directory is searched for + the most recent config file. If no config is found, the working directory is searched. + + Returns + ------- + success : bool + Existing config found and loaded + + """ + conf = None + if participant: + # output directory + logdir = Path(output_dir) / "fmriprep" / f"sub-{participant[0]}" / "log" + for f in logdir.glob(f"**/{CONFIG_FILENAME}"): + conf = conf or f + if f.stat().st_mtime > conf.stat().st_mtime: + conf = f + + if conf is None: + # work directory + conf = Path(work_dir) / f".{CONFIG_FILENAME}" + if not conf.exists(): + conf = None + + # load existing config + if conf is not None: + # settings to avoid reusing + skip = {} if new_uuid else {"execution": ("run_uuid",)} + load(conf, skip=skip) + loggers.cli.warning(f"Loaded previous configuration file {conf}") + return True + + return False From 2ccebdf5c2efcd5e1593b41e4d8f6b10a76ded28 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Fri, 21 Aug 2020 13:49:12 -0400 Subject: [PATCH 3/5] RF: Pass config file explicitly --- .circleci/config.yml | 2 +- fmriprep/cli/parser.py | 20 ++++++++++---------- fmriprep/config.py | 40 +--------------------------------------- 3 files changed, 12 insertions(+), 50 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index e51eadc40..0bba62f12 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -666,7 +666,7 @@ jobs: /tmp/data/${DATASET} /tmp/${DATASET}/derivatives participant \ --fs-no-reconall --sloppy --write-graph \ --output-spaces MNI152NLin2009cAsym:res-2 anat func \ - --reports-only + --reports-only --config-file /tmp/${DATASET}/work/*/config.toml RET=$? set -e [[ "$RET" -eq "1" ]] diff --git a/fmriprep/cli/parser.py b/fmriprep/cli/parser.py index f2b6a7416..ae4a7ff76 100644 --- a/fmriprep/cli/parser.py +++ b/fmriprep/cli/parser.py @@ -505,6 +505,12 @@ def _bids_filter(value): help="only generate reports, don't run workflows. This will only rerun report " "aggregation, not reportlet generation for specific nodes.", ) + g_other.add_argument( + "--config-file", + action="store", + metavar="FILE", + help="Use pre-generated configuration file. Values in file will be overridden " + "by command-line arguments.") g_other.add_argument( "--write-graph", action="store_true", @@ -564,22 +570,16 @@ def _bids_filter(value): def parse_args(args=None, namespace=None): """Parse args and run further checks on the command line.""" - import os import logging from niworkflows.utils.spaces import Reference, SpatialReferences parser = _build_parser() opts = parser.parse_args(args, namespace) - new_config = os.getenv('FMRIPREP_NO_REUSE_CONFIG') - if not new_config: - # attempt to reuse previous config - config.load_previous_config( - opts.output_dir, - opts.work_dir, - participant=opts.participant_label, - new_uuid=not opts.reports_only, - ) + if opts.config_file: + skip = {} if opts.reports_only else {"execution": ("run_uuid",)} + config.load(opts.config_file, skip=skip) + config.loggers.cli.info(f"Loaded previous configuration file {opts.config_file}") config.execution.log_level = int(max(25 - 5 * opts.verbose_count, logging.DEBUG)) config.from_dict(vars(opts)) diff --git a/fmriprep/config.py b/fmriprep/config.py index a01730b5a..76b9dfd59 100644 --- a/fmriprep/config.py +++ b/fmriprep/config.py @@ -566,7 +566,7 @@ def init(cls): cls.workflow.setLevel(execution.log_level) cls.utils.setLevel(execution.log_level) ncfg.update_config( - {"logging": {"log_directory": str(execution.log_dir), "log_to_file": True},} + {"logging": {"log_directory": str(execution.log_dir), "log_to_file": True}} ) @@ -686,41 +686,3 @@ def init_spaces(checkpoint=True): # Make the SpatialReferences object available workflow.spaces = spaces - - -def load_previous_config(output_dir, work_dir, participant=None, new_uuid=True): - """ - Search for existing config file, and carry over previous options. - If a participant label is specified, the output directory is searched for - the most recent config file. If no config is found, the working directory is searched. - - Returns - ------- - success : bool - Existing config found and loaded - - """ - conf = None - if participant: - # output directory - logdir = Path(output_dir) / "fmriprep" / f"sub-{participant[0]}" / "log" - for f in logdir.glob(f"**/{CONFIG_FILENAME}"): - conf = conf or f - if f.stat().st_mtime > conf.stat().st_mtime: - conf = f - - if conf is None: - # work directory - conf = Path(work_dir) / f".{CONFIG_FILENAME}" - if not conf.exists(): - conf = None - - # load existing config - if conf is not None: - # settings to avoid reusing - skip = {} if new_uuid else {"execution": ("run_uuid",)} - load(conf, skip=skip) - loggers.cli.warning(f"Loaded previous configuration file {conf}") - return True - - return False From 9c1b50f3d0a38b311b6ba704fd8fb432113e3778 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Fri, 21 Aug 2020 15:11:53 -0400 Subject: [PATCH 4/5] WRAPPER: Mount and pass config file --- wrapper/fmriprep_docker.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/wrapper/fmriprep_docker.py b/wrapper/fmriprep_docker.py index 06072f918..3958ccf35 100755 --- a/wrapper/fmriprep_docker.py +++ b/wrapper/fmriprep_docker.py @@ -169,6 +169,7 @@ def merge_help(wrapper_help, target_help): 'bids-database-dir', 'fs-license-file', 'fs-subjects-dir', + 'config-file', 'h', 'use-plugin', 'version', @@ -300,6 +301,10 @@ def _is_file(path, parser): '--fs-subjects-dir', metavar='PATH', type=os.path.abspath, help='Path to existing FreeSurfer subjects directory to reuse. ' '(default: OUTPUT_DIR/freesurfer)') + g_wrap.add_argument( + '--config-file', metavar='PATH', type=os.path.abspath, + help="Use pre-generated configuration file. Values in file will be overridden " + "by command-line arguments.") g_wrap.add_argument( '--anat-derivatives', metavar='PATH', type=os.path.abspath, help='Path to existing sMRIPrep/fMRIPrep-anatomical derivatives to fasttrack ' @@ -441,6 +446,10 @@ def main(): command.extend(['-v', '{}:/opt/subjects'.format(opts.fs_subjects_dir)]) unknown_args.extend(['--fs-subjects-dir', '/opt/subjects']) + if opts.config_file: + command.extend(['-v', '{}:/tmp/config.toml'.format(opts.config_file)]) + unknown_args.extend(['--config-file', '/tmp/config.toml']) + if opts.anat_derivatives: command.extend(['-v', '{}:/opt/smriprep/subjects'.format(opts.anat_derivatives)]) unknown_args.extend(['--anat-derivatives', '/opt/smriprep/subjects']) From 25f610c5d80b17855151dc6fdf9c75f9681c1821 Mon Sep 17 00:00:00 2001 From: Chris Markiewicz Date: Mon, 24 Aug 2020 13:48:31 -0400 Subject: [PATCH 5/5] CI: Reuse UUID, rather than generating new --- .circleci/config.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index 0bba62f12..4be6df9d7 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -653,10 +653,10 @@ jobs: - run: name: Generate report with one artificial error command: | + set -x sudo mv /tmp/${DATASET}/derivatives/fmriprep/sub-100185.html \ /tmp/${DATASET}/derivatives/fmriprep/sub-100185_noerror.html - UUID="$(date '+%Y%m%d-%H%M%S_')$(uuidgen)" - mkdir -p /tmp/${DATASET}/derivatives/fmriprep/sub-100185/log/$UUID/ + UUID=$(grep uuid /tmp/${DATASET}/work/*/config.toml | cut -d\" -f 2) cp /tmp/src/fmriprep/fmriprep/data/tests/crash_files/*.txt \ /tmp/${DATASET}/derivatives/fmriprep/sub-100185/log/$UUID/ set +e @@ -666,7 +666,7 @@ jobs: /tmp/data/${DATASET} /tmp/${DATASET}/derivatives participant \ --fs-no-reconall --sloppy --write-graph \ --output-spaces MNI152NLin2009cAsym:res-2 anat func \ - --reports-only --config-file /tmp/${DATASET}/work/*/config.toml + --reports-only --config-file /tmp/${DATASET}/work/${UUID}/config.toml -vv RET=$? set -e [[ "$RET" -eq "1" ]]