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

omp_nthreads not passed to merge_and_denoise_wf on one branch #720

Open
psadil opened this issue Mar 28, 2024 · 1 comment
Open

omp_nthreads not passed to merge_and_denoise_wf on one branch #720

psadil opened this issue Mar 28, 2024 · 1 comment
Labels
bug Something isn't working

Comments

@psadil
Copy link
Contributor

psadil commented Mar 28, 2024

Summary

The merge_and_denoise_wf has a few nodes that can benefit from multithreading, but it looks like there is one instance where omp_nthreads isn't passed.

merge_dwis = init_merge_and_denoise_wf(
raw_dwi_files=dwi_series,
b0_threshold=b0_threshold,
dwi_denoise_window=dwi_denoise_window,
denoise_method=denoise_method,
unringing_method=unringing_method,
dwi_no_biascorr=dwi_no_biascorr,
no_b0_harmonization=no_b0_harmonization,
denoise_before_combining=denoise_before_combining,
orientation=orientation,
calculate_qc=True,
phase_id=dwi_series_pedir,
source_file=source_file,
ignore=ignore,
layout=layout,
)

This results in a few unnecessarily slow steps (e.g., raw_gqi using only 1 thread)

Additional details

  • QSIPrep version:
  • Docker version:
  • Singularity version:
$ apptainer run docker://pennbbl/qsiprep --version
INFO:    Using cached SIF image
qsiprep v0.20.1.dev0+geda84d5.d20240112

What were you trying to do?

Test a patch on qsiprep

What did you expect to happen?

I expected it to run a bit faster, and for the thread_counts in these two command.txt files to match

$ cat $SS_ID_WF/dwi_preproc_ses_RU_wf/pre_hmc_wf/dwi_qc_wf/raw_gqi/command.txt
dsi_studio --action=rec --method=4 --align_acpc=0 --check_btable=0 --dti_no_high_b=1 --source=/tmp/work/qsiprep_wf/single_subject_travel2_wf/dwi_preproc_ses_RU_wf/pre_hmc_wf/dwi_qc_wf/raw_gqi/sub-travel2_ses-RU_dwi_merged.src.gz --num_fiber=3 --thread_count=8 --other_output=all --record_odf=1 --r2_weighted=0 --param0=1.2500 --thread_count=8


$ cat $SS_ID_WF/dwi_preproc_ses_RU_wf/pre_hmc_wf/merge_and_denoise_wf/dwi_qc_wf/raw_gqi/command.txt 

dsi_studio --action=rec --method=4 --align_acpc=0 --check_btable=0 --dti_no_high_b=1 --source=/tmp/work/qsiprep_wf/single_subject_travel2_wf/dwi_preproc_ses_RU_wf/pre_hmc_wf/merge_and_denoise_wf/dwi_qc_wf/raw_gqi/sub-travel2_ses-RU_dwi_merged.src.gz --num_fiber=3 --thread_count=1 --other_output=all --record_odf=1 --r2_weighted=0 --param0=1.2500 --thread_count=1

What actually happened?

thread_count was set to 1 for the dwi_qc workflow as called from the merge_and_denoise workflow

Reproducing the bug

@psadil psadil added the bug Something isn't working label Mar 28, 2024
@psadil psadil changed the title opm_nthreads not passed to merge_and_denoise_wf on some branches omp_nthreads not passed to merge_and_denoise_wf on some branches Mar 28, 2024
@psadil psadil changed the title omp_nthreads not passed to merge_and_denoise_wf on some branches omp_nthreads not passed to merge_and_denoise_wf on one branch Mar 28, 2024
@mattcieslak
Copy link
Collaborator

good catch, thank you! Adding this fix in #725

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants