MRtrix3 / mrtrix3

MRtrix3 provides a set of tools to perform various advanced diffusion MRI analyses, including constrained spherical deconvolution (CSD), probabilistic tractography, track-density imaging, and apparent fibre density
http://www.mrtrix.org
Mozilla Public License 2.0
287 stars 178 forks source link

dwi2mask (dev): Not passing thread count to ants #2470

Open celstark opened 2 years ago

celstark commented 2 years ago

When using the new dev-tree version of dwi2mask, we have many more great options. At least for the ANTS-based options, though, the -nthreads is not propagating down to the antsRegistrationSynQuick calls unless you manually add it to -ants_options.

For example:

# 2m57s
time dwi2mask b02template in.mif b02template_affine.mif -nthreads 8 -config BZeroThreshold 12 -template template.mif template_mask.mif -ants_options "-t a"

#1m2s
time dwi2mask b02template in.mif b02template_affine2.mif -nthreads 8 -config BZeroThreshold 12 -template template.mif template_mask.mif -ants_options "-t a -n 8"

Or

#18m37s
time dwi2mask b02template in.mif b02template_syn.mif -nthreads 8 -config BZeroThreshold 12 -template template.mif template_mask.mif -ants_options "-n 8"

#60m33s
time dwi2mask b02template in.mif b02template_syn2.mif -nthreads 8 -config BZeroThreshold 12 -template template.mif template_mask.mif 

Presumably, if you've asked for N threads at the top, it should continue this in the supporting programs. I'd guess that somewhere around here: https://github.com/MRtrix3/mrtrix3/blob/6ff12ba6bf2cf41f663f72ebac5f50dfe06ec705/lib/mrtrix3/dwi2mask/b02template.py#L158 You'd want to add into your ants_options variable a the -n NTHREADS bit.

Lestropie commented 2 years ago

Correct: If MRtrix3 -nthreads is explicitly specified it should propagate. There's already mechanisms to do this for a couple of external packages; see code here. It looks from code here that antsRegistrationSynQuick.sh is reading from environment variable ITK_GLOBAL_DEFAULT_NUMBER_OF_THREADS, not ITK_GLOBAL_NUMBER_OF_THREADS as MRtrix3 expects. So that's a one-line addition (setting both), and redressing there is overall cleaner than attempting to insert command-line options (e.g. then you have to handle cases like -nthreads being used but also -ants_options including -n).

Any interest in forking and proposing the fix yourself (contribution guidelines)? I can do it quickly myself, but with the former you'd get registered credit for the change.

Thanks for the report Rob