FIX: --cifti-output missing required space (#212)

* CI: Increase machine resources, anchor job skipping

* FIX: Include full specs when parsing for MNIInfant key

* FIX: Re-add non-native MNIInfant to output spaces if generating CIFTIs

* TST: Recify configuration test

* FIX: Backport pre-crownmask fMRIPlot

* CI: Remove cifti output

Because we are working with very downsampled data, functional segmentation is very unreliable

* STY: Flake8

* doc: remove scipy restriction

* TST: Fix failing tests

* Update nibabies/utils/viz.py

Co-authored-by: Thomas Madison <[email protected]>

* ENH: Add GIFTI masking interface

* FIX: Ensure grayord surfaces are properly masked

* FIX: Return runtime duh

* ENH: Use CreateDenseTimeSeries rois for surface masking

* PIN: Avoid nipype 1.8.0

Co-authored-by: Thomas Madison <[email protected]>

Author: mgxd

.circleci/config.yml

@@ -6,6 +6,7 @@ _machine_defaults: &machine_defaults
     image: ubuntu-2004:current
     docker_layer_caching: true
   working_directory: /tmp/src/nibabies
+  resource_class: large
 
 _python_defaults: &python_defaults
   docker:
@@ -39,6 +40,28 @@ _pull_from_registry: &pull_from_registry
     docker pull localhost:5000/nibabies
     docker tag localhost:5000/nibabies nipreps/nibabies:latest
 
+_check_skip_job: &check_skip_job
+  name: Check commit message and determine if job should be skipped
+  command: |
+    set -x +e
+    COMMIT_MSG="$(git log --format='format:%s' -n 1 $CIRCLE_SHA1)"
+    DOCBUILD="$(echo ${COMMIT_MSG} | grep -i -E '^docs?(\(\w+\))?:')"
+    SKIP_BCP="$(echo ${COMMIT_MSG} | grep -i -E '\[skip[ _]?bcp\]' )"
+    # no skipping if tagged build
+    if [[ -n "$CIRCLETAG" ]]; then
+      exit 0
+    elif [[ -n "$DOCSBUILD" ]]; then  # always try to skip docs builds
+      echo "Only docs build"
+      circleci step halt
+    elif [ -n "$CHECK_PYTEST" -a -n "$SKIP_PYTEST" ]; then
+      echo "Skipping pytest"
+      circleci step halt
+    elif [ -n "$CHECK_BCP" -a -n "$SKIP_BCP" ]; then
+      echo "Skipping BCP"
+      circleci step halt
+    fi
+    echo "No skip"
+
 version: 2.1
 orbs:
   docker: circleci/[email protected]
@@ -49,13 +72,7 @@ jobs:
     <<: *machine_defaults
     steps:
       - checkout
-      - run:
-          name: Check whether build should be skipped
-          command: |
-            if [[ "$( git log --format='format:%s' -n 1 $CIRCLE_SHA1 | grep -i -E '^docs?(\(\w+\))?:' )" != "" ]]; then
-              echo "Only docs build"
-              circleci step halt
-            fi
+      - run: *check_skip_job
       - restore_cache:
           keys:
             - build-v4-{{ .Branch }}-{{ .Revision }}
@@ -154,6 +171,7 @@ jobs:
             - /tmp/docker
             - /tmp/images
 
+
   get_data:
     docker:
       - image: continuumio/miniconda3:4.10.3-alpine
@@ -214,19 +232,11 @@ jobs:
 
   test_pytest:
     <<: *machine_defaults
+    environment:
+      CHECK_PYTEST: true
     steps:
       - checkout
-      - run:
-          name: Check whether build should be skipped
-          command: |
-            if [[ "$( git log --format='format:%s' -n 1 $CIRCLE_SHA1 | grep -i -E '^docs?(\(\w+\))?:' )" != "" ]]; then
-              echo "Only docs build"
-              circleci step halt
-            fi
-            if [[ "$( git log --format=oneline -n 1 $CIRCLE_SHA1 | grep -i -E '\[skip[ _]?tests\]' )" != "" ]]; then
-              echo "Skipping pytest job"
-              circleci step halt
-            fi
+      - run: *check_skip_job
       - attach_workspace:
           at: /tmp
       - restore_cache:
@@ -279,28 +289,16 @@ jobs:
       - store_artifacts:
           path: /tmp/data/reports
 
+
   test_bcp:
+    <<: *machine_defaults
     environment:
       - FS_LICENSE: /tmp/fslicense/license.txt
       - DATASET: bcp
-    <<: *machine_defaults
+      - CHECK_BCP: true
     steps:
       - checkout
-      - run:
-          name: Check whether build should be skipped
-          command: |
-            if [[ "$( git log --format='format:%s' -n 1 $CIRCLE_SHA1 | grep -i -E '^docs?(\(\w+\))?:' )" != "" ]]; then
-              echo "Only docs build"
-              circleci step halt
-            fi
-            if [[ "$( git log --format=oneline -n 1 $CIRCLE_SHA1 | grep -i -E '\[skip[ _]?bcp\]' )" != "" ]]; then
-              echo "Skipping bcp build"
-              circleci step halt
-            fi
-            if [[ "$( git log --format=oneline -n 1 $CIRCLE_SHA1 | grep -i -E '\[no[ _-]?fasttrack\]' )" != "" ]]; then
-              touch /tmp/.nofasttrack
-              echo "Anatomical fasttrack reusing sMRIPrep's derivatives will not be used."
-            fi
+      - run: *check_skip_job
       - attach_workspace:
           at: /tmp
       - restore_cache:
@@ -345,8 +343,8 @@ jobs:
                 --fs-subjects-dir /tmp/data/${DATASET}/derivatives/infant-freesurfer \
                 --skull-strip-template UNCInfant:cohort-1 \
                 --output-spaces MNIInfant:cohort-1 func \
-                --sloppy --write-graph --mem_mb 8192 \
-                --nthreads 2 -vv --age-months 2 --sloppy \
+                --sloppy --write-graph --mem-mb 14000 \
+                --nthreads 4 -vv --age-months 2 --sloppy \
                 --derivatives /tmp/data/${DATASET}/derivatives/precomputed \
                 --output-layout bids --anat-only
       - run:
@@ -371,8 +369,8 @@ jobs:
                 --fs-subjects-dir /tmp/data/${DATASET}/derivatives/infant-freesurfer \
                 --skull-strip-template UNCInfant:cohort-1 \
                 --output-spaces MNIInfant:cohort-1 func \
-                --sloppy --write-graph --mem_mb 8192 \
-                --nthreads 2 -vv --age-months 2 --sloppy \
+                --sloppy --write-graph --mem-mb 14000 \
+                --nthreads 4 -vv --age-months 2 --sloppy \
                 --derivatives /tmp/data/${DATASET}/derivatives/precomputed \
                 --output-layout bids
       - run:
@@ -388,6 +386,7 @@ jobs:
       - store_artifacts:
           path: /tmp/bcp/derivatives
 
+
   deploy_docker_patches:
     <<: *machine_defaults
     steps:
@@ -423,6 +422,7 @@ jobs:
             docker tag nipreps/nibabies nipreps/nibabies:${CIRCLE_BRANCH#docker/}
             docker push nipreps/nibabies:${CIRCLE_BRANCH#docker/}
 
+
   deploy_docker:
     <<: *machine_defaults
     steps:
@@ -459,6 +459,7 @@ jobs:
               fi
             fi
 
+
   test_deploy_pypi:
     <<: *python_defaults
     steps:
@@ -528,6 +529,7 @@ jobs:
       - store_artifacts:
           path: /tmp/src/nibabies/wrapper/dist
 
+
   deploy_pypi:
     <<: *python_defaults
     steps:
@@ -564,6 +566,7 @@ jobs:
             export TWINE_PASSWORD=$PYPI_WRAPPER_TOKEN
             python -m twine upload wrapper/dist/nibabies*
 
+
   deployable:
     docker:
       - image: busybox:latest

docs/requirements.txt

@@ -3,5 +3,3 @@ myst_nb
 sphinx-argparse
 # Relative to repository root
 ./wrapper/
-# avoid downloading new matplotlib
-scipy ~= 1.7.3

nibabies/config.py

@@ -722,18 +722,6 @@ def init_spaces(checkpoint=True):
     if checkpoint and not spaces.is_cached():
         spaces.checkpoint()
 
-    # NiBabies' default volumetric space is MNIInfant
-    # However, if age is not provided, do not add a default template as we cannot
-    # make an assumption about cohort.
-    if (
-        not any(s.space == 'MNIInfant' for s in spaces.references)
-        and workflow.age_months is not None
-    ):
-        from .utils.misc import cohort_by_months
-
-        cohort = cohort_by_months("MNIInfant", workflow.age_months)
-        spaces.add(Reference("MNIInfant", {"cohort": cohort}))
-
     # 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.
@@ -746,6 +734,12 @@ def init_spaces(checkpoint=True):
         vol_res = "2" if workflow.cifti_output == "91k" else "1"
         spaces.add(Reference("fsaverage", {"den": "164k"}))
         spaces.add(Reference("MNI152NLin6Asym", {"res": vol_res}))
+        # Ensure a non-native version of MNIInfant is added as a target
+        if workflow.age_months is not None:
+            from .utils.misc import cohort_by_months
+
+            cohort = cohort_by_months("MNIInfant", workflow.age_months)
+            spaces.add(Reference("MNIInfant", {"cohort": cohort}))
 
     # Make the SpatialReferences object available
     workflow.spaces = spaces

nibabies/conftest.py

@@ -34,3 +34,4 @@ def data_dir():
 def set_namespace(doctest_namespace, data_dir):
     doctest_namespace["data_dir"] = data_dir
     doctest_namespace["test_data"] = Path(resource_filename("nibabies", "tests/data"))
+    doctest_namespace["Path"] = Path

nibabies/interfaces/confounds.py

@@ -367,7 +367,9 @@ class FMRISummary(SimpleInterface):
     output_spec = FMRISummaryOutputSpec
 
     def _run_interface(self, runtime):
-        from niworkflows.viz.plots import fMRIPlot
+        # Backwards-compatible fMRIPlot
+        # TODO: Replace with niworkflows next minor version
+        from ..utils.viz import fMRIPlot
 
         self._results["out_file"] = fname_presuffix(
             self.inputs.in_func, suffix="_fmriplot.svg", use_ext=False, newpath=runtime.cwd

nibabies/interfaces/gifti.py

@@ -0,0 +1,96 @@
+from pathlib import Path
+
+import nibabel as nb
+import nibabel.gifti as ngi
+import numpy as np
+from nipype.interfaces.base import (
+    BaseInterfaceInputSpec,
+    File,
+    SimpleInterface,
+    TraitedSpec,
+    traits,
+)
+
+from .. import __version__
+
+
+class _MaskGiftiInputSpec(BaseInterfaceInputSpec):
+    in_file = File(exists=True, mandatory=True, desc="Input GIFTI (n-darrays)")
+    mask_file = File(exists=True, mandatory=True, desc="Input mask (single binary darray)")
+    threshold = traits.Float(
+        desc="If mask is probabilistic, inclusion limit",
+    )
+    metadata = traits.Dict(
+        desc="Metadata to insert into GIFTI",
+    )
+
+
+class _MaskGiftiOutputSpec(TraitedSpec):
+    out_file = File(desc="Masked file")
+
+
+class MaskGifti(SimpleInterface):
+    """Mask file across GIFTI darrays"""
+
+    input_spec = _MaskGiftiInputSpec
+    output_spec = _MaskGiftiOutputSpec
+
+    def _run_interface(self, runtime):
+        self._results["out_file"] = _mask_gifti(
+            self.inputs.in_file,
+            self.inputs.mask_file,
+            threshold=self.inputs.threshold or None,
+            metadata=self.inputs.metadata,
+            newpath=runtime.cwd,
+        )
+        return runtime
+
+
+def _mask_gifti(in_file, mask_file, *, threshold=None, metadata=None, newpath=None):
+    """
+    Mask and create a GIFTI image.
+    """
+    metadata = metadata or {}
+
+    img = nb.load(in_file)
+    mask = nb.load(mask_file).agg_data()
+
+    indices = np.nonzero(mask)[0]
+    if threshold is not None:
+        indices = np.where(mask > threshold)[0]
+
+    data = img.agg_data()
+    if isinstance(data, tuple):
+        try:
+            data = np.vstack(data)
+        except Exception:
+            raise NotImplementedError(f"Tricky GIFTI: {in_file} not supported.")
+    else:
+        data = data.T
+    masked = data[:, indices]
+
+    # rather than creating new GiftiDataArrays, just modify the data directly
+    # and preserve the existing attributes
+    for i, darr in enumerate(img.darrays):
+        darr.data = masked[i]
+        darr.dims = list(masked[i].shape)
+
+    # Finalize by adding additional metadata to file
+    metad = {
+        **{"CreatedBy": f"MaskGifti (NiBabies-{__version__})"},
+        **metadata,
+    }
+    if int(nb.__version__[0]) >= 4:  # API will change in 4.0.0
+        existing_meta = img.meta or {}
+        img.meta = ngi.GiftiMetaData({**metad, **existing_meta})
+    else:
+        meta = img.meta.data or []
+        for k, v in metad.items():
+            meta.append(ngi.GiftiNVPairs(k, v))
+        img.meta.data = meta
+
+    if newpath is None:
+        newpath = Path()
+    out_file = str((Path(newpath) / f"masked_{Path(in_file).name}").absolute())
+    nb.save(img, out_file)
+    return out_file

nibabies/tests/test_config.py

@@ -102,19 +102,23 @@ def test_config_spaces():
         if s.standard and s.dim == 3
     ] == []
 
-    # but adding age will populate output spaces with a default
     config.execution.output_spaces = None
+    config.workflow.cifti_output = "91k"
     config.workflow.use_aroma = False
     config.workflow.age_months = 1
     config.init_spaces()
     spaces = config.workflow.spaces
 
-    assert [str(s) for s in spaces.get_standard(full_spec=True)] == []
+    assert [str(s) for s in spaces.get_standard(full_spec=True)] == [
+        'fsaverage:den-164k',
+        'MNI152NLin6Asym:res-2',
+    ]
+
     assert [
         format_reference((s.fullname, s.spec))
         for s in spaces.references
         if s.standard and s.dim == 3
-    ] == ['MNIInfant_cohort-1']
+    ] == ['MNI152NLin6Asym_res-2', 'MNIInfant_cohort-1']
     _reset_config()