Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
May 14, 2024

Conversation

synchon
Copy link
Member

@synchon synchon commented May 6, 2024

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

@fraimondo fraimondo added bug Something isn't working triage New issues waiting to be reviewed labels May 6, 2024
@fraimondo
Copy link
Contributor Author

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

if self._preprocessors is not None:
for preprocessor in self._preprocessors:
logger.info(
"Validating Preprocessor: "
f"{preprocessor.__class__.__name__}"
)
# Validate preprocessor
t_data = preprocessor.validate(t_data)
logger.info(f"Preprocess output type: {t_data}")

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
Copy link
Contributor Author

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
Copy link
Member

synchon commented May 6, 2024

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?

@synchon synchon added preprocess Issues or pull requests related to preprocessors and removed triage New issues waiting to be reviewed labels May 6, 2024
@synchon synchon requested a review from fraimondo May 6, 2024 16:04
@synchon synchon self-assigned this May 6, 2024
@synchon synchon added this to the 0.0.5 (alpha 4) milestone May 6, 2024
@synchon synchon changed the title [BUG]: Multiple pre-processing steps will fail due to missing dependencies [BUG]: Validation failure of multiple pre-processing steps with different input data types requirement May 6, 2024
Copy link

codecov bot commented May 6, 2024

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.

Copy link

github-actions bot commented May 6, 2024

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

junifer/markers/collection.py Outdated Show resolved Hide resolved
@synchon synchon force-pushed the fix/multi-preprocessor-validation branch from 48592bf to eb68ce9 Compare May 7, 2024 08:57
@synchon synchon requested a review from fraimondo May 7, 2024 08:58
junifer/markers/collection.py Outdated Show resolved Hide resolved
@synchon synchon requested a review from fraimondo May 13, 2024 10:00
@synchon synchon force-pushed the fix/multi-preprocessor-validation branch from 4d92cf0 to 902bdc1 Compare May 14, 2024 08:20
@synchon synchon merged commit 783bc1f into main May 14, 2024
24 of 28 checks passed
@synchon synchon deleted the fix/multi-preprocessor-validation branch May 14, 2024 11:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working preprocess Issues or pull requests related to preprocessors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants