Add --extra_slurm_options to srun_fastsurfer.sh for sbatch scheduling options

[eastagiletracker]

This PR proposes --extra_slurm_options, --extra_slurm_options_seg and --extra_slurm_options_surf for srun_fastsurfer.sh, so scheduling options such as --reservation actually reach the sbatch calls of the segmentation and surface jobs (fixes #586).

What was wrong

srun_fastsurfer.sh forwards every option it does not recognize to FastSurfer itself (POSITIONAL_FASTSURFER), so a scheduling option is not rejected — it is silently moved into the FastSurfer call inside the container. On dev at ef190b9, asking for a reserved node produces jobs that are scheduled without the reservation and then abort inside the container:

$ ./srun_fastsurfer.sh --data data --sd out --work work --pattern '*.nii.gz' \
    --fs_license license.txt --singularity_image fastsurfer.sif \
    --reservation gpu_nodes --dry
sbatch --parsable --mem=7G --cpus-per-task=16 -J FastSurfer-Seg- --time=20 -o WORK/logs/seg_%A_%a.log WORK/scripts/slurm_cmd_seg.sh --gpus-per-task=1
...
  /data/scripts/brun_fastsurfer.sh  --subject_list /data/scripts/subject_list --statusfile /data/scripts/subject_success --sd /data/cases --threads 16 --seg_only --reservation gpu_nodes --fs_license /data/scripts/.fs_license

The sbatch line carries no reservation, while --reservation gpu_nodes ends up on the brun_fastsurfer.sh command line, where run_fastsurfer.sh stops with ERROR: Flag '--reservation' unrecognized. for every case — after the jobs have already been submitted and started.

What this changes

Following the suggestion in #586 to generalize this in the style of --extra_singularity_options, rather than adding a single --reservation flag, this adds three options to srun_fastsurfer.sh: --extra_slurm_options for both jobs, plus --extra_slurm_options_seg and --extra_slurm_options_surf for one of them. Both the segmentation job and the surface job are covered, as requested in the issue. The options are inserted in front of the batch script path, because sbatch stops parsing options at the batch script and passes everything after it to the script instead. Option strings are split at whitespace, so several options can be given at once (documented in --help, which doc/scripts/SLURM.md renders). The cleanup and copy helper jobs are deliberately left alone, matching how --partition behaves today.

$ ./srun_fastsurfer.sh ... --extra_slurm_options_seg "--reservation=gpu_nodes --qos=high"
sbatch --parsable --mem=7G --cpus-per-task=16 -J FastSurfer-Seg- --time=20 -o WORK/logs/seg_%A_%a.log --reservation=gpu_nodes --qos=high WORK/scripts/slurm_cmd_seg.sh --gpus-per-task=1
sbatch --parsable --mem-per-cpu=3G --cpus-per-task=2 --ntasks=1 --nodes=1-1 --hint=nomultithread --depend=afterok:SEG_JOB_ID -J FastSurfer-Surf- -o WORK/logs/surf_%A_%a.log WORK/scripts/slurm_cmd_surf.sh

How it was verified

We ran srun_fastsurfer.sh --dry in a temporary directory and inspected the sbatch commands it prints — no SLURM, singularity or image data needed, and it skips itself off Linux since the script needs GNU coreutils. Six of our seven tests fail against dev at ef190b9 and pass with this change; the seventh asserts that the generated sbatch commands are byte-for-byte what they are today when none of the new options are given, and passes both before and after, which is the backward-compatibility check.

[dkuegler]

This PR looks nice. Thanks for the contribution.

I have a couple of feedback points, maybe you can rebase and address those issues?

[dkuegler]

Resolved comments. Probably squash-merge to drop the extra text script.

[Copilot]

🟡 Changes recommended

The segmentation sbatch argument order currently places --gpus-per-task=1 after the batch script path, so SLURM may not allocate GPUs as intended.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

Adds support in srun_fastsurfer.sh for forwarding arbitrary user-provided sbatch scheduling flags to the segmentation and surface sbatch submissions, addressing the common case where unrecognized SLURM flags were previously forwarded into the container instead of affecting scheduling.

Changes:

• Introduces --extra_slurm_options, --extra_slurm_options_seg, and --extra_slurm_options_surf and injects them into the sbatch argument lists.
• Adjusts --extra_singularity_options* parsing to allow multiple occurrences by accumulating values.
• Adds logic to split extra SLURM options into argument arrays before composing sbatch commands.

File summaries

File	Description
srun_fastsurfer.sh	Adds extra SLURM option forwarding and adjusts scheduling argument composition for seg/surf jobs.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 3
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

The segmentation GPU request remains positioned after the batch-script path and is therefore not parsed by sbatch.

Review details

Suppressed comments (1)

srun_fastsurfer.sh:604

• $seg_cmd_filename still precedes the subsequently appended --gpus-per-task=1. Because sbatch treats arguments after the script path as script arguments, GPU segmentation jobs will be submitted without requesting a GPU. Append the script path only after the conditional GPU option.

                   -o "$hpc_work/logs/seg_%A_%a.log" "${slurm_extra_seg[@]}" "$seg_cmd_filename")
  if [[ "$cpu_only" == "true" ]] ; then debug "Schedule SLURM job without gpu"
  else seg_slurm_sched+=(--gpus-per-task=1)
  fi

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[dkuegler]

🔵 Needs a closer look

The segmentation GPU request remains positioned after the batch-script path and is therefore not parsed by sbatch.

Failed to push commit :-/

[Copilot]

🟢 Approval recommended

The core forwarding and ordering logic looks correct and addresses the reported SLURM option-loss behavior, with only minor wording/maintainer-clarity nits noted.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 2
• Review effort level: Lite