Repository: dask/dask-jobqueue
Pull request #598 (2022-11-17 15:35:56)

[OAR] Rename memory per core property name and other tweaks
update following @lesteve 's comments on the PR #595 already merged

lesteve: Yep I'll try to have a look, thanks a lot @ychiat35 for the follow-up PR! 

Probably not this week, but next week should be doable!

Patch:
diff --git a/dask_jobqueue/jobqueue.yaml b/dask_jobqueue/jobqueue.yaml
index 3ce3dc0..bd7b9c5 100644
--- a/dask_jobqueue/jobqueue.yaml
+++ b/dask_jobqueue/jobqueue.yaml
@@ -26,7 +26,7 @@ jobqueue:
     job-extra-directives: []
     job-directives-skip: []
     log-directory: null
-    mem-core-property-name: null
+    memory-per-core-property-name: null
 
     # Scheduler options
     scheduler-options: {}
diff --git a/dask_jobqueue/oar.py b/dask_jobqueue/oar.py
index 36125e1..8dba580 100644
--- a/dask_jobqueue/oar.py
+++ b/dask_jobqueue/oar.py
@@ -1,8 +1,7 @@
 import logging
 import shlex
 import warnings
-
-from dask.utils import parse_bytes
+import math
 
 import dask
 
@@ -27,7 +26,7 @@ class OARJob(Job):
         project=None,
         resource_spec=None,
         walltime=None,
-        mem_core_property_name=None,
+        memory_per_core_property_name=None,
         config_name=None,
         **base_class_kwargs
     ):
@@ -82,33 +81,41 @@ class OARJob(Job):
 
         # Memory
         if self.worker_memory is not None:
-            if mem_core_property_name is None:
-                mem_core_property_name = dask.config.get(
-                    "jobqueue.%s.mem-core-property-name" % self.config_name
+            if memory_per_core_property_name is None:
+                memory_per_core_property_name = dask.config.get(
+                    "jobqueue.%s.memory-per-core-property-name" % self.config_name
                 )
-            if mem_core_property_name is None:
+            if memory_per_core_property_name is None:
                 warn = (
                     "OAR Job memory reserved resources will not be set according to Dask Worker memory limit, "
-                    "which can cause crashes."
+                    "which can cause crashes. See `memory_per_core_property_name` documentation for more details"
                 )
                 warnings.warn(warn, category=UserWarning)
             else:
-                # OAR expects MiB as memory unit
-                oar_memory = int(
-                    parse_bytes(self.worker_memory / self.worker_cores) / 2**20
-                )
-                # OAR needs to have the properties on a single line, with SQL syntax
-                # If there are several "#OAR -p" lines, only the last one will be taken into account by OAR
-                last_job_property = return_last_job_property(self.job_extra_directives)
-                if last_job_property is not None:
-                    header_lines.append(
-                        "#OAR -p '%s AND %s>=%s'"
-                        % (last_job_property, mem_core_property_name, oar_memory)
+                if memory_per_core_property_name != "not_applicable":
+                    # OAR expects MiB as memory unit
+                    oar_memory = math.ceil(
+                        self.worker_memory / (self.worker_cores * 2**20)
                     )
-                else:
-                    header_lines.append(
-                        "#OAR -p %s>=%s" % (mem_core_property_name, oar_memory)
+                    # OAR needs to have the properties on a single line, with SQL syntax
+                    # If there are several "#OAR -p" lines, only the last one will be taken into account by OAR
+                    last_job_property = return_last_job_property(
+                        self.job_extra_directives
                     )
+                    if last_job_property is not None:
+                        header_lines.append(
+                            "#OAR -p '%s AND %s>=%s'"
+                            % (
+                                last_job_property,
+                                memory_per_core_property_name,
+                                oar_memory,
+                            )
+                        )
+                    else:
+                        header_lines.append(
+                            "#OAR -p %s>=%s"
+                            % (memory_per_core_property_name, oar_memory)
+                        )
 
         self.job_header = "\n".join(header_lines)
 
@@ -154,15 +161,24 @@ class OARCluster(JobQueueCluster):
     walltime : str
         Walltime for each worker job.
     job_extra : list
-        Deprecated: use ``job_extra_directives`` instead. This parameter will be removed in a future version.
+        Deprecated: use ``job_extra_directives`` instead. This parameter will
+        be removed in a future version.
     job_extra_directives : list
-        List of other OAR options, for example `-t besteffort`. Each option will be prepended with the #OAR prefix.
-    mem_core_property_name : str
-        The memory per core property name of your OAR cluster (usually named `memcore` or `mem_core`).
-        Existing properties can be listed by executing `oarnodes` command.
+        List of other OAR options, for example `-t besteffort`. Each option
+        will be prepended with the #OAR prefix.
+    memory_per_core_property_name : str
+        The memory per core property name of your OAR cluster (usually
+        `memcore` or `mem_core`).
+        Existing properties can be listed by executing the `oarnodes` command.
         Note that the memory per core property might not exist on your cluster.
-        In this case, do not specify a value for mem_core_property_name parameter.
-        If this parameter is None, you will be warned that the memory parameter will not be taken into account by OAR.
+        If that is the case, you can set
+        `memory_per_core_property_name="not_applicable"` to avoid getting a warning.
+        If you leave `memory_per_core_property_name` set to its default value,
+        you will get a warning.
+        `memory_per_core_property_name` is `"not_applicable"` or set to its
+        default value, allocated nodes may not have enough memory to match the
+        `memory` parameter and Dask worker memory management may not work
+        correctly.
 
     Examples
     --------


Test patch:
diff --git a/dask_jobqueue/tests/test_oar.py b/dask_jobqueue/tests/test_oar.py
index abc2449..52fd51b 100644
--- a/dask_jobqueue/tests/test_oar.py
+++ b/dask_jobqueue/tests/test_oar.py
@@ -28,7 +28,9 @@ def test_header():
         assert "#OAR -q regular" in cluster.job_header
         assert "#OAR -t besteffort" in cluster.job_header
 
-    with OARCluster(cores=4, memory="8GB", mem_core_property_name="memcore") as cluster:
+    with OARCluster(
+        cores=4, memory="8GB", memory_per_core_property_name="memcore"
+    ) as cluster:
         assert "#OAR -n dask-worker" in cluster.job_header
         assert "walltime=" in cluster.job_header
         assert "#OAR -p memcore" in cluster.job_header
@@ -40,21 +42,21 @@ def test_header():
         processes=4,
         cores=8,
         memory="8GiB",
-        mem_core_property_name="mem_core",
+        memory_per_core_property_name="mem_core",
     ) as cluster:
         assert "#OAR -n dask-worker" in cluster.job_header
         assert "#OAR -l /nodes=1/core=8,walltime=00:02:00" in cluster.job_header
         assert "#OAR -p mem_core>=1024" in cluster.job_header
 
     with OARCluster(
-        cores=4,
+        cores=5,
         memory="28MiB",
         job_extra_directives=["-p gpu_count=1"],
-        mem_core_property_name="mem_core",
+        memory_per_core_property_name="mem_core",
     ) as cluster:
         assert "#OAR -n dask-worker" in cluster.job_header
         assert "walltime=" in cluster.job_header
-        assert "#OAR -p 'gpu_count=1 AND mem_core>=7'" in cluster.job_header
+        assert "#OAR -p 'gpu_count=1 AND mem_core>=6'" in cluster.job_header
 
 
 def test_job_script():
@@ -63,7 +65,7 @@ def test_job_script():
         processes=4,
         cores=8,
         memory="28GB",
-        mem_core_property_name="memcore",
+        memory_per_core_property_name="memcore",
     ) as cluster:
         job_script = cluster.job_script()
         assert "#OAR" in job_script
@@ -143,7 +145,7 @@ def test_config_name_oar_takes_custom_config():
         "job-cpu": None,
         "job-mem": None,
         "resource-spec": None,
-        "mem-core-property-name": "memcore",
+        "memory-per-core-property-name": "memcore",
     }
 
     with dask.config.set({"jobqueue.oar-config-name": conf}):
@@ -151,7 +153,7 @@ def test_config_name_oar_takes_custom_config():
             assert cluster.job_name == "myname"
 
 
-def test_mem_core_property_name_none_warning():
+def test_memory_per_core_property_name_none_warning():
     import warnings
 
     # test issuing of warning
@@ -175,8 +177,20 @@ def test_mem_core_property_name_none_warning():
         job = job_cls(
             cores=1,
             memory="1 GB",
-            mem_core_property_name="memcore",
+            memory_per_core_property_name="memcore",
         )
         assert len(w) == 0
         job_script = job.job_script()
         assert "#OAR -p memcore" in job_script
+
+    with warnings.catch_warnings(record=True) as w:
+        # should not give a warning
+        # should not take into account the memory parameter either
+        job = job_cls(
+            cores=1,
+            memory="1 GB",
+            memory_per_core_property_name="not_applicable",
+        )
+        assert len(w) == 0
+        job_script = job.job_script()
+        assert "#OAR -p" not in job_script