Skip to content

fix: avoid 'None' prefix in SlurmExecutor job names - #565

Merged
ko3n1g merged 3 commits into
NVIDIA-NeMo:mainfrom
andrewwhitecdw:andrewwhitecdw/fix/slurm-none-job-name-prefix
Aug 7, 2026
Merged

fix: avoid 'None' prefix in SlurmExecutor job names#565
ko3n1g merged 3 commits into
NVIDIA-NeMo:mainfrom
andrewwhitecdw:andrewwhitecdw/fix/slurm-none-job-name-prefix

Conversation

@andrewwhitecdw

Copy link
Copy Markdown
Contributor

Bug\nSlurmExecutor.job_name_prefix defaults to None, so the alloc() and srun() f-strings produced job names like Noneinteractive.\n\n## Fix\nUse self.job_name_prefix or '' in both methods, matching the existing handling elsewhere in the file (lines 825–830 and 980).\n\n## Test\nAdded test/core/execution/test_slurm_job_name.py with regression tests for both alloc() and srun(), covering the unset-prefix and set-prefix cases.\n\n## Verification\nuv run pytest test/core/execution/test_slurm_job_name.py -v passes.\nuv run --group lint ruff check ... and ruff format --check ... pass.

SlurmExecutor.job_name_prefix defaults to None, so f-strings in alloc() and

srun() produced job names like 'Noneinteractive'. Use  so the prefix

is omitted when unset, matching the existing handling in other methods.

Adds regression tests for both alloc() and srun().

Signed-off-by: Andrew White <andrewh@cdw.com>

Copy link
Copy Markdown
Contributor

PR #565 — LGTM
Blocking findings: None.
Non-blocking findings: None. nemo_run/core/execution/slurm.py:436-460 now normalizes an unset prefix to an empty string in both alloc() and srun(). test/core/execution/test_slurm_job_name.py:22-75 covers unset and configured prefixes for both paths.
Validation gaps: Only successful DCO is reported; no test or lint result is visible.

@ko3n1g
ko3n1g enabled auto-merge (squash) August 7, 2026 11:45
@ko3n1g
ko3n1g merged commit 9339e12 into NVIDIA-NeMo:main Aug 7, 2026
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants