Merge pull request #496 from ExaWorks/default_node_count_slurm

Use default values for resource requests

Author: hategan

src/psij/executors/batch/cobalt/cobalt.mustache

@@ -6,12 +6,12 @@
 #COBALT --cwd={{.}}
 {{/job.spec.directory}}
 {{#job.spec.resources}}
-    {{#node_count}}
+    {{#computed_node_count}}
 #COBALT --nodecount={{.}}
-    {{/node_count}}
-    {{#process_count}}
+    {{/computed_node_count}}
+    {{#computed_process_count}}
 #COBALT --proccount={{.}}
-    {{/process_count}}
+    {{/computed_process_count}}
 {{/job.spec.resources}}
 {{#formatted_job_duration}}
 #COBALT --time={{duration}}

src/psij/executors/batch/lsf/lsf.mustache

@@ -20,13 +20,13 @@
 
 {{#job.spec.resources}}
 
-    {{#node_count}}
+    {{#computed_node_count}}
 #BSUB -nnodes {{.}}
-    {{/node_count}}
+    {{/computed_node_count}}
 
-    {{#process_count}}
+    {{#computed_process_count}}
 #BSUB -n {{.}}
-    {{/process_count}}
+    {{/computed_process_count}}
 
     {{#gpu_cores_per_process}}
 #BSUB -gpu num={{.}}/task

src/psij/executors/batch/slurm/slurm.mustache

@@ -14,17 +14,17 @@
 #SBATCH --exclusive
     {{/exclusive_node_use}}
 
-    {{#node_count}}
+    {{#computed_node_count}}
 #SBATCH --nodes={{.}}
-    {{/node_count}}
+    {{/computed_node_count}}
 
-    {{#process_count}}
+    {{#computed_process_count}}
 #SBATCH --ntasks={{.}}
-    {{/process_count}}
+    {{/computed_process_count}}
 
-    {{#processes_per_node}}
+    {{#computed_processes_per_node}}
 #SBATCH --ntasks-per-node={{.}}
-    {{/processes_per_node}}
+    {{/computed_processes_per_node}}
 
     {{#gpu_cores_per_process}}
 #SBATCH --gpus-per-task={{.}}

src/psij/job_executor.py

@@ -14,11 +14,15 @@
 from psij.job_executor_config import JobExecutorConfig
 from psij.job_launcher import Launcher
 from psij.job_spec import JobSpec
+from psij.resource_spec import ResourceSpecV1
 
 
 logger = logging.getLogger(__name__)
 
 
+_DEFAULT_RESOURCES = ResourceSpecV1()
+
+
 class JobExecutor(ABC):
     """An abstract base class for all JobExecutor implementations."""
 
@@ -92,6 +96,8 @@ def _check_job(self, job: Job) -> JobSpec:
         spec = job.spec
         if not spec:
             raise InvalidJobException('Missing specification')
+        if not spec.resources:
+            spec.resources = _DEFAULT_RESOURCES
 
         if __debug__:
             if spec.environment is not None:

src/psij/resource_spec.py

@@ -54,7 +54,7 @@ def __init__(self, node_count: Optional[int] = None,
                  processes_per_node: Optional[int] = None,
                  cpu_cores_per_process: Optional[int] = None,
                  gpu_cores_per_process: Optional[int] = None,
-                 exclusive_node_use: bool = True) -> None:
+                 exclusive_node_use: bool = False) -> None:
         """
         Some of the properties of this class are constrained. Specifically,
         `process_count = node_count * processes_per_node`. Specifying all constrained properties

tests/plugins1/_batch_test/test/test.mustache

@@ -9,9 +9,9 @@ cd "{{.}}"
 export PSIJ_TEST_BATCH_EXEC_COUNT=1
 
 {{#job.spec.resources}}
-    {{#process_count}}
+    {{#computed_process_count}}
 export PSIJ_TEST_BATCH_EXEC_COUNT={{.}}
-    {{/process_count}}
+    {{/computed_process_count}}
 {{/job.spec.resources}}
 
 {{#job.spec.attributes}}

tests/test_executor.py

@@ -224,3 +224,17 @@ def test_submit_script_generation(exec_name: str) -> None:
                      lambda k, v: setattr(attrs, k, v))
     _check_str_attrs(ex, job, [prefix + '.cust_attr1', prefix + '.cust_attr2'],
                      lambda k, v: c_attrs.__setitem__(k, v))
+
+
+def test_resource_generation1() -> None:
+    res = ResourceSpecV1()
+    spec = JobSpec('/bin/date', resources=res)
+    job = Job(spec=spec)
+    ex = JobExecutor.get_instance('slurm')
+    assert isinstance(ex, BatchSchedulerExecutor)
+    with TemporaryFile(mode='w+') as f:
+        ex.generate_submit_script(job, ex._create_script_context(job), f)
+        f.seek(0)
+        contents = f.read()
+        if contents.find('--exclusive') != -1:
+            pytest.fail('Spurious exclusive flag')