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

✨ Seamless ingress of fmriprep outputs #1902

Closed
3 tasks
sgiavasis opened this issue Mar 8, 2023 · 4 comments
Closed
3 tasks

✨ Seamless ingress of fmriprep outputs #1902

sgiavasis opened this issue Mar 8, 2023 · 4 comments
Assignees
Labels
BIDS https://bids-specification.readthedocs.io/ engine Internal implementation changes that may not be visible to users. feature
Milestone

Comments

@sgiavasis
Copy link
Collaborator

sgiavasis commented Mar 8, 2023

Related problem

It's not easy (or possible) to directly ingress already-computed outputs into C-PAC from other pipelines (or specifically fmriprep) in order to perform further processing.

Proposed feature

The ability to ingress fmriprep output data directly into the C-PAC resource pool, so that users can then proceed to perform further processing. Example: pulling in the confounds.tsv and also the BOLD time series that has already been preprocessed and warped to template; then, C-PAC can proceed with denoising and anything else, including calculating connectomes and other derivatives.

Note: C-PAC v1.8+ has code/infrastructure to allow ingress of already-computed data into the resource pool, but needs to be expanded to make the process seamless.

Acceptance criteria

  • New field in the data configuration YAML which holds the path to an fmriprep output directory
  • Each possible layout of fmriprep outputs able to be pulled into C-PAC flexibly without issues (i.e. surface on or off)
  • Clearly-defined user flow: if the user has preprocessing selected in the pipeline config, and already-preprocessed data is pulled in, C-PAC needs to handle this safely

Additional context

There is also the question and possibility of forking. It's possible we can have the user provide an fmriprep output directory and also select a modified version of the fmriprep-options preconfig which includes denoising and derivatives, and have C-PAC write out the end result of both for comparison easily.

Reference

Branch: https://github.com/FCP-INDI/C-PAC/tree/feature/fmriprep_ingress

@sgiavasis sgiavasis added feature 2 - High Priority engine Internal implementation changes that may not be visible to users. BIDS https://bids-specification.readthedocs.io/ labels Mar 8, 2023
@sgiavasis sgiavasis added this to the 1.8.6 release milestone Mar 8, 2023
@sgiavasis sgiavasis moved this to 🏗 In progress in C-PAC Development Mar 8, 2023
@sgiavasis
Copy link
Collaborator Author

sgiavasis commented Mar 9, 2023

Unit/smaller testing targets:

  • 1. Passthrough test where we see that the fmriprep output files are ingressed into CPAC and written directly back out.
  • 2. Build tests which ensure multiple BOLD scan files go in and come back out with the right labels.
  • 3. For those not listed in the outputs.tsv, make decisions regarding re-naming or inclusion/exclusion.
    • Regressors/Confounds TSV?
    • Other naming differences?
  • 4. Build tests which ensure space-* and res-* BIDS tags are converted from specific --> generic --> specific.

Integration:

  • 1. Taking the whole output dataset in, will C-PAC smoothly proceed with denoising/nuisance regression and derivative calculation?
  • 2. Are there guardrails for the intersection between fmriprep-preprocessed native-space data and a CPAC pipeline config requesting native-space processing? (Until point no.3 below is complete- post v1.8.6).
  • 3. Taking only the T1w-related outputs in, and anatomical preprocessing and registration turned Off in C-PAC, will C-PAC seamlessly proceed with BOLD preprocessing, coregistration, and final BOLD time-series warp to template?

@sgiavasis
Copy link
Collaborator Author

sgiavasis commented May 1, 2023

@e-kenneally resolves issue with the func datasource connection.

Next:

  • Update the internal guardrails to accommodate a data dictionary that works with fmriprep output directory files.
  • Try using fmriprep's confounds.tsv directly with AFNI 3dTproject to see if it works out of the box.
  • Ascertain BIDS standard for confounds/regressors and change CPAC's naming of regressors to confounds if necessary.

@e-kenneally
Copy link
Collaborator

Current issue: quadruple-nested workdir folders - there is a _scan-{scan_name} folder for EVERY scan inside every _scan-{scan-name} folder in the working directory.

This is what I know about the problem so far:

  • I see the same forking issue (super nested workdir folders) if I create a datasource node for every func file, or just for each key
  • Forking doesn’t happen if I only pass one scan name for every resource in the inputnode.iterables field , but then only one version / scan of each file ends up in the func output
  • Everything runs smoothly and the scan iterables are handled when scan iterables handled outside of loop, but then not tied to the correct ingress node, just the last one to be created in the loop.
    - Also, this is the case where they aren’t correctly matched up with the files.

@shnizzedy shnizzedy mentioned this issue Jun 16, 2023
8 tasks
@sgiavasis sgiavasis self-assigned this Jul 17, 2023
@sgiavasis sgiavasis moved this from 🏗 In progress to 👀 In review in C-PAC Development Jan 8, 2024
@sgiavasis sgiavasis moved this from 👀 In review to Close on release in C-PAC Development Jan 22, 2024
@sgiavasis
Copy link
Collaborator Author

This was resolved in v1.8.6.

@github-project-automation github-project-automation bot moved this from Close on release to ✅ Done in C-PAC Development Jan 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BIDS https://bids-specification.readthedocs.io/ engine Internal implementation changes that may not be visible to users. feature
Projects
Status: Done
Development

No branches or pull requests

2 participants