Merge pull request #23 from flywheel-apps/add-inputs

Add inputs

.gitignore

@@ -175,3 +175,5 @@ fabric.properties
 
 # vim
 .*.swp
+
+.idea/

Dockerfile

@@ -16,6 +16,7 @@ RUN python -c 'import os, json; f = open("/flywheel/v0/gear_environ.json", "w");
 RUN apt-get update && \
     curl -sL https://deb.nodesource.com/setup_12.x | bash - && \
     apt-get install -y \
+    time \
     zip \
     nodejs \
     tree && \

README.md

@@ -39,14 +39,21 @@ Because the project has been BIDS curated, all the proper T1, T2, and fMRI files
 A list of patterns (like .gitignore syntax) defining files that should be ignored by the
 bids validator.
 
+### bids-filter-file (optional)
+
+A JSON file describing custom BIDS input filters using PyBIDS.  [See here](https://fmriprep.org/en/20.2.6/faq.html#how-do-i-select-only-certain-files-to-be-input-to-fmriprep) for details.  Recommendation: start with the default and edit it to be like the example shown there.  This does the opposite of what providing a bidsignore file does: it only keeps what matches the filters.
+
 ### freesurfer_license (optional)
 Your FreeSurfer license file. [Obtaining a license is free](https://surfer.nmr.mgh.harvard.edu/registration.html).
 This file will be copied into the $FSHOME directory.  There are [three ways](https://docs.flywheel.io/hc/en-us/articles/360013235453-How-to-include-a-Freesurfer-license-file-in-order-to-run-the-fMRIPrep-gear-)
 to provide the license to this gear.  A license is required for this gear to run.
 
-## fs-subjects-dir
+### fs-subjects-dir (optional)
 Zip file of existing FreeSurfer subject's directory to reuse.  If the output of FreeSurfer recon-all is provided to fMRIPrep, that output will be used rather than re-running recon-all.  Unzipping the file should produce a particular subject's directory which will be placed in the $FREESURFER_HOME/subjects directory.  The name of the directory must match the -subjid as passed to recon-all.  This version of fMRIPrep uses Freesurfer v6.0.1.
 
+### work-dir (optional)
+THIS IS A FUTURE OPTIONAL INPUT.  It has not yet been added.  Provide intermediate fMRIPrep results as a zip file.  This file will be unzipped into the work directory so that previous results will be used instead of re-calculating them.  This option is provided so that bids-fmriprep can be run incrementally as new data is acquired.  The zip file to provide can be produced by using the gear-save-intermediate-output configuration option.  You definitely also want to use the fs-subject-dir input (above) so that FreeSurfer won't be run multiple times.
+
 ## Config:
 Most config options are identical to those used in fmriprep, and so documentation can be found here https://fmriprep.readthedocs.io/en/stable/usage.html.
 
@@ -88,9 +95,16 @@ Copy the contents of the license file and paste it into this argument.
 
 ## Troubleshooting
 
+### Resources
+
+fMRIPrep can require a large amount of memory and disk space depending on the number of acquisitions being analyzed.  There is also a trade-off between the cost of analysis and the amount of time necessary.
+There is a helpful discussion of this in the [FAQ](https://fmriprep.org/en/20.2.6/faq.html#how-much-cpu-time-and-ram-should-i-allocate-for-a-typical-fmriprep-run) and also on [NeuroStars](https://neurostars.org/t/how-much-ram-cpus-is-reasonable-to-run-pipelines-like-fmriprep/1086).  At the top of your job log, you should see the configuration of the virtual machine you are running on.  When a job finishes, the output of the GNU `time` command is placed into the "Custom Information" (metadata) on the analysis.  To see it, go to the "Analyses" tab for a project, subject, or session, click on an analysis and then on the "Custom Information" tab.
+
+### Metadata
+
 Depending upon your fMRIPrep workflow preferences, a variety of metadata and files may be required for successful execution. And because of this variation, not all cases will be caught during BIDS validation. If you are running into issues executing bids-fmriprep, we recommend reading through the configuration options explained with the [fMRIPrep Usage Notes](https://fmriprep.org/en/stable/usage.html) and double-checking the following:
 
-### Fieldmaps ([BIDS specification](https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/01-magnetic-resonance-imaging-data.html#fieldmap-data))
+#### Fieldmaps ([BIDS specification](https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/01-magnetic-resonance-imaging-data.html#fieldmap-data))
 
 - Phase encoding directions for fieldmaps and bold must be opposite.
     - Look for the `PhaseEncodingDirection` key in the file metadata (or exported JSON sidecar) and note the value (e.g., `j-`). If the fieldmap and the bold series it is `IntendedFor` do not have opposite `PhaseEncodingDirection`, fMRIPrep will error out with "ValueError: None of the discovered fieldmaps has the right phase encoding direction." BIDS specification details can be found [here](https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/01-magnetic-resonance-imaging-data.html#case-4-multiple-phase-encoded-directions-pepolar). For more information see discussions [here](https://github.com/nipreps/fmriprep/issues/1148#issuecomment-392363308) and [here](https://neurostars.org/t/phase-encoding-error-for-field-maps/2650).
@@ -131,6 +145,6 @@ Depending upon your fMRIPrep workflow preferences, a variety of metadata and fil
 
 - Note: `PhaseEncodingDirection` and `TotalReadoutTime` are typically required.
 
-### Functional ([BIDS specification](https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/01-magnetic-resonance-imaging-data.html#task-including-resting-state-imaging-data))
+#### Functional ([BIDS specification](https://bids-specification.readthedocs.io/en/stable/04-modality-specific-files/01-magnetic-resonance-imaging-data.html#task-including-resting-state-imaging-data))
 
 - Each functional NIfTI needs the following metadata: `EchoTime`, `EffectiveEchoSpacing`, `PhaseEncodingDirection`,`RepetitionTime`, `SliceTiming`, and `TaskName`.

manifest.json

@@ -148,7 +148,7 @@
     },
     "gear-run-bids-validation": {
       "default": false,
-      "description": "Gear will run BIDS validation before running fMRIPrep and print out all warnings and errors.  fMRIPrep runs a version of the bids validator so having the gear run it is not necessary, but might be informative.  If validation fails and gear-abort-on-bids-error is true, mriq will NOT be run.",
+      "description": "Gear will run BIDS validation before running fMRIPrep and print out all warnings and errors.  fMRIPrep runs a version of the bids validator so having the gear run it is not necessary, but might be informative.  If validation fails and gear-abort-on-bids-error is true, fMRIPrep will NOT be run.",
       "type": "boolean"
     },
     "gear-save-intermediate-output": {
@@ -303,12 +303,13 @@
     }
   },
   "custom": {
+    "docker-image": "flywheel/bids-fmriprep:1.2.3_20.2.6",
     "flywheel": {
       "suite": "BIDS Apps"
     },
     "gear-builder": {
       "category": "analysis",
-      "image": "flywheel/bids-fmriprep:1.2.2_20.2.6"
+      "image": "flywheel/bids-fmriprep:1.2.3_20.2.6"
     },
     "license": {
       "dependencies": [
@@ -351,6 +352,11 @@
       "description": "A .bidsignore file to provide to the bids-validator that this gear runs before running the main command.",
       "optional": true
     },
+    "bids-filter-file": {
+      "base": "file",
+      "description": "a JSON file describing custom BIDS input filters using PyBIDS.",
+      "optional": true
+    },
     "freesurfer_license": {
       "base": "file",
       "description": "FreeSurfer license file, provided during registration with FreeSurfer. This file will by copied to the $FSHOME directory and used during execution of the Gear.",
@@ -361,6 +367,11 @@
       "description": "Zip file of existing FreeSurfer subject's directory to reuse.  If the output of FreeSurfer recon-all is provided to fMRIPrep, that output will be used rather than re-running recon-all.  Unzipping the file should produce a particular subject's directory which will be placed in the $FREESURFER_HOME/subjects directory.  The name of the directory must match the -subjid as passed to recon-all.  This version of fMRIPrep uses Freesurfer v6.0.1.",
       "optional": true
     },
+    "previous-results": {
+      "base": "file",
+      "description": "Provide previously calculated fMRIPrep output results as a zip file.  This file will be unzipped into the output directory so that previous results will be used instead of re-calculating them.  This input is provided so that bids-fmriprep can be run incrementally as new data is acquired.",
+      "optional": true
+    },
     "key": {
       "base": "api-key",
       "read-only": true
@@ -372,5 +383,5 @@
   "name": "bids-fmriprep",
   "source": "https://github.com/nipreps/fmriprep",
   "url": "https://github.com/flywheel-apps/bids-fmriprep/blob/master/README.md",
-  "version": "1.2.2_20.2.6"
+  "version": "1.2.3_20.2.6"
 }

run.py

@@ -1,6 +1,7 @@
 #!/usr/bin/env python3
 """Run the gear: set up for and call command-line command."""
 
+import json
 import logging
 import os
 import re
@@ -66,6 +67,9 @@ def generate_command(config, work_dir, output_analysis_id_dir, errors, warnings)
 
     # start with the command itself:
     cmd = [
+        "/usr/bin/time",
+        "-v",
+        "--output=time_output.txt",
         BIDS_APP,
         os.path.join(work_dir, "bids"),
         str(output_analysis_id_dir),
@@ -116,7 +120,7 @@ def generate_command(config, work_dir, output_analysis_id_dir, errors, warnings)
 def main(gtk_context):
 
     FWV0 = Path.cwd()
-    log.debug("Running gear in %s", FWV0)
+    log.info("Running gear in %s", FWV0)
 
     gtk_context.log_config()
 
@@ -127,9 +131,9 @@ def main(gtk_context):
     warnings = []
 
     output_dir = gtk_context.output_dir
-    log.debug("output_dir is %s", output_dir)
+    log.info("output_dir is %s", output_dir)
     work_dir = gtk_context.work_dir
-    log.debug("work_dir is %s", work_dir)
+    log.info("work_dir is %s", work_dir)
     gear_name = gtk_context.manifest["name"]
 
     # run-time configuration options from the gear's context.json
@@ -150,6 +154,14 @@ def main(gtk_context):
     # This allows the raw output to be deleted so that a zipped archive
     # can be returned.
     output_analysis_id_dir = output_dir / destination_id
+    log.info("Creating output directory %s", output_analysis_id_dir)
+    if Path(output_analysis_id_dir).exists():
+        log.info(
+            "Not actually creating output directory %s because it exists.  This must be a test",
+            output_analysis_id_dir,
+        )
+    else:
+        Path(output_analysis_id_dir).mkdir()
 
     environ = get_and_log_environment()
 
@@ -172,15 +184,90 @@ def main(gtk_context):
         (subjects_dir / "fsaverage5").symlink_to(orig_subject_dir / "fsaverage5")
         (subjects_dir / "fsaverage6").symlink_to(orig_subject_dir / "fsaverage6")
 
+    bids_filter_file_path = gtk_context.get_input_path("bids-filter-file")
+    if bids_filter_file_path:
+        paths = list(Path("input/bids-filter-file").glob("*"))
+        log.info("Using provided PyBIDS filter file %s", str(paths[0]))
+        config["bids-filter-file"] = str(paths[0])
+
+    previous_work_zip_file_path = gtk_context.get_input_path("work-dir")
+    if previous_work_zip_file_path:
+        paths = list(Path("input/work-dir").glob("*"))
+        log.info("Using provided fMRIPrep intermediate work file %s", str(paths[0]))
+        unzip_dir = FWV0 / "unzip-work-dir"
+        unzip_dir.mkdir(parents=True)
+        unzip_archive(paths[0], unzip_dir)
+        for a_dir in unzip_dir.glob("*/*"):
+            if (
+                a_dir.name == "bids"
+            ):  # skip previous bids directory so current bids data will be used
+                log.info(
+                    "Found %s, but ignoring it to use current bids data", a_dir.name
+                )
+            else:
+                log.info("Found %s", a_dir.name)
+                a_dir.rename(FWV0 / "work" / a_dir.name)
+        hash_file = list(Path("work/fmriprep_wf/").glob("fsdir_run_*/_0x*.json"))[0]
+        if hash_file.exists():
+            with open(hash_file) as json_file:
+                data = json.load(json_file)
+                old_tmp_path = data[0][1]
+                old_tmp_name = old_tmp_path.split("/")[2]
+                log.info("Found old tmp name: %s", old_tmp_name)
+                cur_tmp_name = str(FWV0).split("/")[2]
+                # rename the directory to the old name
+                Path("/tmp/" + cur_tmp_name).replace(Path("/tmp/" + old_tmp_name))
+                # create a symbolic link using the new name to the old name just in case
+                Path("/tmp/" + cur_tmp_name).symlink_to(
+                    Path("/tmp/" + old_tmp_name), target_is_directory=True
+                )
+                # update all variables to have the old directory name in them
+                FWV0 = Path("/tmp/" + old_tmp_name + "/flywheel/v0")
+                output_dir = str(output_dir).replace(cur_tmp_name, old_tmp_name)
+                output_analysis_id_dir = Path(
+                    str(output_analysis_id_dir).replace(cur_tmp_name, old_tmp_name)
+                )
+                log.info("new output directory is: %s", output_dir)
+                work_dir = Path(str(work_dir).replace(cur_tmp_name, old_tmp_name))
+                log.info("new work directory is: %s", work_dir)
+                subjects_dir = Path(
+                    str(subjects_dir).replace(cur_tmp_name, old_tmp_name)
+                )
+                config["fs-subjects-dir"] = subjects_dir
+                log.info("new FreeSurfer subjects directory is: %s", subjects_dir)
+                # for old work to be recognized, switch to running from the old path
+                os.chdir(FWV0)
+                log.info("cd %s", FWV0)
+        else:
+            log.info("Could not find hash file")
+        config["work-dir"] = str(FWV0 / "work")
+
     subject_zip_file_path = gtk_context.get_input_path("fs-subjects-dir")
     if subject_zip_file_path:
         paths = list(Path("input/fs-subjects-dir").glob("*"))
         log.info("Using provided Freesurfer subject file %s", str(paths[0]))
-        unzip_archive(paths[0], subjects_dir)
-
-        # Add --fs-subjects-dir argument to the command
+        unzip_dir = FWV0 / "unzip-fs-subjects-dir"
+        unzip_dir.mkdir(parents=True)
+        unzip_archive(paths[0], unzip_dir)
+        for a_subject in unzip_dir.glob("*/*"):
+            if (subjects_dir / a_subject.name).exists():
+                log.info("Found %s but using existing", a_subject.name)
+            else:
+                log.info("Found %s", a_subject.name)
+                a_subject.rename(subjects_dir / a_subject.name)
         config["fs-subjects-dir"] = subjects_dir
 
+    previous_results_zip_file_path = gtk_context.get_input_path("previous-results")
+    if previous_results_zip_file_path:
+        paths = list(Path("input/previous-results").glob("*"))
+        log.info("Using provided fMRIPrep previous results file %s", str(paths[0]))
+        unzip_dir = FWV0 / "unzip-previous-results"
+        unzip_dir.mkdir(parents=True)
+        unzip_archive(paths[0], unzip_dir)
+        for a_dir in unzip_dir.glob("*/*"):
+            log.info("Found %s", a_dir.name)
+            a_dir.rename(output_analysis_id_dir / a_dir.name)
+
     environ["FS_LICENSE"] = str(FWV0 / "freesurfer/license.txt")
 
     license_list = list(Path("input/freesurfer_license").glob("*"))
@@ -205,15 +292,12 @@ def main(gtk_context):
         config, work_dir, output_analysis_id_dir, errors, warnings
     )
 
-    # This is used as part of the name of output files
-    command_name = make_file_name_safe(command[0])
-
     # Download BIDS Formatted data
     if len(errors) == 0:
 
         # Create HTML file that shows BIDS "Tree" like output
         tree = True
-        tree_title = f"{command_name} BIDS Tree"
+        tree_title = f"{gear_name} BIDS Tree"
 
         error_code = download_bids_for_runlevel(
             gtk_context,
@@ -248,13 +332,9 @@ def main(gtk_context):
             pretend_it_ran(destination_id)
 
         else:
-            # Create output directory
-            log.info("Creating output directory %s", output_analysis_id_dir)
-            Path(output_analysis_id_dir).mkdir()
-
             if config["gear-log-level"] != "INFO":
                 # show what's in the current working directory just before running
-                os.system("tree -a .")
+                os.system("tree -al .")
 
             if "gear-timeout" in config:
                 command = [f"timeout {config['gear-timeout']}"] + command
@@ -272,6 +352,28 @@ def main(gtk_context):
 
     finally:
 
+        # Save time, etc. resources used in metadata on analysis
+        if Path("time_output.txt").exists():  # some tests won't have this file
+            metadata = {
+                "analysis": {"info": {"resources used": {},},},
+            }
+            with open("time_output.txt") as file:
+                for line in file:
+                    if ":" in line:
+                        if (
+                            "Elapsed" in line
+                        ):  # special case "Elapsed (wall clock) time (h:mm:ss or m:ss): 0:08.11"
+                            sline = re.split(r"\):", line)
+                            sline[0] += ")"
+                        else:
+                            sline = line.split(":")
+                        key = sline[0].strip()
+                        val = sline[1].strip(' "\n')
+                        metadata["analysis"]["info"]["resources used"][key] = val
+            with open(f"{output_dir}/.metadata.json", "w") as fff:
+                json.dump(metadata, fff)
+                log.info(f"Wrote {output_dir}/.metadata.json")
+
         # Cleanup, move all results to the output directory
 
         # Remove all fsaverage* directories

tests/bin/pack-gear-tests.py

@@ -5,8 +5,7 @@
 This will REMOVE the test .zip file if it already exists and it
 will remove the test directory after zipping.
 
-Use "pack-tests.py all" to zip every test unless it is already
-unzipped.
+Use "pack-tests.py all" to zip every directory.
 
 Example:
     pack-tests.py hello_world

tests/conftest.py

@@ -28,10 +28,23 @@ def _method(zip_name):
 
         print("\nRemoving previous gear...")
 
-        if Path(FWV0 / "config.json").exists():
-            Path(FWV0 / "config.json").unlink()
-
-        for dir_name in ["input", "output", "work", "freesurfer", "templateflow"]:
+        for file_name in [
+            "config.json",
+            "time_output.txt",
+        ]:
+            if Path(FWV0 / file_name).exists():
+                Path(FWV0 / file_name).unlink()
+
+        for dir_name in [
+            "input",
+            "output",
+            "work",
+            "freesurfer",
+            "templateflow",
+            "unzip-work-dir",
+            "unzip-fs-subjects-dir",
+            "unzip-previous-results",
+        ]:
             path = Path(FWV0 / dir_name)
             if path.exists():
                 print(f"shutil.rmtree({str(path)}")

tests/integration_tests/test_bids_error_reports.py

@@ -1,3 +1,4 @@
+import json
 import logging
 from pathlib import Path
 from unittest import TestCase
@@ -14,6 +15,10 @@ def test_bids_error_reports(caplog, install_gear, search_caplog):
     user_json = Path(Path.home() / ".config/flywheel/user.json")
     if not user_json.exists():
         TestCase.skipTest("", f"No API key available in {str(user_json)}")
+    with open(user_json) as json_file:
+        data = json.load(json_file)
+        if "ga" not in data["key"]:
+            TestCase.skipTest("", "Not logged in to ga.")
 
     install_gear("bids_error.zip")