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

Update the way parallelization is configured #651

Merged
merged 5 commits into from
Feb 7, 2025

Conversation

dkuegler
Copy link
Member

@dkuegler dkuegler commented Feb 6, 2025

  • Update the --parallel_subjects* flags and rename them to --parallel* (brun_fastsurfer.sh, long_fastsurfer.sh)

  • Update the "old" --parallel flag (making it legacy, removing from documentation everywhere incl. all examples, but also from the "functionality of the scripts")

  • Update the way --threads works for surface reconstruction. Before: run_fastsurfer.sh fixes --threads value, if --parallel is set; Now: run_fastsurfer.sh does nothing to the threads value for surface reconstruction, but recon-surf.sh has 2 values for threads: for joint parts and for parallel hemisphere parts; if threads is >= 2 parallel hemispheres is automatically activated and the number of threads available to each parallel hemisphere is halved.

  • Update the documentation throughout.

- Update the --parallel_subjects* flags and rename them to --parallel* (brun_fastsurfer.sh, long_fastsurfer.sh)
- Update the "old" --parallel flag (making it legacy, removing from documentation everywhere incl. all examples, but also from the "functionality of the scripts")
- Update the way --threads works for surface reconstruction. Before: run_fastsurfer.sh fixes --threads value, if --parallel is set; Now: run_fastsurfer.sh does nothing to the threads value for surface reconstruction, but recon-surf.sh has 2 values for threads: for joint parts and for parallel hemisphere parts; if threads is >= 2 parallel hemispheres is automatically activated and the number of threads available to each parallel hemisphere is halved.

- Update the documentation throughout.
@dkuegler dkuegler force-pushed the feature/change-parallel-flags branch from a1bb507 to 08fd189 Compare February 6, 2025 16:19
Docker/README.md Outdated
@@ -27,8 +27,7 @@ docker run --gpus all -v /home/user/my_mri_data:/data \
--rm --user $(id -u):$(id -g) deepmi/fastsurfer:latest \
--fs_license /fs_license/license.txt \
--t1 /data/subjectX/t1-weighted.nii.gz \
--sid subjectX --sd /output \
--parallel
--sid subjectX --sd /output
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't we have to put threads to 2 in order to have parallel behaviour?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same further below.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely, I mean, we can also put 2 threads as the default ....

I just removed the --parallel flags because those are not really wanted/needed any more.


### Single parallel pipeline
This mode is ideal for CPU-based processing for segmentation. It will process segmentations and surfaces in series
in the same process, but multiple cases are processed at the same time.

```bash
$FASTSURFER_HOME/brun_fastsurfer.sh --parallel_subjects 4 --threads 2 --parallel
$FASTSURFER_HOME/brun_fastsurfer.sh --parallel 4 --threads 2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for same behaviour as before threads would need to be 4?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, but the text below is updated to reflect the behavior as "requested".

m-reuter and others added 3 commits February 7, 2025 12:10
… the logfile exists.

Append those messages to the logfile once the logfile is created. Cleanup of the temporary logfile.
(Don't care if the temporary logfile gets messages, but run_fastsurfer.sh is terminated because of a different error. It is only a temporary file that is very small.)
…md and Docker/README.md. Also remove some outdated information in Docker/README.md.
@m-reuter m-reuter merged commit 5212e47 into Deep-MI:dev Feb 7, 2025
3 checks passed
@dkuegler dkuegler deleted the feature/change-parallel-flags branch February 7, 2025 16:25
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.

2 participants