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: Uniformize the grid&affine across EPI "blips" before TOPUP #197

Merged
merged 5 commits into from
May 14, 2021

Conversation

mgxd
Copy link
Contributor

@mgxd mgxd commented Apr 28, 2021

Closes #192

Adds a preprocessing workflow for TOPUP's EPI files.

The workflow:

  • Takes in a list of EPI files, each pertaining to one phase-encoding (PE) direction.
  • Flattens any 4D EPIs into 3D files, and makes a template "blip".
  • Applies N4's INU correction.
  • Registers all "blips" to the same space.
  • Merges the "blips" into a 4D file to be used by TOPUP.

@pep8speaks
Copy link

pep8speaks commented Apr 28, 2021

Hello @mgxd, Thank you for updating!

Cheers! There are no style issues detected in this Pull Request. 🍻 To test for issues locally, pip install flake8 and then run flake8 sdcflows.

Comment last updated at 2021-05-14 05:48:07 UTC

@mgxd
Copy link
Contributor Author

mgxd commented Apr 28, 2021

@oesteban when testing this branch, TOPUP is instantly failing with the following:

command
topup --config=/usr/local/miniconda/lib/python3.7/site-packages/sdcflows/data/flirtsch/b02b0.cnf --datain=/scratch/fmriprep_wf/single_subject_voice969_wf/fmap_preproc_wf/wf_auto_00000/topup/template_corrected_merged_encfile.txt --imain=/scratch/fmriprep_wf/single_subject_voice969_wf/fmap_preproc_wf/wf_auto_00000/prepare_blips_wf/merge_blips/template_corrected_merged.nii.gz --out=template_corrected_merged_base --iout=template_corrected_merged_corrected.nii.gz --fout=template_corrected_merged_field.nii.gz --jacout=jac --logout=template_corrected_merged_topup.log --rbmout=xfm --dfout=warpfield
Standard output:

Standard error:

Part of FSL (build 509)
topup

Usage: 
topup --imain=<some 4D image> --datain=<text file> --config=<text file with parameters> --out=my_topup_results


Compulsory arguments (You MUST set one or more of):
	--imain		name of 4D file with images
	--datain	name of text file with PE directions/times

Optional arguments (You may optionally specify one or more of):
	--out		base-name of output files (spline coefficients (Hz) and movement parameters)
	--fout		name of image file with field (Hz)
	--iout		name of 4D image file with unwarped images
	--logout	Name of log-file
	--warpres	(approximate) resolution (in mm) of warp basis for the different sub-sampling levels, default 10
	--subsamp	sub-sampling scheme, default 1
	--fwhm		FWHM (in mm) of gaussian smoothing kernel, default 8
	--config	Name of config file specifying command line arguments
	--miter		Max # of non-linear iterations, default 5
	--lambda	Weight of regularisation, default depending on --ssqlambda and --regmod switches. See user documetation.
	--ssqlambda	If set (=1), lambda is weighted by current ssq, default 1
	--regmod	Model for regularisation of warp-field [membrane_energy bending_energy], default bending_energy
	--estmov	Estimate movements if set, default 1 (true)
	--minmet	Minimisation method 0=Levenberg-Marquardt, 1=Scaled Conjugate Gradient, default 0 (LM)
	--splineorder	Order of spline, 2->Qadratic spline, 3->Cubic spline. Default=3
	--numprec	Precision for representing Hessian, double or float. Default double
	--interp	Image interpolation model, linear or spline. Default spline
	--scale		If set (=1), the images are individually scaled to a common mean, default 0 (false)
	--regrid		If set (=1), the calculations are done in a different grid, default 1 (true)
	-h,--help	display help info
	-v,--verbose	Print diagonostic information while running
	-h,--help	display help info




Return code: 1

It seems it doesn't like the --config option, though I'm not sure why (all files look like what we'd expect)

@codecov
Copy link

codecov bot commented Apr 28, 2021

Codecov Report

Merging #197 (b8a9872) into master (102a73a) will increase coverage by 2.14%.
The diff coverage is 88.88%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #197      +/-   ##
==========================================
+ Coverage   94.45%   96.59%   +2.14%     
==========================================
  Files          23       23              
  Lines        1622     1645      +23     
  Branches      188      189       +1     
==========================================
+ Hits         1532     1589      +57     
+ Misses         61       30      -31     
+ Partials       29       26       -3     
Flag Coverage Δ
travis 89.65% <88.88%> (-0.04%) ⬇️
unittests 96.58% <88.88%> (+2.27%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
sdcflows/workflows/fit/pepolar.py 96.66% <88.88%> (-3.34%) ⬇️
sdcflows/interfaces/utils.py 98.86% <0.00%> (+5.11%) ⬆️
sdcflows/interfaces/brainmask.py 100.00% <0.00%> (+43.75%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 102a73a...b8a9872. Read the comment docs.

@oesteban oesteban force-pushed the enh/pepolar-alignment branch 2 times, most recently from d02b869 to 31d54f3 Compare May 13, 2021 07:43
@oesteban oesteban marked this pull request as draft May 13, 2021 07:46
@oesteban
Copy link
Member

oesteban commented May 13, 2021

@mgxd I'll be working on this until the meeting.

@mgxd
Copy link
Contributor Author

mgxd commented May 13, 2021

when testing this branch, TOPUP is instantly failing
It seems it doesn't like the --config option, though I'm not sure why (all files look like what we'd expect)

I was able to track this down - turns out it was (like you suggested) a problem with the even/odd number of slices.

@oesteban oesteban force-pushed the enh/pepolar-alignment branch 2 times, most recently from dd2f93f to 95d4157 Compare May 13, 2021 13:21
@oesteban
Copy link
Member

Let's split the problem:

  • The even/odd number of voxels (although it is weird it worked if passing arguments in the command line)
  • The differences in grinding

I've been thinking this through and I'm more convinced that we should use the reference workflow as input for the topup workflow. But, before that can be done, I guess we can just trust that inputs are roughly aligned - TOPUP will run head-motion internally anyways.

So, I'm testing a simplification where we only add one node that ensures all volumes at the input have the same shape/affine at the output.

oesteban added a commit to oesteban/sdcflows that referenced this pull request May 13, 2021
The new tests also exercise the problems being addressed in nipreps#197.
oesteban added a commit to oesteban/sdcflows that referenced this pull request May 13, 2021
The new tests also exercise the problems being addressed in nipreps#197.
oesteban added a commit to oesteban/sdcflows that referenced this pull request May 13, 2021
The new tests also exercise the problems being addressed in nipreps#197.
oesteban added a commit to oesteban/sdcflows that referenced this pull request May 13, 2021
The new tests also exercise the problems being addressed in nipreps#197.
oesteban added a commit to oesteban/sdcflows that referenced this pull request May 13, 2021
The new tests also exercise the problems being addressed in nipreps#197.
oesteban added a commit to mgxd/sdcflows that referenced this pull request May 13, 2021
The new tests also exercise the problems being addressed in nipreps#197.
@oesteban oesteban force-pushed the enh/pepolar-alignment branch 2 times, most recently from 545bcd3 to 372efb2 Compare May 13, 2021 16:37
mgxd and others added 4 commits May 13, 2021 11:08
This commit is the result of squashing three previous commits:

  * WIP: Pepolar alignment workflow to ensure EPI "blips" are in register
  * ENH: EPI preproc workflow before TOPUP
  * STY: Black, move readout node to topup workflow

and then rebasing on upstream/master.
@oesteban oesteban marked this pull request as ready for review May 13, 2021 18:10
@oesteban
Copy link
Member

@oesteban oesteban force-pushed the enh/pepolar-alignment branch 2 times, most recently from a554c9b to a1491f1 Compare May 14, 2021 05:25
@oesteban
Copy link
Member

Alignment is only necessary for debug settings as the topup config does not apply it.

@codecov-commenter
Copy link

Codecov Report

Merging #197 (38f30ca) into master (d184cec) will increase coverage by 3.82%.
The diff coverage is 42.85%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #197      +/-   ##
==========================================
+ Coverage   89.72%   93.55%   +3.82%     
==========================================
  Files          24       24              
  Lines        1645     1676      +31     
  Branches      193      193              
==========================================
+ Hits         1476     1568      +92     
+ Misses        142       81      -61     
  Partials       27       27              
Flag Coverage Δ
travis 88.84% <42.85%> (-0.89%) ⬇️
unittests 93.41% <42.85%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
sdcflows/interfaces/utils.py 83.33% <29.41%> (-10.42%) ⬇️
sdcflows/interfaces/epi.py 100.00% <100.00%> (ø)
sdcflows/workflows/fit/pepolar.py 100.00% <100.00%> (+7.22%) ⬆️
sdcflows/workflows/outputs.py 93.54% <0.00%> (+1.61%) ⬆️
sdcflows/interfaces/reportlets.py 89.83% <0.00%> (+1.69%) ⬆️
sdcflows/interfaces/bspline.py 93.71% <0.00%> (+3.66%) ⬆️
sdcflows/interfaces/fmap.py 96.96% <0.00%> (+19.69%) ⬆️
sdcflows/workflows/fit/syn.py 94.16% <0.00%> (+27.73%) ⬆️
sdcflows/utils/phasemanip.py 94.02% <0.00%> (+28.35%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d184cec...38f30ca. Read the comment docs.

@oesteban oesteban changed the title ENH: Align EPI "blips" before TOPUP ENH: Uniformize the grid&affine across EPI "blips" before TOPUP May 14, 2021
@oesteban oesteban merged commit 336c611 into nipreps:master May 14, 2021
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.

Improve PEPOLAR robustness
4 participants