Merge pull request #2011 from FCP-INDI/fix/smoke-1.8.6-dev

🐛 Fix some bugs found via 1.8.6-dev smoke tests

.github/workflows/smoke_test_participant.yml

@@ -18,6 +18,7 @@ name: Run participant smoke test
 
 on:
   workflow_call:
+  workflow_dispatch:
 
 jobs:
   smoke_test_human:

CPAC/func_preproc/func_preproc.py

@@ -1369,7 +1369,7 @@ def bold_mask_anatomical_resampled(wf, cfg, strat_pool, pipe_num, opt=None):
     option_val='CCS_Anatomical_Refined',
     inputs=[['desc-motion_bold', 'desc-preproc_bold', 'bold'], 'desc-brain_T1w',
             ['desc-preproc_T1w', 'desc-reorient_T1w', 'T1w']],
-    outputs=['space-bold_desc-brain_mask', 'desc-ROIbrain_bold']
+    outputs=['space-bold_desc-brain_mask', 'desc-ref_bold']
 )
 def bold_mask_ccs(wf, cfg, strat_pool, pipe_num, opt=None):
     '''Generate the BOLD mask by basing it off of the anatomical brain.
@@ -1496,7 +1496,7 @@ def bold_mask_ccs(wf, cfg, strat_pool, pipe_num, opt=None):
 
     outputs = {
         'space-bold_desc-brain_mask': (intersect_mask, 'out_file'),
-        'desc-ROIbrain_bold': (example_func_brain, 'out_file')
+        'desc-ref_bold': (example_func_brain, 'out_file')
     }
 
     return (wf, outputs)

CPAC/nuisance/nuisance.py

@@ -2514,24 +2514,7 @@ def nuisance_regression(wf, cfg, strat_pool, pipe_num, opt, space, res=None):
 
     outputs : dict
     '''
-    ingress = strat_pool.check_rpool('parsed_regressors')
-    if not ingress:
-        try:
-            regressor_prov = strat_pool.get_cpac_provenance('desc-confounds_timeseries')
-            regressor_strat_name = regressor_prov[-1].split('_')[-1]
-        except KeyError:
-            raise Exception("[!] No regressors in resource pool. \n\n Try turning " \
-                            "on create_regressors or ingress_regressors.")
-    else:
-        # name regressor workflow without regressor_prov
-        regressor_strat_name = cfg.nuisance_corrections['2-nuisance_regression'][
-                'ingress_regressors']['Regressors']['Name']
-    for regressor_dct in cfg['nuisance_corrections']['2-nuisance_regression'][
-            'Regressors']:
-        if regressor_dct['Name'] == regressor_strat_name:
-            opt = regressor_dct
-            break
-
+    opt = strat_pool.regressor_dct(cfg)
     bandpass = 'Bandpass' in opt
     bandpass_before = bandpass and cfg['nuisance_corrections',
                                        '2-nuisance_regression',

CPAC/pipeline/cpac_pipeline.py

@@ -1116,14 +1116,13 @@ def connect_pipeline(wf, cfg, rpool, pipeline_blocks):
                 LOGTAIL['warnings'].append(WARNING_FREESURFER_OFF_WITH_DATA)
                 continue
             previous_nb_str = (
-                f"after node block '{previous_nb.get_name()}': "
+                f"after node block '{previous_nb.get_name()}':"
             ) if previous_nb else 'at beginning:'
             # Alert user to block that raises error
             e.args = (
                 'When trying to connect node block '
                 f"'{NodeBlock(block).get_name()}' "
-                f"to workflow '{wf}' " + previous_nb_str + e.args[0],
-            )
+                f"to workflow '{wf}' {previous_nb_str} {e.args[0]}",)
             if cfg.pipeline_setup['Debugging']['verbose']:
                 verbose_logger = getLogger('engine')
                 verbose_logger.debug(e.args[0])
@@ -1480,11 +1479,22 @@ def build_workflow(subject_id, sub_dict, cfg, pipeline_name=None,
     try:
         wf = connect_pipeline(wf, cfg, rpool, pipeline_blocks)
     except LookupError as lookup_error:
-        errorstrings = lookup_error.args[0].split('\n')
-        missing_key = errorstrings[
-            errorstrings.index('[!] C-PAC says: The listed resource is not '
-                               'in the resource pool:') + 1]
-        if missing_key.endswith('_bold') and 'func' not in sub_dict:
+        missing_key = None
+        errorstrings = [arg for arg in lookup_error.args[0].split('\n') if
+                        arg.strip()]
+        if lookup_error.args[0].startswith('When trying to connect node b'):
+            missing_key = lookup_error.args[0].split("': ")[-1]
+        for errorstring in [
+            '[!] C-PAC says: The listed resource is not in the resource pool:',
+            '[!] C-PAC says: None of the listed resources are in the resource '
+            'pool:',
+            '[!] C-PAC says: None of the listed resources in the node block '
+            'being connected exist in the resource pool.\n\nResources:'
+        ]:
+            if errorstring in lookup_error.args[0]:
+                missing_key = errorstrings[errorstrings.index(errorstring) + 1]
+        if missing_key and missing_key.endswith('_bold'
+                                                ) and 'func' not in sub_dict:
             raise FileNotFoundError(
                 'The provided pipeline configuration requires functional '
                 'data but no functional data were found for ' +

CPAC/pipeline/engine.py

@@ -22,6 +22,7 @@
 import logging
 import os
 import re
+from typing import Any, Optional, Union
 import warnings
 
 from CPAC.pipeline import \
@@ -49,6 +50,7 @@
 from CPAC.utils.monitoring import getLogger, LOGTAIL, \
                                   WARNING_FREESURFER_OFF_WITH_DATA
 from CPAC.utils.outputs import Outputs
+from CPAC.utils.typing import LIST_OR_STR, TUPLE
 from CPAC.utils.utils import check_prov_for_regtool, \
     create_id_string, get_last_prov_entry, read_json, write_output_json
 
@@ -252,6 +254,35 @@ def get_resource_from_prov(prov):
         elif isinstance(prov[-1], str):
             return prov[-1].split(':')[0]
 
+    def regressor_dct(self, cfg) -> dict:
+        """Returns the regressor dictionary for the current strategy if
+        one exists. Raises KeyError otherwise."""
+        # pylint: disable=attribute-defined-outside-init
+        if hasattr(self, '_regressor_dct'):  # memoized
+            # pylint: disable=access-member-before-definition
+            return self._regressor_dct
+        key_error = KeyError("[!] No regressors in resource pool. \n\n"
+                             "Try turning on create_regressors or "
+                             "ingress_regressors.")
+        _nr = cfg['nuisance_corrections', '2-nuisance_regression']
+        if not hasattr(self, 'regressors'):
+            self.regressors = {reg["Name"]: reg for reg in _nr['Regressors']}
+        if self.check_rpool('parsed_regressors'):  # ingressed regressor
+            # name regressor workflow without regressor_prov
+            strat_name = _nr['ingress_regressors']['Regressors']['Name']
+            if strat_name in self.regressors:
+                self._regressor_dct = self.regressors[strat_name]
+                return self._regressor_dct
+            raise key_error
+        prov = self.get_cpac_provenance('desc-confounds_timeseries')
+        strat_name_components = prov[-1].split('_')
+        for _ in list(range(prov[-1].count('_'))):
+            reg_name = '_'.join(strat_name_components[-_:])
+            if reg_name in self.regressors:
+                self._regressor_dct = self.regressors[reg_name]
+                return self._regressor_dct
+        raise key_error
+
     def set_data(self, resource, node, output, json_info, pipe_idx, node_name,
                  fork=False, inject=False):
         json_info = json_info.copy()
@@ -299,49 +330,44 @@ def set_data(self, resource, node, output, json_info, pipe_idx, node_name,
             self.pipe_list.append(new_pipe_idx)
 
         self.rpool[resource][new_pipe_idx]['data'] = (node, output)
-        self.rpool[resource][new_pipe_idx]['json'] = json_info 
+        self.rpool[resource][new_pipe_idx]['json'] = json_info
 
-    def get(self, resource, pipe_idx=None, report_fetched=False,
-            optional=False):
+    def get(self, resource: LIST_OR_STR, pipe_idx: Optional[str] = None,
+            report_fetched: Optional[bool] = False,
+            optional: Optional[bool] = False) -> Union[
+                TUPLE[Optional[dict], Optional[str]], Optional[dict]]:
         # NOTE!!!
         #   if this is the main rpool, this will return a dictionary of strats, and inside those, are dictionaries like {'data': (node, out), 'json': info}
         #   BUT, if this is a sub rpool (i.e. a strat_pool), this will return a one-level dictionary of {'data': (node, out), 'json': info} WITHOUT THE LEVEL OF STRAT KEYS ABOVE IT
-
-        info_msg = "\n\n[!] C-PAC says: None of the listed resources are in " \
-                   f"the resource pool:\n\n  {resource}\n\nOptions:\n- You " \
-                   "can enable a node block earlier in the pipeline which " \
-                   "produces these resources. Check the 'outputs:' field in " \
-                   "a node block's documentation.\n- You can directly " \
-                   "provide this required data by pulling it from another " \
-                   "BIDS directory using 'source_outputs_dir:' in the " \
-                   "pipeline configuration, or by placing it directly in " \
-                   "your C-PAC output directory.\n- If you have done these, " \
-                   "and you still get this message, please let us know " \
-                   "through any of our support channels at: " \
-                   "https://fcp-indi.github.io/\n"
-
-        if isinstance(resource, list):
-            # if a list of potential inputs are given, pick the first one
-            # found
-            for label in resource:
-                if label in self.rpool.keys():
-                    if report_fetched:
-                        return (self.rpool[label], label)
-                    return self.rpool[label]
-        else:
-            if resource not in self.rpool.keys():
-                if optional:
-                    if report_fetched:
-                        return (None, None)
-                    return None
-                raise LookupError(info_msg)
-            if report_fetched:
+        if not isinstance(resource, list):
+            resource = [resource]
+        # if a list of potential inputs are given, pick the first one
+        # found
+        for label in resource:
+            if label in self.rpool.keys():
+                _found = self.rpool[label]
                 if pipe_idx:
-                    return (self.rpool[resource][pipe_idx], resource)
-                return (self.rpool[resource], resource)
-            if pipe_idx:
-                return self.rpool[resource][pipe_idx]
-            return self.rpool[resource]
+                    _found = _found[pipe_idx]
+                if report_fetched:
+                    return _found, label
+                return _found
+        if optional:
+            if report_fetched:
+                return (None, None)
+            return None
+        raise LookupError(
+            "\n\n[!] C-PAC says: None of the listed resources are in "
+            f"the resource pool:\n\n  {resource}\n\nOptions:\n- You "
+            "can enable a node block earlier in the pipeline which "
+            "produces these resources. Check the 'outputs:' field in "
+            "a node block's documentation.\n- You can directly "
+            "provide this required data by pulling it from another "
+            "BIDS directory using 'source_outputs_dir:' in the "
+            "pipeline configuration, or by placing it directly in "
+            "your C-PAC output directory.\n- If you have done these, "
+            "and you still get this message, please let us know "
+            "through any of our support channels at: "
+            "https://fcp-indi.github.io/\n")
 
     def get_data(self, resource, pipe_idx=None, report_fetched=False,
                  quick_single=False):

CPAC/resources/configs/pipeline_config_abcd-options.yml

@@ -350,9 +350,6 @@ nuisance_corrections:
         # If using erosion, using both proportion and millimeters is not recommended.
         gm_erosion_prop: 0.6
 
-    # switch to Off if nuisance regression is off and you don't want to write out the regressors
-    create_regressors: On
-
 timeseries_extraction:
   connectivity_matrix:
 

CPAC/resources/configs/pipeline_config_blank.yml

@@ -1281,7 +1281,7 @@ nuisance_corrections:
         Columns: [global_signal]
 
     # switch to Off if nuisance regression is off and you don't want to write out the regressors
-    create_regressors: Off
+    create_regressors: On
 
     # Standard Lateral Ventricles Binary Mask
     # used in CSF mask refinement for CSF signal-related regressions

CPAC/resources/configs/pipeline_config_ccs-options.yml

@@ -286,9 +286,6 @@ nuisance_corrections:
         # If using erosion, using both proportion and millimeters is not recommended.
         gm_erosion_prop: 0.6
 
-    # switch to Off if nuisance regression is off and you don't want to write out the regressors
-    create_regressors: On
-
 timeseries_extraction:
   connectivity_matrix:
 

CPAC/resources/configs/pipeline_config_fmriprep-ingress.yml

@@ -125,6 +125,9 @@ nuisance_corrections:
       Regressors:
         Columns: [global_signal, white_matter]
 
+    # switch to Off if nuisance regression is off and you don't want to write out the regressors
+    create_regressors: Off
+
 timeseries_extraction:
   run: On
   connectivity_matrix:

CPAC/resources/configs/pipeline_config_fmriprep-options.yml

@@ -484,9 +484,6 @@ nuisance_corrections:
     #   - If ``registration_workflows: functional_registration: func_registration_to_template: apply_trasnform: using: single_step_resampling_from_stc``, this must only be set to template
     space: [template]
 
-    # switch to Off if nuisance regression is off and you don't want to write out the regressors
-    create_regressors: On
-
     # Standard Lateral Ventricles Binary Mask
     # used in CSF mask refinement for CSF signal-related regressions
     lateral_ventricles_mask:

CPAC/utils/typing.py

@@ -41,11 +41,11 @@
     from typing import Iterable, List
     LIST = List
 if sys.version_info >= (3, 10):
-    LIST_OR_STR = list | str  # pylint: disable=invalid-name
+    LIST_OR_STR = LIST[str] | str  # pylint: disable=invalid-name
     TUPLE = tuple
 else:
     from typing import Tuple
-    LIST_OR_STR = Union[list, str]  # pylint: disable=invalid-name
+    LIST_OR_STR = Union[LIST[str], str]  # pylint: disable=invalid-name
     TUPLE = Tuple
 ITERABLE = Iterable
 ConfigKeyType = Union[str, LIST[str]]