juaml / junifer

Forschungszentrum Jülich Neuroimaging Feature Extractor
https://juaml.github.io/junifer
GNU Affero General Public License v3.0
14 stars 13 forks source link

[BUG]: Validation failure of multiple pre-processing steps with different input data types requirement #339

Closed fraimondo closed 3 months ago

fraimondo commented 4 months ago

Is there an existing issue for this?

Current Behavior

I'm running a YAML with two pre-processing steps: confound remover + Space Warper.

The process fails because the Space Warper requires the T1w + Warp which is not outputed by the fMRIPrepConfoundRemover.

The bug is basically that: it's not accounting that the data object has the T1w and Warp objects. It just gets the output of the fMRIPrepConfoundRemover.

Expected Behavior

I would expect that the two steps can be used.

Steps To Reproduce

  1. Install latest junifer
  2. Run: junifer run --element sub-0001 NAME_OF_YAML
workdir: /tmp

datagrabber:
    kind: DataladAOMICID1000
    native_t1w: true

preprocess:
  - kind: fMRIPrepConfoundRemover
    detrend: true
    standardize: true
    strategy:
        motion: full
        wm_csf: full
        global_signal: full
    low_pass: 0.08
    high_pass: 0.01
    masks:
      - inherit
      - compute_epi_mask
      - threshold: 0
  - kind: SpaceWarper
    reference: T1w
    on: BOLD
    using: ants

markers:
  - name: FC-DMNBuckner-5mm
    kind: FunctionalConnectivitySpheres
    cor_method: correlation
    coords: DMNBuckner
    radius: 5
    allow_overlap: true
    masks: 
      - inherit

  - name: FC-Schaefer100x17
    kind: FunctionalConnectivityParcels
    cor_method: correlation
    parcellation: Schaefer100x17
    masks: 
      - inherit

storage:
  kind: HDF5FeatureStorage
  uri: /data/group/riseml/fraimondo/2024_HIP/features/ds003097_FC_native/ds003097_FC_native.hdf5

Environment

junifer:
  version: 0.0.5.dev33
python:
  version: 3.11.3
  implementation: CPython
dependencies:
  click: 8.1.7
  numpy: 1.26.4
  scipy: 1.10.1
  datalad: 0.18.2+59.gc5054cb91
  pandas: 1.5.3
  nibabel: 4.0.2
  nilearn: 0.10.0
  sqlalchemy: 1.4.48
  ruamel.yaml: 0.17.31
  httpx: 0.26.0
  tqdm: 4.66.1
  templateflow: 24.2.0
  looseversion: None
system:
  platform: Linux-6.6.13+bpo-amd64-x86_64-with-glibc2.36
environment:
  LC_CTYPE: en_US.UTF-8
  PATH: 
    /home/fraimondo/miniconda3/envs/junifer/bin:/home/fraimondo/miniconda3/condabin:/home/fraimondo/.dotfiles/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/X11R6/bin:/usr/local/games:/usr/games

Relevant log output

(junifer)cpu44 ➜  1_junifer  junifer run --element sub-0001 aomic_fc_native.yaml
2024-05-06 16:42:57,313 - JUNIFER - INFO - ===== Lib Versions =====
2024-05-06 16:42:57,314 - JUNIFER - INFO - numpy: 1.26.4
2024-05-06 16:42:57,314 - JUNIFER - INFO - scipy: 1.10.1
2024-05-06 16:42:57,314 - JUNIFER - INFO - pandas: 1.5.3
2024-05-06 16:42:57,314 - JUNIFER - INFO - nilearn: 0.10.0
2024-05-06 16:42:57,314 - JUNIFER - INFO - nibabel: 4.0.2
2024-05-06 16:42:57,314 - JUNIFER - INFO - junifer: 0.0.5.dev33
2024-05-06 16:42:57,314 - JUNIFER - INFO - ========================
2024-05-06 16:42:57,314 - JUNIFER - INFO - Parsing yaml file: /home/fraimondo/dev/events/2024_HIP/1_junifer/aomic_fc_native.yaml
2024-05-06 16:42:57,323 - JUNIFER - INFO - `datadir` is None, creating a temporary directory
2024-05-06 16:42:57,323 - JUNIFER - INFO - `datadir` set to /tmp/tmp4q265aae/datadir
2024-05-06 16:42:57,325 - JUNIFER - INFO - Validating Marker Collection
2024-05-06 16:42:57,325 - JUNIFER - INFO - DataGrabber output type: ['BOLD', 'BOLD_confounds', 'BOLD_mask', 'T1w', 'T1w_mask', 'VBM_CSF', 'VBM_GM', 'VBM_WM', 'DWI', 'Warp']
2024-05-06 16:42:57,325 - JUNIFER - INFO - Validating Data Reader:
2024-05-06 16:42:57,325 - JUNIFER - INFO - Data Reader output type: ['BOLD', 'BOLD_confounds', 'BOLD_mask', 'T1w', 'T1w_mask', 'VBM_CSF', 'VBM_GM', 'VBM_WM', 'DWI', 'Warp']
2024-05-06 16:42:57,325 - JUNIFER - INFO - Validating Preprocessor: fMRIPrepConfoundRemover
2024-05-06 16:42:57,325 - JUNIFER - INFO - Preprocess output type: ['BOLD']
2024-05-06 16:42:57,325 - JUNIFER - INFO - Validating Preprocessor: SpaceWarper
2024-05-06 16:42:58,761 - JUNIFER - WARNING - ANTs is installed but some of the required commands were not found. These are the results: {'ResampleImage': 'not found', 'antsApplyTransforms': 'not found'}
/home/fraimondo/dev/tbox/junifer/junifer/pipeline/utils.py:242: RuntimeWarning: ANTs is installed but some of the required commands were not found. These are the results: {'ResampleImage': 'not found', 'antsApplyTransforms': 'not found'}
  warn_with_log(
2024-05-06 16:42:58,763 - JUNIFER - ERROR - Input does not have the required data.   Input: ['BOLD']     Required (all of): ['T1w', 'Warp', 'BOLD']
Traceback (most recent call last):
  File "/home/fraimondo/miniconda3/envs/junifer/bin/junifer", line 8, in <module>
    sys.exit(cli())
             ^^^^^
  File "/home/fraimondo/miniconda3/envs/junifer/lib/python3.11/site-packages/click/core.py", line 1157, in __call__
    return self.main(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/fraimondo/miniconda3/envs/junifer/lib/python3.11/site-packages/click/core.py", line 1078, in main
    rv = self.invoke(ctx)
         ^^^^^^^^^^^^^^^^
  File "/home/fraimondo/miniconda3/envs/junifer/lib/python3.11/site-packages/click/core.py", line 1688, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/fraimondo/miniconda3/envs/junifer/lib/python3.11/site-packages/click/core.py", line 1434, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/fraimondo/miniconda3/envs/junifer/lib/python3.11/site-packages/click/core.py", line 783, in invoke
    return __callback(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/fraimondo/dev/tbox/junifer/junifer/api/cli.py", line 224, in run
    api_run(
  File "/home/fraimondo/dev/tbox/junifer/junifer/api/functions.py", line 169, in run
    mc.validate(datagrabber_object)
  File "/home/fraimondo/dev/tbox/junifer/junifer/markers/collection.py", line 140, in validate
    t_data = preprocessor.validate(t_data)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/fraimondo/dev/tbox/junifer/junifer/pipeline/pipeline_step_mixin.py", line 204, in validate
    fit_input = self.validate_input(input=input)
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/fraimondo/dev/tbox/junifer/junifer/preprocess/base.py", line 82, in validate_input
    raise_error(
  File "/home/fraimondo/dev/tbox/junifer/junifer/utils/logging.py", line 300, in raise_error
    raise klass(msg)
ValueError: Input does not have the required data.   Input: ['BOLD']     Required (all of): ['T1w', 'Warp', 'BOLD']

Anything else?

No response

fraimondo commented 4 months ago

Issue can be solved either at the marker collection level, by "merging" the new and old t_data in line 140 here:

https://github.com/juaml/junifer/blob/e0cb0f233b071bb9e15818cabef036f7bd6695f3/junifer/markers/collection.py#L133-L142

OR, at the PipelineStepMixin level, which I don't really recomend as it is not something of the step, but the application of subsequent steps.

fraimondo commented 4 months ago

Something like this works:

                # Validate preprocessor
                old_t_data = t_data.copy()
                logger.info(f"Preprocess input type: {t_data}")
                new_t_data = preprocessor.validate(old_t_data)
                t_data = list(set(old_t_data) | set(new_t_data))
                logger.info(f"Preprocess output type: {t_data}")
synchon commented 4 months ago

Something like this works:

                # Validate preprocessor
                old_t_data = t_data.copy()
                logger.info(f"Preprocess input type: {t_data}")
                new_t_data = preprocessor.validate(old_t_data)
                t_data = list(set(old_t_data) | set(new_t_data))
                logger.info(f"Preprocess output type: {t_data}")

Can you check if the validation passes?

codecov[bot] commented 4 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (208cfdc) to head (902bdc1). Report is 1 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/juaml/junifer/pull/339/graphs/tree.svg?width=650&height=150&src=pr&token=5H21JuZXMw&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml)](https://app.codecov.io/gh/juaml/junifer/pull/339?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) ```diff @@ Coverage Diff @@ ## main #339 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 1 1 Lines 1 1 ========================================= Hits 1 1 ``` | [Flag](https://app.codecov.io/gh/juaml/junifer/pull/339/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | Coverage Δ | | |---|---|---| | [docs](https://app.codecov.io/gh/juaml/junifer/pull/339/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) | `100.00% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml#carryforward-flags-in-the-pull-request-comment) to find out more.
github-actions[bot] commented 4 months ago

PR Preview Action v1.4.7 :---: Preview removed because the pull request was closed. 2024-05-14 11:21 UTC