Deep-MI / FastSurfer

PyTorch implementation of FastSurferCNN
Apache License 2.0
459 stars 119 forks source link

[Question] Parallel processing in recon-surf #22

Closed tashrifbillah closed 4 years ago

tashrifbillah commented 4 years ago

In this block, you have set up a bunch of variables for parallel processing:

fsthreads=""
export OMP_NUM_THREADS=$threads
export ITK_GLOBAL_DEFAULT_NUMBER_OF_THREADS=$threads
if [ "$threads" -gt "1" ]
then
  fsthreads="-threads $threads -itkthreads $threads"
fi

while this says -parallel -openmp <num> only. So why is this discrepancy?

m-reuter commented 4 years ago

-threads is equivalent with -openmp . Additionally, we set the itkthreads to the same, which is set to 4 by the parallel flag.

tashrifbillah commented 4 years ago

which is set to 4 by the parallel flag

It is modified by -openmp <num> according to the documentation I attached before. Where do you get direction about itkthreads?

LeHenschel commented 4 years ago

It is the default when you only specify -parallel for recon-all: https://github.com/freesurfer/freesurfer/blob/dev/scripts/recon-all#L7317

    case "-parallel":
      set DoParallel = 1;
      if ( ! $OMP_NUM_SET ) then
        # the user can override this thread count with -openmp
        setenv OMP_NUM_THREADS 4
        setenv FS_OMP_NUM_THREADS 4 # support for re-entrant recon-all
      endif
      # override: -itkthreads <num_threads>
      setenv ITK_GLOBAL_DEFAULT_NUMBER_OF_THREADS 4
      breaksw
tashrifbillah commented 4 years ago

Sure, all I am saying is-- the default should be modified by -openmp <num> flag. I don't see any documentation on FreeSurfer that says modify that using -itkthreads flag.

LeHenschel commented 4 years ago

I do not quite understand what you mean. If you set -parallel in FreeSurfer, the ITK_GLOBAL_DEFAULT_NUMBER_OF_THREADS and OMP_NUM_THREADS will both be set to 4 as per the code I posted above. You can separately set a different value with -openmp and -itkthreads:

https://github.com/freesurfer/freesurfer/blob/dev/scripts/recon-all#L7328 https://github.com/freesurfer/freesurfer/blob/dev/scripts/recon-all#L7337

Do you want us to include a separate flag for -itkthreads in FastSurfer, so you can set this to a value of your choice?

tashrifbillah commented 4 years ago

The two references help. Thanks.

Do you want us to include a separate flag for -itkthreads

No, you are good.

tashrifbillah commented 4 years ago

Referencing my thread from FreeSurfer forum. It looks like -parallel openmp <num> is enough for parallelization:

https://www.mail-archive.com/freesurfer@nmr.mgh.harvard.edu/msg67061.html