-
Notifications
You must be signed in to change notification settings - Fork 298
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
Remove ICA-AROMA #2936
Comments
I just wanted to put in a plug for keeping ICA-AROMA. I appreciate that it may need to be updated to be compatible with the latest MELODIC, but that seems like a better move than removing it from fmriprep. In addition to folks in my lab, a number of colleagues continue to use ICA-AROMA regularly, and removing it from fmriprep would generate a new problem for us to solve. I tested the base installation of ICA-AROMA on our university cluster with FSL 6.0.6 and did not encounter problems. Do you have a reproducible example of what causes it to crash in fmriprep? I'd be happy to take a look at this if so.
|
@michaelhallquist See #2980 (CI output): 230404-21:40:22,922 nipype.workflow INFO:
***********************************
230404-21:40:22,922 nipype.workflow ERROR:
could not run node: fmriprep_23_1_wf.single_subject_01_wf.func_preproc_task_mixedgamblestask_run_02_wf.ica_aroma_wf.ica_aroma
230404-21:40:22,926 nipype.workflow INFO:
crashfile: /out/fmriprep/sub-01/log/20230404-211412_d4123983-8b9f-43d6-9458-674954fd1c18/crash-20230404-213215-UID1001-ica_aroma-22a68672-708e-4197-8cf8-6ea89ff46711.txt
230404-21:40:22,926 nipype.workflow INFO:
***********************************
230404-21:40:22,970 nipype.workflow CRITICAL:
fMRIPrep failed: Traceback (most recent call last):
File "/opt/conda/lib/python3.9/site-packages/nipype/pipeline/plugins/multiproc.py", line 67, in run_node
result["result"] = node.run(updatehash=updatehash)
File "/opt/conda/lib/python3.9/site-packages/nipype/pipeline/engine/nodes.py", line 527, in run
result = self._run_interface(execute=True)
File "/opt/conda/lib/python3.9/site-packages/nipype/pipeline/engine/nodes.py", line 645, in _run_interface
return self._run_command(execute)
File "/opt/conda/lib/python3.9/site-packages/nipype/pipeline/engine/nodes.py", line 771, in _run_command
raise NodeExecutionError(msg)
nipype.pipeline.engine.nodes.NodeExecutionError: Exception raised while executing Node ica_aroma.
Cmdline:
ICA_AROMA.py -tr 2.000 -np -den nonaggr -i /scratch/fmriprep_23_1_wf/single_subject_01_wf/func_preproc_task_mixedgamblestask_run_02_wf/ica_aroma_wf/smooth/vol0000_xform-00000_clipped_merged_smooth.nii.gz -m /scratch/fmriprep_23_1_wf/single_subject_01_wf/func_preproc_task_mixedgamblestask_run_02_wf/bold_std_trans_wf/_std_target_MNI152NLin6Asym.res2/mask_std_tfm/vol0000_unwarped_merged_valid_average_corrected_brain_mask_maths_trans.nii.gz -meldir /scratch/fmriprep_23_1_wf/single_subject_01_wf/func_preproc_task_mixedgamblestask_run_02_wf/ica_aroma_wf/melodic -mc /scratch/fmriprep_23_1_wf/single_subject_01_wf/func_preproc_task_mixedgamblestask_run_02_wf/bold_hmc_wf/normalize_motion/motion_params.txt -o /scratch/fmriprep_23_1_wf/single_subject_01_wf/func_preproc_task_mixedgamblestask_run_02_wf/ica_aroma_wf/ica_aroma/out
Stdout:
------------------------------- RUNNING ICA-AROMA -------------------------------
--------------- 'ICA-based Automatic Removal Of Motion Artifacts' ---------------
Step 1) MELODIC
- The existing/specified MELODIC directory will be used.
Step 2) Automatic classification of the components
- registering the spatial maps to MNI
Stderr:
Image Exception : #21 :: Invalid ROI dimensions
Invalid ROI dimensions
Image Exception : #21 :: Invalid ROI dimensions
Invalid ROI dimensions
Image Exception : #63 :: No image files match: /scratch/fmriprep_23_1_wf/single_subject_01_wf/func_preproc_task_mixedgamblestask_run_02_wf/ica_aroma_wf/ica_aroma/out/thr_zstat????
terminate called after throwing an instance of 'std::runtime_error'
what(): No image files match: /scratch/fmriprep_23_1_wf/single_subject_01_wf/func_preproc_task_mixedgamblestask_run_02_wf/ica_aroma_wf/ica_aroma/out/thr_zstat????
Aborted (core dumped)
rm: cannot remove '/scratch/fmriprep_23_1_wf/single_subject_01_wf/func_preproc_task_mixedgamblestask_run_02_wf/ica_aroma_wf/ica_aroma/out/thr_zstat????.nii.gz': No such file or directory
Image Exception : #63 :: No image files match: /scratch/fmriprep_23_1_wf/single_subject_01_wf/func_preproc_task_mixedgamblestask_run_02_wf/ica_aroma_wf/ica_aroma/out/melodic_IC_thr
terminate called after throwing an instance of 'std::runtime_error'
what(): No image files match: /scratch/fmriprep_23_1_wf/single_subject_01_wf/func_preproc_task_mixedgamblestask_run_02_wf/ica_aroma_wf/ica_aroma/out/melodic_IC_thr
Aborted (core dumped)
Traceback (most recent call last):
File "/opt/ICA-AROMA/ICA_AROMA.py", line 210, in <module>
aromafunc.register2MNI(fslDir, melIC, melIC_MNI, affmat, warp)
File "/opt/ICA-AROMA/ICA_AROMA_functions.py", line 161, in register2MNI
pixdim1 = float(subprocess.getoutput('%sfslinfo %s | grep pixdim1 | awk \'{print $2}\'' % (fslDir, inFile)))
ValueError: could not convert string to float: 'Image Exception : #63 :: No image files match: /scratch/fmriprep_23_1_wf/single_subject_01_wf/func_preproc_task_mixedgamblestask_run_02_wf/ica_aroma_wf/ica_aroma/out/melodic_IC_thr\nNo image files match: /scratch/fmriprep_23_1_wf/single_subject_01_wf/func_preproc_task_mixedgamblestask_run_02_wf/ica_aroma_wf/ica_aroma/out/melodic_IC_thr'
Traceback:
Traceback (most recent call last):
File "/opt/conda/lib/python3.9/site-packages/nipype/interfaces/base/core.py", line 454, in aggregate_outputs
setattr(outputs, key, val)
File "/opt/conda/lib/python3.9/site-packages/nipype/interfaces/base/traits_extension.py", line 330, in validate
value = super(File, self).validate(objekt, name, value, return_pathlike=True)
File "/opt/conda/lib/python3.9/site-packages/nipype/interfaces/base/traits_extension.py", line 135, in validate
self.error(objekt, name, str(value))
File "/opt/conda/lib/python3.9/site-packages/traits/base_trait_handler.py", line 74, in error
raise TraitError(
traits.trait_errors.TraitError: The 'nonaggr_denoised_file' trait of a _ICA_AROMAOutputSpecRPT instance must be a pathlike object or string representing an existing file, but a value of '/scratch/fmriprep_23_1_wf/single_subject_01_wf/func_preproc_task_mixedgamblestask_run_02_wf/ica_aroma_wf/ica_aroma/out/denoised_func_data_nonaggr.nii.gz' <class 'str'> was specified.
During handling of the above exception, another exception occurred:
Traceback (most recent call last):
File "/opt/conda/lib/python3.9/site-packages/nipype/interfaces/base/core.py", line 399, in run
runtime = self._post_run_hook(runtime)
File "/opt/conda/lib/python3.9/site-packages/niworkflows/interfaces/reportlets/segmentation.py", line 231, in _post_run_hook
outputs = self.aggregate_outputs(runtime=runtime)
File "/opt/conda/lib/python3.9/site-packages/nipype/interfaces/base/core.py", line 461, in aggregate_outputs
raise FileNotFoundError(msg)
FileNotFoundError: No such file or directory '/scratch/fmriprep_23_1_wf/single_subject_01_wf/func_preproc_task_mixedgamblestask_run_02_wf/ica_aroma_wf/ica_aroma/out/denoised_func_data_nonaggr.nii.gz' for output 'nonaggr_denoised_file' of a ICA_AROMARPT interface |
## Changes proposed in this pull request Remove ICA-AROMA. Closes #2936.
Hi Chris @effigies, I just stumbled onto this. I wouldn't go as far as saying that AROMA is not maintained. I agree it isn't the nicest codebase, but thus far it has required limited changes to fit into existing pipelines and thus appear unmaintained. Issues are monitored though... Without knowing the details, it looks like the error reported above is related to using a different MNI template than the original MNI152 one that is shipped with FSL. The incompatibility between templates is a very annoying feature of MRI research and one that I would indeed like AROMA to not rely on... Then again I'm unfortunately not following fMRIprep close enough to know whether you in the mean time have made your own implementation of AROMA available. If not, I would be hesitant to simply remove AROMA if the only alternative is scrubbing... |
Hi @maartenmennes. The FSL compatibility issue caused us to rethink the inclusion of AROMA, but once I realized that AROMA could be performed post-fMRIPrep without losing functionality, it became very compelling. fMRIPrep is a large and intricate enough workflow without it. I would like to rebundle the workflow here so that users can easily get the derivatives and reports they were getting before: https://github.com/nipreps/fmripost-aroma. AROMA being a slow-moving target compared to the rest of fMRIPrep, this seems like a better fit. I think it's about 1-2 days of work to build and test, and would make a good OHBM hackathon project. |
Although now that you mention the atlas issue, it seems that the failure was probably that we didn't include the atlas package from conda. So we may still be able to take advantage of the FSL conda distribution in the new workflow. We probably need |
re the atlas, yes as long as the origin and dimensions are the same it should work out. Having aroma as a post package sounds like a reasonable alternative, but in my mind defeats the purpose of fMRIprep. Unless you are moving towards a 'building block' model where users can attach different workflows. But like I said I haven't followed fMRIprep close enough to make educated suggestions, but if we can avoid having aroma excluded due to some silly technical issue I'm happy to help resolve this! |
I wanted to echo Maarten's comment that having AROMA as an option in fmriprep is a great idea from my perspective. We've already built software (https://clpipe.readthedocs.io/en/latest/) around this that puts fmriprep into a larger workflow that includes later steps of preprocessing (e.g., confound regression). Our software is built assuming that AROMA can be enabled in fmriprep. So, removing it from fmriprep will generate the need for us to rework our confound step substantially. Likewise, I know other widely-used software such as halfpipe (https://onlinelibrary.wiley.com/doi/full/10.1002/hbm.25829) depends on the AROMA workflow within fmriprep. Finally, more substantively, I think that AROMA is a good data-driven alternative to scrubbing that has performed well in several confound pipeline comparisons. Taking it out of fmriprep will generate friction for the typical user and may tilt their choices regarding confound regression toward FD-based scrubbing simply because it's already in the confounds tsv file. I would hope that users make critical choices about confound regression not based on convenience, but substance, and I fear that removing AROMA from fmriprep may short-circuit this consideration. |
I suppose we should reopen the discussion. Philosophically, I do not think ICA-AROMA belongs in fMRIPrep because it is both outside the consensus necessary preprocessing steps and does not require access to intermediate results that do not appear in fMRIPrep derivatives. We added it about 5 years ago with the understanding that it would be very difficult to use it post-fMRIPrep. This was at least mostly true at the time, as we did not have TemplateFlow to make selecting a normalization target trivial. Under the current architecture, it is basically an isolated component and one more thing to keep from breaking. It may also help to understand where I would like to take fMRIPrep. The traditional (and current) mode of fMRIPrep has been to generate as many derivatives as possible, on the rough grounds that space is cheap, computation expensive, and recomputation is rarely deterministic so might as well do it once and have a common ground to start all further analysis from. This works well for small (N<50) datasets, but when we start talking thousands of subjects and tens of thousands of hours of BOLD, every resampling target has a significant cost. When running fMRIPrep on ABCD or OpenNeuro, we need to consider both computation time and space, so resampling to two MNI templates, a FreeSurfer surface and fsLR is untenable. It is wiser to drop the resampled BOLD series and store instead the transformations that can be used to reconstruct it deterministically. My ideal workflow for fMRIPrep + ICA-AROMA would thus be:
If you've written pipelines to handle fMRIPrep, I suspect you're already familiar with the storage/reusability tradeoff that the current model demands. Possibly you only take the AROMA results and either don't calculate or throw away the other resampling targets. For data providers that want to leave the options wide open to the users, the tradeoffs of the current approach are stark. Now we aren't there yet. As you'll have seen, fmripost-aroma doesn't exist yet; the minimal derivatives mode doesn't exist either (I'm working on that). If we can get AROMA back without being forced to revert to old FSL, I would be willing to extend the deprecation timeline, but I think it would be a good idea to have a roadmap for removing it and updating downstream pipelines to work with the minimal derivatives. I'll make a PR and see if it's manageable. And that's a good point that there are tools like HalfPipe and XCP-D (cc @HippocampusGirl @tsalo) that depend on fMRIPrep derivatives. We should broaden this discussion to make sure that there's a path forward for those tools too. I'm starting to think that what is currently fmriprep can become This would be good to discuss at OHBM, if people are going to be around. |
@maartenmennes @michaelhallquist If you can figure out what's needed in #2995, please feel free. I've spent a couple hours on this and need to move on. If it's useful, here is the full 6.0.6.2 dependency specification: name: FSL
channels:
- https://fsl.fmrib.ox.ac.uk/fsldownloads/fslconda/public/
- conda-forge
dependencies:
- boost-cpp 1.74.*
- ciftilib 1.5.3
- c-compiler
- cxx-compiler
- fmrib-unpack 3.5.2
- fsleyes 1.5.0
- fslpy 3.10.0
- fsl-add_module 0.3.2
- fsl-armawrap 0.6.0
- fsl-asl_mfree v1.0.2
- fsl-avwutils 2209.0
- fsl-base 2212.0
- fsl-basil v4.0.4
- fsl-basisfield 2203.1
- fsl-baycest 2111.0
- fsl-bet2 2111.0
- fsl-bianca 2209.2
- fsl-biancadata 2006.0
- fsl-bint 2111.0
- fsl-cluster 2201.0
- fsl-cprob 2111.0
- fsl-cudimot 2006.0
- fsl-data_atlases 2103.0
- fsl-data_atlases_xtract 1.1.0
- fsl-data_first_models_317_bin 2006.0
- fsl-data_first_models_336_bin 2006.0
- fsl-data_linearmni 1.0.0
- fsl-data_possum 2106.0
- fsl-data_standard 2208.0
- fsl-deface 1.3.0
- fsl-discreteopt 2008.0
- fsl-eddy 2111.0
- fsl-eddy_qc v1.1.1
- fsl-fabber_core v4.0.10
- fsl-fabber_models_asl v2.0.7
- fsl-fabber_models_cest v2.1.6
- fsl-fabber_models_dce v2.1.5
- fsl-fabber_models_dsc v2.0.5
- fsl-fabber_models_dualecho v2.0.4
- fsl-fabber_models_dwi v2.0.3
- fsl-fabber_models_pet v1.0.1
- fsl-fabber_models_qbold v1.0.2
- fsl-fabber_models_t1 v2.0.2
- fsl-fast4 2111.0
- fsl-fastpdlib 2111.1
- fsl-fdt 2202.5
- fsl-feat5 2201.2
- fsl-film 2111.0
- fsl-filmbabe 2111.0
- fsl-first 2203.0
- fsl-first_lib 2111.0
- fsl-flameo 2111.0
- fsl-flirt 2111.0
- fsl-fnirt 2203.0
- fsl-fugue 2201.2
- fsl-get_standard 0.1.0
- fsl-giftiio 2111.0
- fsl-gps 2211.0
- fsl-installer 3.0.0
- fsl-lesions 2111.0
- fsl-libgdc 2111.0
- fsl-libmeshutils 2111.0
- fsl-libvis 2111.0
- fsl-mcflirt 2111.0
- fsl-melodic 2111.1
- fsl-meshclass 2111.0
- fsl-misc_c 2111.0
- fsl-misc_scripts 2111.0
- fsl-misc_tcl 2206.0
- fsl-miscmaths 2203.2
- fsl-miscvis 2201.0
- fsl-mist 2111.3
- fsl-mm 2111.0
- fsl_mrs 2.0.9
- fsl-msm 2008.0
- fsl-msmreglib 2008.0
- fsl-mvdisc 2111.0
- fsl-newimage 2203.6
- fsl-newmesh 2111.0
- fsl-newnifti 4.1.0
- fsl-newran 2111.0
- fsl-oxford_asl v4.0.28
- fsl-possum 2111.2
- fsl-ptx2 2111.4
- fsl-pyfeeds 0.10.1
- fsl-pyfeeds-tests 2210.0
- fsl-qboot 2111.0
- fsl-randomise 2203.2
- fsl-shapemodel 2111.0
- fsl-siena 2111.0
- fsl-slicetimer 2111.0
- fsl_sub 2.7.5
- fsl_sub_plugin_sge 1.5.2
- fsl_sub_plugin_slurm 1.4.3
- fsl-surface 2111.0
- fsl-susan 2111.0
- fsl-swe v1.1.0
- fsl-tbss 2111.0
- fsl-tirl v2.2.1
- fsl-topup 2203.1
- fsl-utils 2203.2
- fsl-vbm 2006.0
- fsl-verbena v4.0.3
- fsl-vtkio 2111.0
- fsl-warpfns 2203.0
- fsl-wire 1.0.0
- fsl-xtract 1.5.1
- fsl-xtract_data 1.3.4
- fsl-znzlib 2111.0
- ghalton 0.6.1
- hlsvdpropy 2.0.1
- libopenblas 0.3.18 *openmp*
- make
- nidmresults 2.1.0
- nidmresults-fsl 2.2.0
- numpy 1.21.*
- openslide-python v1.1.2
- pkg-config
- python 3.10.*
- tk 8.6.*
- vtk 9.1
- wxpython 4.1.*
- zlib 1.2.*
- fsl-cudabasisfield-cuda-10.2 1.1.1
- fsl-eddy-cuda-10.2 2111.0
- fsl-fdt-cuda-10.2 2202.5
- fsl-ptx2-cuda-10.2 2111.4 We currently have it reduced to the following (and their dependencies): - fsl-bet2=2111.0
- fsl-flirt=2111.0
- fsl-fast4=2111.0
- fsl-fugue=2201.2
- fsl-mcflirt=2111.0
- fsl-miscmaths=2203.2
- fsl-susan=2111.0
- fsl-data_atlases=2103.0
- fsl-melodic=2111.1
- fsl-topup=2203.1 |
@maartenmennes @michaelhallquist Reminder that if you would like ICA-AROMA restored to fMRIPrep 23.1.0, I will need someone to spend the time on figuring out what exactly it needs to run under FSL 6. |
Apologies for the delay, @effigies. I gave this an initial look (I don't have a testing suite for fmriprep setup, so wasn't able to confirm whether it would fix things). ICA-AROMA depends on the MNI152_T1_2mm_brain.nii.gz file, which is provided by the fsl-data_standard package/repo (https://git.fmrib.ox.ac.uk/fsl/data_standard). This is also in $FSLDIR/data/standard if a full FSL installation is present. Could it be that the failure in aromafunc.register2MNI reflects the need to add fsl-data_standard=2208.0 to the fsl-conda dependencies? The fsl-data_atlases package does not contain the critical files for AROMA. https://github.com/maartenmennes/ICA-AROMA/blob/f49dbc160653c4027809a25a557b33e07e042da5/ICA_AROMA_functions.py#L156 Thanks, |
Unfortunately, that didn't do it. Here's the failed run: https://app.circleci.com/pipelines/github/nipreps/fmriprep/1408/workflows/eb2e712e-70e4-4ae1-8f6a-ebe68cf3f64e/jobs/7739
|
23.1.0 (June 12, 2023) New feature release in the 23.1.x series. This release substantially reworks the resampling to fsLR grayordinate space, better accounting for partial volumes and high variance voxels. If you are resampling using ``--project-goodvoxels``, we strongly recommend upgrading. Fieldmap handling is improved, with better preference given to single-band references in both PEPolar and SyN-SDC schemes. Additionally, fMRIPrep will no longer estimate fieldmaps that are not intended to be used to correct BOLD series, reducing unneeded processing. This release removes ICA-AROMA from the fMRIPrep workflow. To use ICA-AROMA, set ``MNI152NLin6Asym:res-2`` as a target output space. MELODIC and ICA-AROMA can be run on the resulting images in a separate pipeline. For further information on the reasoning behind this change, see `GitHub issue #2936 <https://github.com/nipreps/fmriprep/issues/2936>`__. This release increments the versions of ANTs and FSL bundled in the Docker image. With thanks to Eilidh MacNicol, Basille Pinsard and Taylor Salo for contributions in fMRIPrep and SDCflows. * FIX: Raise RuntimeError at build if echos have mismatched shapes (#3028) * FIX: Inconsistent fmapless estimation when ignoring fieldmaps (#2994) * FIX: Dilate BOLD mask by 2 voxels to prevent over-aggressive masking degrading T2* map estimation (#2986) * FIX: Estimate free memory with "available", not "free" (#2985) * ENH: Add ``--me-t2s-fit-method`` parameter (#3030) * ENH: Resample BOLD to fsLR directly, dropping fsaverage intermediate (#3011) * ENH: Allow SBref+EPI PEPolar fieldmaps to correct BOLD series (#3008) * ENH: Remove ICA-AROMA from workflow and docs (#2966) * RF: Filter fieldmaps based on whether they will be used to correct a BOLD series (#3025) * MNT: Update ANTs pin in Docker image (#3016) * MNT: Update governance docs (#2992) * MNT: Refactor Docker build process (#2982) * MNT: Pin conda environment more strictly (#2853) * MNT: Require niworkflows ~1.3.6 (#2740) * CI: Use registry for layer caching (#3012) * CI: Upgrade docker orb (#2865)
What would you like to see added in fMRIPrep?
ICA-AROMA is unmaintained, and appears to be incompatible with the version of MELODIC released in FSL 6.0.6.2.
Given that people can request outputs in MNI152NLin6Asym space, it should be possible to run the necessary steps post-preprocessing, and then we will not have our hands tied by unmaintained dependencies.
cc @jdkent
Do you have any interest in helping implement the feature?
Yes
Additional information / screenshots
No response
The text was updated successfully, but these errors were encountered: