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

Merged
synchon merged 4 commits from fix/multi-preprocessor-validation into main 2024-05-14 11:04:51 +00:00
synchon commented 2024-05-06 15:47:52 +00:00 (Migrated from github.com)

Is there an existing issue for this?

  • I have searched the existing issues

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

### Is there an existing issue for this? - [X] I have searched the existing issues ### 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` ```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 ```markdown 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 ```shell (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 2024-05-06 15:29:44 +00:00 (Migrated from github.com)

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

github.com/juaml/junifer@e0cb0f233b/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.

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 2024-05-06 15:38:25 +00:00 (Migrated from github.com)

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}")
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 2024-05-06 15:50:11 +00:00 (Migrated from github.com)

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?

> 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 2024-05-06 16:08:43 +00:00 (Migrated from github.com)

Codecov Report

All modified and coverable lines are covered by tests ✅

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

@@            Coverage Diff            @@
##              main      #339   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines            1         1           
=========================================
  Hits             1         1           
Flag Coverage Δ
docs 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

## [Codecov](https://app.codecov.io/gh/juaml/junifer/pull/339?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) Report All modified and coverable lines are covered by tests :white_check_mark: > Project coverage is 100.00%. Comparing base [(`208cfdc`)](https://app.codecov.io/gh/juaml/junifer/commit/208cfdc0afff288965e8c93deb6fd8ec0cbf4020?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml) to head [(`902bdc1`)](https://app.codecov.io/gh/juaml/junifer/pull/339?dropdown=coverage&src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=juaml). > Report is 1 commits behind head on main. <details><summary>Additional details and impacted files</summary> [![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. </details>
github-actions[bot] commented 2024-05-06 16:12:16 +00:00 (Migrated from github.com)
PR Preview Action v1.4.7
Preview removed because the pull request was closed.
2024-05-14 11:21 UTC
[PR Preview Action](https://github.com/rossjrw/pr-preview-action) v1.4.7 :---: Preview removed because the pull request was closed. 2024-05-14 11:21 UTC <!-- Sticky Pull Request Commentpr-preview -->
fraimondo (Migrated from github.com) requested changes 2024-05-06 19:44:47 +00:00
fraimondo (Migrated from github.com) commented 2024-05-06 19:44:33 +00:00

This will not work in case you have a preprocessing step that only modifies one type and passes the others through.

Basically, after pre-processing you should have AT LEAST the same elements as before pre-processing:

t_data = list(set(validated_input_data_types) | set(t_data))

This will not work in case you have a preprocessing step that only modifies one type and passes the others through. Basically, after pre-processing you should have AT LEAST the same elements as before pre-processing: `t_data = list(set(validated_input_data_types) | set(t_data))`
synchon (Migrated from github.com) reviewed 2024-05-07 08:58:25 +00:00
synchon (Migrated from github.com) commented 2024-05-07 08:58:25 +00:00

Yeah agree, good point, made the change.

Yeah agree, good point, made the change.
fraimondo (Migrated from github.com) requested changes 2024-05-13 08:12:01 +00:00
fraimondo (Migrated from github.com) commented 2024-05-13 08:11:52 +00:00

I still think this needs to be done inside the for. What if a preprocessor adds a new type than then is used by another preprocessor?

I still think this needs to be done inside the for. What if a preprocessor adds a new type than then is used by another preprocessor?
synchon (Migrated from github.com) reviewed 2024-05-13 10:00:24 +00:00
synchon (Migrated from github.com) commented 2024-05-13 10:00:24 +00:00

Fair argument, have updated with the code you posted.

Fair argument, have updated with the code you posted.
fraimondo (Migrated from github.com) approved these changes 2024-05-14 08:19:15 +00:00
Sign in to join this conversation.
No reviewers
No milestone
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
juaml/junifer!339
No description provided.