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

ENH: Resample BOLD to fsLR directly, dropping fsaverage intermediate #3011

Merged
merged 21 commits into from
Jun 6, 2023

Conversation

effigies
Copy link
Member

@effigies effigies commented May 18, 2023

Changes proposed in this pull request

  1. Remove goodvoxels and CIFTI-specific code from mri_vol2surf resampling workflow. fsnative and fsaverage will remain the same for now.
  2. Removes fsaverage as an implicit resampling target for --cifti-output. fsaverage and fsLR are now entirely independent.
  3. Create a subject-specific cortical ROI mask in GIFTI space. Not currently output, as it's only needed internally; it's not a great mask but is intended to be more liberal than the fsLR mask applied later.
  4. Adds a number of wb_command commands, including an OpenMP mixin that allows OMP_NUM_THREADS to be set in the environment. I think most commands can take advantage, but I only tagged slow ones (take >10s) that I could verify actually used them.
  5. Depends on ENH: Project fsLR mesh onto native sphere to enable single-shot resampling into fsLR smriprep#339 and FIX: Move from smoothwm to white niworkflows#806.

TODO:

  • Restore literate workflow code.
  • Restore saving goodvoxels mask, if desired.

Documentation that should be reviewed

TODO


Closes #2990.

@effigies effigies requested a review from mgxd June 5, 2023 17:34
@effigies
Copy link
Member Author

effigies commented Jun 5, 2023

@mgxd I just realized this hasn't been reviewed. Any chance you could have a look? I'll work on knocking off those two checklist points above.

Copy link
Collaborator

@mgxd mgxd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, skimmed through and nothing jumps out.

One note - the more that this can be upstreamed, the better - since we'll be using them in nibabies as well.

@@ -437,7 +433,7 @@ def init_func_preproc_wf(bold_file, has_fieldmap=False):
bids_root=layout.root,
cifti_output=config.workflow.cifti_output,
freesurfer=freesurfer,
project_goodvoxels=project_goodvoxels,
project_goodvoxels=False,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noting we should still output the ribbon mask

fmriprep/workflows/bold/resampling.py Outdated Show resolved Hide resolved
(goodvoxels_ribbon_mask, outputnode, [("out_file", "goodvoxels_ribbon")]),
]
)

return workflow


def init_bold_fsLR_resampling_wf(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is a really nice digestion of the wb commands used

@effigies
Copy link
Member Author

effigies commented Jun 6, 2023

One note - the more that this can be upstreamed, the better - since we'll be using them in nibabies as well.

@mgxd Do you mean push interfaces into niworkflows?

@mgxd
Copy link
Collaborator

mgxd commented Jun 6, 2023

Yes, but it was not to say that should hold this up - we can port it over for 1.8.1, cut a nwf release, and then remove it from here.

@effigies effigies merged commit c56bec6 into nipreps:master Jun 6, 2023
effigies added a commit that referenced this pull request Jun 12, 2023
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)
from fmriprep.workflows.bold.resampling import init_bold_fsLR_resampling_wf
wf = init_bold_fsLR_resampling_wf(
estimate_goodvoxels=True,
grayord_density='92k',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps grayord_density='91k' instead of 92k based on the Parameters below?

@effigies effigies deleted the fix/goodvoxels branch August 24, 2023 03:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

good voxels masking error
3 participants