Skip to content

Commit bd1f09e

Browse files
committed
feat: support container-image None in slurm
Signed-off-by: Hemil Desai <hemild@nvidia.com>
1 parent b1590e8 commit bd1f09e

2 files changed

Lines changed: 177 additions & 13 deletions

File tree

nemo_run/core/execution/slurm.py

Lines changed: 30 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -369,12 +369,12 @@ def merge(
369369
main_executor.run_as_group = True
370370

371371
if main_executor.het_group_indices:
372-
assert main_executor.heterogeneous, (
373-
"heterogeneous must be True if het_group_indices is provided"
374-
)
375-
assert len(main_executor.het_group_indices) == num_tasks, (
376-
"het_group_indices must be the same length as the number of tasks"
377-
)
372+
assert (
373+
main_executor.heterogeneous
374+
), "heterogeneous must be True if het_group_indices is provided"
375+
assert (
376+
len(main_executor.het_group_indices) == num_tasks
377+
), "het_group_indices must be the same length as the number of tasks"
378378
assert all(
379379
x <= y
380380
for x, y in zip(
@@ -858,9 +858,9 @@ def materialize(self) -> str:
858858

859859
sbatch_flags = []
860860
if self.executor.heterogeneous:
861-
assert len(self.jobs) == len(self.executor.resource_group), (
862-
f"Number of jobs {len(self.jobs)} must match number of resource group requests {len(self.executor.resource_group)}.\nIf you are just submitting a single job, make sure that heterogeneous=False in the executor."
863-
)
861+
assert (
862+
len(self.jobs) == len(self.executor.resource_group)
863+
), f"Number of jobs {len(self.jobs)} must match number of resource group requests {len(self.executor.resource_group)}.\nIf you are just submitting a single job, make sure that heterogeneous=False in the executor."
864864
final_group_index = len(self.executor.resource_group) - 1
865865
if self.executor.het_group_indices:
866866
final_group_index = self.executor.het_group_indices.index(
@@ -870,9 +870,9 @@ def materialize(self) -> str:
870870
for i in range(len(self.executor.resource_group)):
871871
resource_req = self.executor.resource_group[i]
872872
if resource_req.het_group_index is not None:
873-
assert self.executor.resource_group[i - 1].het_group_index is not None, (
874-
"het_group_index must be set for all requests in resource_group"
875-
)
873+
assert (
874+
self.executor.resource_group[i - 1].het_group_index is not None
875+
), "het_group_index must be set for all requests in resource_group"
876876
if (
877877
i > 0
878878
and resource_req.het_group_index
@@ -938,7 +938,18 @@ def get_container_flags(
938938
container_image: Optional[str],
939939
container_env: Optional[list[str]] = None,
940940
) -> list[str]:
941-
_container_flags = ["--container-image", container_image] if container_image else []
941+
"""Get srun flags for container or non-container mode.
942+
943+
For non-container mode, returns --chdir flag to set working directory.
944+
For container mode, returns container-related flags (image, mounts, workdir, env).
945+
"""
946+
if container_image is None:
947+
# Non-container mode: use --chdir to set working directory
948+
workdir = os.path.join(src_job_dir, "code")
949+
return ["--chdir", workdir]
950+
951+
# Container mode: set up container mounts and workdir
952+
_container_flags = ["--container-image", container_image]
942953

943954
new_mounts = copy.deepcopy(base_mounts)
944955
for i, mount in enumerate(new_mounts):
@@ -1079,6 +1090,12 @@ def get_container_flags(
10791090
vars_to_fill["fault_tol_job_results_file"] = self.launcher.job_results_file
10801091

10811092
sbatch_script = fill_template("slurm.sh.j2", vars_to_fill)
1093+
1094+
# For non-container mode, substitute /{RUNDIR_NAME} paths with actual job directory
1095+
if self.executor.container_image is None:
1096+
actual_job_dir = os.path.join(slurm_job_dir, job_directory_name)
1097+
sbatch_script = sbatch_script.replace(f"/{RUNDIR_NAME}", actual_job_dir)
1098+
10821099
return sbatch_script
10831100

10841101
def __repr__(self) -> str:

test/core/execution/test_slurm.py

Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,10 @@
1818

1919
import pytest
2020

21+
from nemo_run.config import RUNDIR_NAME
2122
from nemo_run.core.execution.launcher import SlurmTemplate, Torchrun
2223
from nemo_run.core.execution.slurm import (
24+
SlurmBatchRequest,
2325
SlurmExecutor,
2426
SlurmJobDetails,
2527
SlurmTunnelCallback,
@@ -403,3 +405,148 @@ def test_merge_mismatch(self):
403405
[SlurmExecutor(account="account1"), SlurmExecutor(account="account2")],
404406
num_tasks=3,
405407
)
408+
409+
410+
class TestSlurmBatchRequestNonContainerMode:
411+
"""Tests for non-container mode support (container_image=None)."""
412+
413+
@pytest.fixture
414+
def executor_with_container(self):
415+
"""Create an executor with container image."""
416+
executor = SlurmExecutor(
417+
account="test_account",
418+
partition="gpu",
419+
nodes=2,
420+
ntasks_per_node=8,
421+
container_image="nvcr.io/nvidia/pytorch:24.01-py3",
422+
container_mounts=["/data:/data"],
423+
)
424+
executor.job_name = "test-job"
425+
executor.experiment_dir = "/local/experiments"
426+
executor.job_dir = "/local/experiments/test-job"
427+
executor.experiment_id = "exp-123"
428+
429+
# Mock tunnel
430+
tunnel = MagicMock(spec=LocalTunnel)
431+
tunnel.job_dir = "/remote/experiments/exp-123"
432+
executor.tunnel = tunnel
433+
434+
return executor
435+
436+
@pytest.fixture
437+
def executor_without_container(self):
438+
"""Create an executor without container image (non-container mode)."""
439+
executor = SlurmExecutor(
440+
account="test_account",
441+
partition="gpu",
442+
nodes=2,
443+
ntasks_per_node=8,
444+
container_image=None, # Non-container mode
445+
)
446+
executor.job_name = "test-job"
447+
executor.experiment_dir = "/local/experiments"
448+
executor.job_dir = "/local/experiments/test-job"
449+
executor.experiment_id = "exp-123"
450+
451+
# Mock tunnel
452+
tunnel = MagicMock(spec=LocalTunnel)
453+
tunnel.job_dir = "/remote/experiments/exp-123"
454+
executor.tunnel = tunnel
455+
456+
return executor
457+
458+
def test_materialize_with_container_uses_container_flags(self, executor_with_container):
459+
"""Test that materialize uses container flags when container_image is set."""
460+
request = SlurmBatchRequest(
461+
launch_cmd=["sbatch", "--parsable"],
462+
jobs=["test-job"],
463+
command_groups=[["python train.py"]],
464+
executor=executor_with_container,
465+
max_retries=0,
466+
extra_env={},
467+
)
468+
469+
script = request.materialize()
470+
471+
# Should contain container flags
472+
assert "--container-image" in script
473+
assert "--container-mounts" in script
474+
assert "--container-workdir" in script
475+
# Should NOT contain --chdir (used for non-container mode)
476+
assert "--chdir" not in script
477+
# Should contain /nemo_run paths (not substituted)
478+
assert f"/{RUNDIR_NAME}" in script
479+
480+
def test_materialize_without_container_uses_chdir(self, executor_without_container):
481+
"""Test that materialize uses --chdir when container_image is None."""
482+
request = SlurmBatchRequest(
483+
launch_cmd=["sbatch", "--parsable"],
484+
jobs=["test-job"],
485+
command_groups=[["python train.py"]],
486+
executor=executor_without_container,
487+
max_retries=0,
488+
extra_env={},
489+
)
490+
491+
script = request.materialize()
492+
493+
# Should contain --chdir flag for working directory
494+
assert "--chdir" in script
495+
# Should NOT contain container flags
496+
assert "--container-image" not in script
497+
assert "--container-mounts" not in script
498+
assert "--container-workdir" not in script
499+
500+
def test_materialize_without_container_substitutes_rundir_paths(
501+
self, executor_without_container
502+
):
503+
"""Test that /{RUNDIR_NAME} paths are substituted with actual paths in non-container mode."""
504+
request = SlurmBatchRequest(
505+
launch_cmd=["sbatch", "--parsable"],
506+
jobs=["test-job"],
507+
command_groups=[["python train.py"]],
508+
executor=executor_without_container,
509+
max_retries=0,
510+
extra_env={},
511+
)
512+
513+
script = request.materialize()
514+
515+
# Should NOT contain /nemo_run paths (should be substituted)
516+
assert f"/{RUNDIR_NAME}/code" not in script
517+
# Should contain the actual job directory path
518+
actual_job_dir = "/remote/experiments/exp-123/test-job"
519+
assert f"{actual_job_dir}/code" in script
520+
521+
def test_materialize_with_container_preserves_rundir_paths(self, executor_with_container):
522+
"""Test that /{RUNDIR_NAME} paths are NOT substituted when using container."""
523+
request = SlurmBatchRequest(
524+
launch_cmd=["sbatch", "--parsable"],
525+
jobs=["test-job"],
526+
command_groups=[["python train.py"]],
527+
executor=executor_with_container,
528+
max_retries=0,
529+
extra_env={},
530+
)
531+
532+
script = request.materialize()
533+
534+
# Should contain /nemo_run paths (not substituted for container mode)
535+
assert f"/{RUNDIR_NAME}" in script
536+
537+
def test_non_container_mode_chdir_points_to_code_directory(self, executor_without_container):
538+
"""Test that --chdir in non-container mode points to the code directory."""
539+
request = SlurmBatchRequest(
540+
launch_cmd=["sbatch", "--parsable"],
541+
jobs=["test-job"],
542+
command_groups=[["python train.py"]],
543+
executor=executor_without_container,
544+
max_retries=0,
545+
extra_env={},
546+
)
547+
548+
script = request.materialize()
549+
550+
# The --chdir should point to {job_dir}/code
551+
expected_chdir = "--chdir /remote/experiments/exp-123/test-job/code"
552+
assert expected_chdir in script

0 commit comments

Comments
 (0)