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: Add option to exclude projecting high variance voxels to surface (update of #2855) #2956

Merged
merged 17 commits into from
Mar 6, 2023

Conversation

madisoth
Copy link
Collaborator

Changes proposed in this pull request

Adds option --project-goodvoxels to address #2822.

If enabled, voxels whose timeseries have locally high coefficient of variation, or which lie outside the cortex (based on signed distance volumes generated from the WM and pial surfaces), are excluded from volume-to-surface sampling of BOLD timeseries. Not enabled by default.

  • adds MetricDilate interface for Workbench's wb_command -metric-dilate, which, when using the --project-goodvoxels option, fills zero-value vertices in the BOLD surface timeseries by dilating in data from the nearest "good" (nonzero, nonempty) vertex.

Documentation that should be reviewed

@codecov-commenter
Copy link

codecov-commenter commented Feb 17, 2023

Codecov Report

Base: 46.45% // Head: 46.42% // Decreases project coverage by -0.04% ⚠️

Coverage data is based on head (714e361) compared to base (f831961).
Patch coverage: 43.07% of modified lines in pull request are covered.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2956      +/-   ##
==========================================
- Coverage   46.45%   46.42%   -0.04%     
==========================================
  Files          45       46       +1     
  Lines        3388     3511     +123     
==========================================
+ Hits         1574     1630      +56     
- Misses       1814     1881      +67     
Impacted Files Coverage Δ
fmriprep/workflows/base.py 9.39% <ø> (ø)
fmriprep/workflows/bold/base.py 16.26% <ø> (ø)
fmriprep/workflows/bold/outputs.py 23.44% <0.00%> (-0.50%) ⬇️
fmriprep/workflows/bold/resampling.py 11.73% <9.83%> (-0.22%) ⬇️
fmriprep/interfaces/metric.py 74.60% <74.60%> (ø)
fmriprep/cli/parser.py 82.58% <100.00%> (+0.07%) ⬆️
fmriprep/config.py 81.99% <100.00%> (+0.08%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@effigies effigies force-pushed the enh/project-goodvoxels-020623 branch from 4cf0427 to 7a8d6e7 Compare February 21, 2023 16:16
@effigies
Copy link
Member

@madisoth I think I got the CI all set.

@effigies
Copy link
Member

@madisoth It seems using --project-goodvoxels and --output-spaces fsaverage5 fails with

/opt/workbench/bin_linux64/../exe_linux64/wb_command -metric-dilate /scratch/fmriprep_23_0_wf/single_subject_01_wf/func_preproc_task_mixedgamblestask_run_02_wf/bold_surf_wf/_target_fsaverage5/sampler/mapflow/_sampler0/lh.fsaverage5.gii /home/fmriprep/.cache/templateflow/tpl-fsaverage/tpl-fsaverage_hemi-L_den-164k_midthickness.surf.gii 10.000000 lh.fsaverage5.func.gii -nearest

	ERROR: surface and metric number of vertices do not match

I guess this requires a midthickness surface for every possible output space?

@effigies
Copy link
Member

Okay, running mris_expand on other fsaverages doesn't work (segfaults). I think MNE-Python has a method to decimate surfaces, but I really don't want to do it just for this. We could try just throwing an error if people want to use this with a space besides fsaverage.

@madisoth
Copy link
Collaborator Author

I'm fine with requiring fsaverage as the target when using --project-goodvoxels for the time being, with a plan to support fsaverage3-6 in the future

@effigies
Copy link
Member

The problem is that our use of iterables means that we're using a single workflow for all surface spaces. It's going to be a pain to refactor the workflow, so for the moment I'm just going to emit a loud warning and disable --project-goodvoxels if other surface targets besides --output-space fsaverage or --cifti-output are used. Not ideal, but I'm worried about introducing last minute bugs.

@effigies effigies force-pushed the enh/project-goodvoxels-020623 branch 2 times, most recently from d23b4a9 to 2ecbc68 Compare February 27, 2023 14:51
@effigies effigies added this to the 23.0.0 milestone Feb 27, 2023
@effigies effigies force-pushed the enh/project-goodvoxels-020623 branch 4 times, most recently from c8d3847 to 4b609af Compare February 28, 2023 13:17
@effigies effigies marked this pull request as ready for review February 28, 2023 15:05
@effigies effigies force-pushed the enh/project-goodvoxels-020623 branch from 4b609af to 2d802cb Compare March 6, 2023 14:40
@effigies effigies merged commit 11be54e into nipreps:master Mar 6, 2023
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.

3 participants