ENH: Add option to exclude projecting high variance voxels to surface #235

madisoth · 2022-09-15T17:09:13Z

Adds option --project-goodvoxels.

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.

Note that unlike HCP / ciftify, no dilation or spatial smoothing is applied to the surface-resampled BOLD timeseries

…ing Workbench interfaces

…pler option (to be continued in a separate branch)

…urfaces for test sub)

codecov-commenter · 2022-09-15T22:15:08Z

Codecov Report

Base: 30.70% // Head: 31.52% // Increases project coverage by +0.81% 🎉

Coverage data is based on head (ba5bac8) compared to base (177de34).
Patch coverage: 29.73% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #235      +/-   ##
==========================================
+ Coverage   30.70%   31.52%   +0.81%     
==========================================
  Files          43       49       +6     
  Lines        3696     4634     +938     
==========================================
+ Hits         1135     1461     +326     
- Misses       2561     3173     +612

Impacted Files	Coverage Δ
nibabies/cli/parser.py	`2.20% <0.00%> (-0.10%)`	⬇️
nibabies/utils/confounds.py	`3.92% <0.00%> (ø)`
nibabies/utils/filtering.py	`4.87% <0.00%> (ø)`
nibabies/utils/validation.py	`21.05% <ø> (ø)`
nibabies/cli/run.py	`7.14% <2.63%> (+0.89%)`	⬆️
nibabies/utils/viz.py	`4.87% <4.87%> (ø)`
nibabies/reports/core.py	`17.30% <11.11%> (-5.77%)`	⬇️
nibabies/workflows/anatomical/base.py	`6.25% <11.11%> (-0.21%)`	⬇️
nibabies/workflows/anatomical/surfaces.py	`11.42% <11.11%> (-11.98%)`	⬇️
nibabies/cli/workflow.py	`2.66% <11.76%> (+0.06%)`	⬆️
... and 37 more

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.

mgxd

Hey Thomas, thank you for the PR! I've gone through a first pass and left some comments.

Regarding some of the style changes - nibabies leverages pre-commit to run styling hooks automatically when committing. To get this, first install it (pip install pre-commit) and then run pre-commit install in the root. This will trigger some automatic checks when attempting to git commit. You may have to run black and isort manually to fix the previous commits. I'm just realizing we don't really have a nibabies-specific contributor's guideline, so will work on adding one.

mgxd · 2022-09-19T14:35:50Z

nibabies/interfaces/volume.py

+    def _list_outputs(self):
+        outputs = super(CreateSignedDistanceVolume, self)._list_outputs()
+        return outputs


No need for this since it's just calling the inherited method

Suggested change

def _list_outputs(self):

outputs = super(CreateSignedDistanceVolume, self)._list_outputs()

return outputs

mgxd · 2022-09-20T14:47:30Z

nibabies/workflows/bold/resampling.py

@@ -135,48 +199,422 @@ def select_target(subject_id, space):
        name="outputnode",
    )

-    # fmt: off
+    # 0, 1 = wm; 2, 3 = pial; 6, 7 = mid


Since this is such a large addition, I think it would make more sense to make this a subworkflow, and call it within init_bold_surf_wf so we don't clutter the connections.

mgxd · 2022-09-20T14:49:40Z

nibabies/workflows/bold/resampling.py

+        mem_gb=DEFAULT_MEMORY_MIN_GB,
+    )


Generally, we can run most nipype utility interfaces on the master process since they are not intense operations - no need to potentially submit to a job queue.

Suggested change

mem_gb=DEFAULT_MEMORY_MIN_GB,

)

mem_gb=DEFAULT_MEMORY_MIN_GB,

run_without_submitting=True,

)

mgxd · 2022-09-20T14:50:20Z

nibabies/workflows/bold/resampling.py

+        mem_gb=DEFAULT_MEMORY_MIN_GB,
+    )


Suggested change

mem_gb=DEFAULT_MEMORY_MIN_GB,

)

mem_gb=DEFAULT_MEMORY_MIN_GB,

run_without_submitting=True,

)

mgxd · 2022-09-20T14:50:53Z

nibabies/workflows/bold/resampling.py

+    select_midthick = pe.Node(
+        niu.Select(index=[6, 7]),
+        name="select_midthick",
+        mem_gb=DEFAULT_MEMORY_MIN_GB,


Suggested change

mem_gb=DEFAULT_MEMORY_MIN_GB,

mem_gb=DEFAULT_MEMORY_MIN_GB,

run_without_submitting=True,

mgxd · 2022-09-20T14:55:37Z

nibabies/interfaces/volume.py

+    surface = File(
+        exists=True,
+        mandatory=True,
+        argstr="%s ",


I don't think you need to add a trailing space for these

Suggested change

argstr="%s ",

argstr="%s",

mgxd · 2022-09-20T14:57:15Z

nibabies/workflows/bold/resampling.py

+    )
+
+    cov_ribbon_mean = pe.Node(
+        fsl.ImageStats(op_string='-M '),


Trailing spaces again.

Suggested change

fsl.ImageStats(op_string='-M '),

fsl.ImageStats(op_string='-M'),

mgxd · 2022-09-22T18:24:22Z

@madisoth @effigies for the sake of keeping things consistent, I'm going to move my review over to fmriprep. Feel free to leave this as a draft or close it, and then once it has been merged within fmriprep, we can port that over and make minor changes as needed.

effigies · 2022-09-22T18:27:11Z

Ah, sorry, I hadn't noticed you'd started a review over here. Looks like we at least were thinking in the same direction where we overlapped.

Removed duplicate code by abstracting it into functions, clarifying the goodvoxels logic

- Implemented the formatting suggestions by Mathias at the URLs below: - nipreps#235 - nipreps/fmriprep#2855 - Split the 3 workflow-connect blocks in the project-goodvoxels section of resampling.py into 3 different functions: ribbon, outliers, and vol-to-surf - Incomplete comments in resampling.py

Reorganized/cleaned goodvoxels resampling.py

… into madisoth-goodvoxels

mgxd

Hi @GregConan - thanks for contributing and pushing on this! I started going through and leaving some comments - let me know if anything doesn't make sense, or if you would like to schedule a call to go over some things.

mgxd · 2022-12-07T18:57:16Z

nibabies/interfaces/volume.py

@@ -0,0 +1,133 @@
+# -*- coding: utf-8 -*-


I think this would be better suited to go in the workbench module https://github.com/nipreps/nibabies/blob/master/nibabies/interfaces/workbench.py

mgxd · 2022-12-07T18:58:44Z

nibabies/workflows/bold/base.py

@@ -321,6 +321,7 @@ def init_func_preproc_wf(bold_file, has_fieldmap=False, existing_derivatives=Non
                "anat2std_xfm",
                "std2anat_xfm",
                "template",
+                "anat_giftis",


This should probably be anat_surfaces to avoid confusion, now that morphometric files (which are also giftis) are available

mgxd · 2022-12-07T19:01:39Z

nibabies/workflows/bold/resampling.py

+                "subjects_dir",
+                "t1w2fsnative_xfm",
+                "anat_giftis",
+                "t1w_mask",


Not sure if this is T1w specific, but one goal for down the line is to allow T1/T2 only processing, so to future-proof may want to rename this to anat_mask

mgxd · 2022-12-07T19:04:36Z

nibabies/workflows/bold/resampling.py

+    if project_goodvoxels:
+        workflow.__desc__ += """\
+Before resampling, a "goodvoxels" mask [@hcppipelines] was applied,
+excluding voxels whose time-series have a locally high coetfficient of


Suggested change

excluding voxels whose time-series have a locally high coetfficient of

excluding voxels whose time-series have a locally high coefficient of

mgxd · 2022-12-07T19:08:40Z

nibabies/workflows/bold/resampling.py


-    from niworkflows.interfaces.freesurfer import MedialNaNs
+def make_ribbon_file(workflow, mem_gb, inputnode):


This seems like an anatomical workflow, so it should go in the https://github.com/nipreps/nibabies/tree/master/nibabies/workflows/anatomical module.

Also, to stay consistent with nipreps best practices, let's rename/restructure this as a separate workflow.

Suggested change

def make_ribbon_file(workflow, mem_gb, inputnode):

def init_ribbon_volume_wf(mem_gb, name='ribbon_volume_wf'):

mgxd · 2022-12-07T19:17:19Z

nibabies/workflows/bold/resampling.py

+
+
+def apply_med_surf_nans_if(
+    medial_surface_nan, workflow, inputnode, sampler, update_metadata, medial_nans


We generally stay away from passing workflows / inputs / nodes as parameters, as that greatly complicates the logic / testing.

These connections can be applied within the init_ workflow function

mgxd · 2022-12-07T19:17:37Z

nibabies/workflows/bold/resampling.py

+    return workflow
+
+
+def connect_bold_surf_wf(


Same as above

FIX: Minor anat wf & workbench.py fixes (+5 squashed commit) Squashed commit: [e995ec4] RF: Moved ribbon wf (from bold to anat) and reformatted [9676f14] Updated Docker version tag [532875e] STY: black and isort [946dc7e] Formatting fixes and resampling.py modularization - Implemented the formatting suggestions by Mathias at the URLs below: - nipreps#235 - nipreps/fmriprep#2855 - Split the 3 workflow-connect blocks in the project-goodvoxels section of resampling.py into 3 different functions: ribbon, outliers, and vol-to-surf - Incomplete comments in resampling.py [114f982] Modularized goodvoxels code in resampling.py Removed duplicate code by abstracting it into functions, clarifying the goodvoxels logic

ENH: Add option to exclude projecting high variance voxels to surface (update of #235)

madisoth added 5 commits September 3, 2022 19:38

add options "--project-goodvoxels", "--surface-sampler", plus support…

42481ea

…ing Workbench interfaces

additional work on --project-goodvoxels, revert work on --surface-sam…

b14cf08

…pler option (to be continued in a separate branch)

docstring and style fixes

568191c

remove CreateSignedDistanceVolume doctest for now (needs anatomical s…

2493e8c

…urfaces for test sub)

docstring update

5bd684a

mgxd reviewed Sep 20, 2022

View reviewed changes

mgxd added the blocked needs dependency update label Sep 22, 2022

mgxd assigned madisoth Sep 23, 2022

mgxd changed the title ~~Enh/project goodvoxels~~ ENH: Add option to exclude projecting high variance voxels to surface Sep 23, 2022

GregConan added 3 commits November 24, 2022 03:45

Modularized goodvoxels code in resampling.py

114f982

Removed duplicate code by abstracting it into functions, clarifying the goodvoxels logic

STY: black and isort

532875e

GregConan mentioned this pull request Dec 5, 2022

Reorganized/cleaned goodvoxels resampling.py madisoth/nibabies#2

Merged

GregConan added 3 commits December 5, 2022 16:19

Merge pull request #2 from madisoth/enh/project-goodvoxels-refactor

0f426b8

Reorganized/cleaned goodvoxels resampling.py

Updated Docker version tag

9676f14

Merge branch 'enh/project-goodvoxels' of github.com:madisoth/nibabies…

ba5bac8

… into madisoth-goodvoxels

mgxd reviewed Dec 7, 2022

View reviewed changes

GregConan added 2 commits December 9, 2022 16:00

RF: Moved ribbon wf (from bold to anat) and reformatted

e995ec4

FIX: Minor anat wf & workbench.py fixes

7077ffb

mgxd added a commit that referenced this pull request Apr 13, 2023

Merge pull request #278 from madisoth/enh/project-goodvoxels-new

4de3897

ENH: Add option to exclude projecting high variance voxels to surface (update of #235)

mgxd merged commit bd2bfd0 into nipreps:master Apr 13, 2023

	def _list_outputs(self):
	outputs = super(CreateSignedDistanceVolume, self)._list_outputs()
	return outputs

	mem_gb=DEFAULT_MEMORY_MIN_GB,
	mem_gb=DEFAULT_MEMORY_MIN_GB,
	run_without_submitting=True,

	fsl.ImageStats(op_string='-M '),
	fsl.ImageStats(op_string='-M'),

	excluding voxels whose time-series have a locally high coetfficient of
	excluding voxels whose time-series have a locally high coefficient of


		from niworkflows.interfaces.freesurfer import MedialNaNs
		def make_ribbon_file(workflow, mem_gb, inputnode):

	def make_ribbon_file(workflow, mem_gb, inputnode):
	def init_ribbon_volume_wf(mem_gb, name='ribbon_volume_wf'):



		def apply_med_surf_nans_if(
		medial_surface_nan, workflow, inputnode, sampler, update_metadata, medial_nans

ENH: Add option to exclude projecting high variance voxels to surface #235

ENH: Add option to exclude projecting high variance voxels to surface #235

Uh oh!

Conversation

madisoth commented Sep 15, 2022

Uh oh!

codecov-commenter commented Sep 15, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mgxd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mgxd commented Sep 22, 2022

Uh oh!

effigies commented Sep 22, 2022

Uh oh!

mgxd left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov-commenter commented Sep 15, 2022 •

edited

Loading