From c6a2578d04058e2eb939a48641010fc1d68b3134 Mon Sep 17 00:00:00 2001 From: Guido Petretto Date: Wed, 21 Feb 2024 17:28:38 +0100 Subject: [PATCH 1/8] accept empty QResources --- src/qtoolkit/core/data_objects.py | 28 +++++++++++++++++++++++++--- src/qtoolkit/io/base.py | 3 ++- src/qtoolkit/io/shell.py | 6 ++++-- tests/core/test_data_objects.py | 29 +++++++++++++++++++++++++++++ 4 files changed, 60 insertions(+), 6 deletions(-) diff --git a/src/qtoolkit/core/data_objects.py b/src/qtoolkit/core/data_objects.py index 124acb7..6a0f440 100644 --- a/src/qtoolkit/core/data_objects.py +++ b/src/qtoolkit/core/data_objects.py @@ -1,7 +1,7 @@ from __future__ import annotations import abc -from dataclasses import dataclass +from dataclasses import dataclass, fields from pathlib import Path from qtoolkit.core.base import QTKEnum, QTKObject @@ -179,14 +179,36 @@ def __post_init__(self): self.process_placement = ProcessPlacement.NO_CONSTRAINTS # type: ignore # due to QTKEnum elif self.nodes and self.processes_per_node and not self.processes: self.process_placement = ProcessPlacement.EVENLY_DISTRIBUTED - else: + elif not self._check_no_values(): msg = ( "When process_placement is None either define only nodes " - "plus processes_per_node or only processes" + "plus processes_per_node or only processes to get a default value. " + "Otherwise all the fields must be empty" ) raise UnsupportedResourcesError(msg) self.scheduler_kwargs = self.scheduler_kwargs or {} + def _check_no_values(self) -> bool: + """ + Check if all the attributes are None or empty + """ + for f in fields(self): + if self.__getattribute__(f.name): + return False + + return True + + def check_empty(self) -> bool: + """ + Check if the QResouces is empty and its content is coherent. + Raises an error if process_placement is None, but some attributes are set. + """ + if self.process_placement is not None: + return False + if not self._check_no_values(): + raise ValueError("process_placement is None, but some values are set") + return True + @classmethod def no_constraints(cls, processes, **kwargs): if "nodes" in kwargs or "processes_per_node" in kwargs: diff --git a/src/qtoolkit/io/base.py b/src/qtoolkit/io/base.py index b4ad46e..3beaf98 100644 --- a/src/qtoolkit/io/base.py +++ b/src/qtoolkit/io/base.py @@ -75,7 +75,8 @@ def generate_header(self, options: dict | QResources | None) -> str: options = options or {} if isinstance(options, QResources): - options = self.check_convert_qresources(options) + if not options.check_empty(): + options = self.check_convert_qresources(options) template = QTemplate(self.header_template) diff --git a/src/qtoolkit/io/shell.py b/src/qtoolkit/io/shell.py index 6c758ff..770febd 100644 --- a/src/qtoolkit/io/shell.py +++ b/src/qtoolkit/io/shell.py @@ -255,9 +255,11 @@ def _convert_qresources(self, resources: QResources) -> dict: """ Converts a QResources instance to a dict that will be used to fill in the header of the submission script. - Not implemented for ShellIO + Only an empty QResources is accepted in ShellIO. """ - raise UnsupportedResourcesError # pragma: no cover + if not resources.check_empty(): + raise UnsupportedResourcesError # pragma: no cover + return {} @property def supported_qresources_keys(self) -> list: diff --git a/tests/core/test_data_objects.py b/tests/core/test_data_objects.py index 08057fa..b36521f 100644 --- a/tests/core/test_data_objects.py +++ b/tests/core/test_data_objects.py @@ -238,6 +238,16 @@ def test_no_process_placement(self): ): QResources(processes=8, processes_per_node=2) + with pytest.raises( + UnsupportedResourcesError, + match=r"When process_placement is None either define only nodes " + r"plus processes_per_node or only processes", + ): + QResources(project="xxx") + + # This is acceptable for empty process placement and no details passed + assert QResources() + @pytest.mark.skipif(monty is None, reason="monty is not installed") def test_msonable(self, test_utils): qr1 = QResources( @@ -534,6 +544,25 @@ def test_get_processes_distribution(self): ) proc_distr = qr.get_processes_distribution() assert proc_distr == ["a", "b", "c"] + qr = QResources( + process_placement=None, + ) + proc_distr = qr.get_processes_distribution() + assert proc_distr == [None, None, None] + + def test_is_empty(self): + qr = QResources() + assert qr.check_empty() + + qr = QResources(process_placement=ProcessPlacement.NO_CONSTRAINTS, processes=10) + assert not qr.check_empty() + + qr = QResources(process_placement=None) + qr.processes = 10 + with pytest.raises( + ValueError, match="process_placement is None, but some values are set" + ): + qr.check_empty() class TestQJobInfo: From 0251840c723223659d054efcbaf41642eca05f54 Mon Sep 17 00:00:00 2001 From: Guido Petretto Date: Wed, 21 Feb 2024 20:43:07 +0100 Subject: [PATCH 2/8] map all slurm states --- src/qtoolkit/core/data_objects.py | 14 ++++++++++++++ src/qtoolkit/io/slurm.py | 24 +++++++++++++++++++++++- 2 files changed, 37 insertions(+), 1 deletion(-) diff --git a/src/qtoolkit/core/data_objects.py b/src/qtoolkit/core/data_objects.py index 6a0f440..c06e4fe 100644 --- a/src/qtoolkit/core/data_objects.py +++ b/src/qtoolkit/core/data_objects.py @@ -58,6 +58,20 @@ class QState(QTKEnum): queue manager (e.g. PBS, SLURM, ...) needs to be defined. + UNDETERMINED: The job status cannot be determined. This is a permanent + issue, not being solvable by asking again for the job state. + QUEUED: The job is queued for being scheduled and executed. + QUEUED HELD: The job has been placed on hold by the system, the + administrator, or the submitting user. + RUNNING: The job is running on an execution host. + SUSPENDED: The job has been suspended by the user, the system or the + administrator. + REQUEUED: The job was re-queued by the DRM system, and is eligible to run. + REQUEUED HELD: The job was re-queued by the DRM system, and is currently + placed on hold by the system, the administrator, or the submitting user. + DONE: The job finished without an error. + FAILED: The job exited abnormally before finishing. + Note that not all these standardized states are available in the actual queue manager implementations. """ diff --git a/src/qtoolkit/io/slurm.py b/src/qtoolkit/io/slurm.py index 8254f2a..08f48d3 100644 --- a/src/qtoolkit/io/slurm.py +++ b/src/qtoolkit/io/slurm.py @@ -88,6 +88,7 @@ class SlurmState(QSubState): + BOOT_FAIL = "BOOT_FAIL", "BF" CANCELLED = "CANCELLED", "CA" COMPLETING = "COMPLETING", "CG" COMPLETED = "COMPLETED", "CD" @@ -97,7 +98,17 @@ class SlurmState(QSubState): NODE_FAIL = "NODE_FAIL", "NF" OUT_OF_MEMORY = "OUT_OF_MEMORY", "OOM" PENDING = "PENDING", "PD" + PREEMPTED = "PREEMPTED", "PR" + RESV_DEL_HOLD = "RESV_DEL_HOLD", "RD" + REQUEUE_FED = "REQUEUE_FED", "RF" + REQUEUE_HOLD = "REQUEUE_HOLD", "RH" + RESIZING = "RESIZING", "RS" + REVOKED = "REVOKED", "RV" RUNNING = "RUNNING", "R" + SIGNALING = "SIGNALING", "SI" + SPECIAL_EXIT = "SPECIAL_EXIT", "SE" + STAGE_OUT = "STAGE_OUT", "SO" + STOPPED = "STOPPED", "ST" SUSPENDED = "SUSPENDED", "S" TIMEOUT = "TIMEOUT", "TO" @@ -108,7 +119,8 @@ def qstate(self) -> QState: _STATUS_MAPPING = { - SlurmState.CANCELLED: QState.SUSPENDED, # Should this be failed ? + SlurmState.BOOT_FAIL: QState.FAILED, + SlurmState.CANCELLED: QState.FAILED, SlurmState.COMPLETING: QState.RUNNING, SlurmState.COMPLETED: QState.DONE, SlurmState.CONFIGURING: QState.QUEUED, @@ -117,7 +129,17 @@ def qstate(self) -> QState: SlurmState.NODE_FAIL: QState.FAILED, SlurmState.OUT_OF_MEMORY: QState.FAILED, SlurmState.PENDING: QState.QUEUED, + SlurmState.PREEMPTED: QState.FAILED, + SlurmState.RESV_DEL_HOLD: QState.QUEUED_HELD, + SlurmState.REQUEUE_FED: QState.QUEUED, + SlurmState.REQUEUE_HOLD: QState.QUEUED, + SlurmState.RESIZING: QState.RUNNING, + SlurmState.REVOKED: QState.FAILED, SlurmState.RUNNING: QState.RUNNING, + SlurmState.SIGNALING: QState.RUNNING, + SlurmState.SPECIAL_EXIT: QState.FAILED, + SlurmState.STAGE_OUT: QState.RUNNING, + SlurmState.STOPPED: QState.RUNNING, SlurmState.SUSPENDED: QState.SUSPENDED, SlurmState.TIMEOUT: QState.FAILED, } From 72827817c8ac9116817654e1da8ac95188aadf38 Mon Sep 17 00:00:00 2001 From: Guido Petretto Date: Thu, 22 Feb 2024 10:32:07 +0100 Subject: [PATCH 3/8] Update src/qtoolkit/core/data_objects.py Co-authored-by: David Waroquiers --- src/qtoolkit/core/data_objects.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qtoolkit/core/data_objects.py b/src/qtoolkit/core/data_objects.py index c06e4fe..527faea 100644 --- a/src/qtoolkit/core/data_objects.py +++ b/src/qtoolkit/core/data_objects.py @@ -197,7 +197,7 @@ def __post_init__(self): msg = ( "When process_placement is None either define only nodes " "plus processes_per_node or only processes to get a default value. " - "Otherwise all the fields must be empty" + "Otherwise all the fields must be empty." ) raise UnsupportedResourcesError(msg) self.scheduler_kwargs = self.scheduler_kwargs or {} From 021bdf41c8138c3b29ccd2d254f623251010a2c9 Mon Sep 17 00:00:00 2001 From: Guido Petretto Date: Thu, 22 Feb 2024 10:45:21 +0100 Subject: [PATCH 4/8] Update src/qtoolkit/core/data_objects.py Co-authored-by: David Waroquiers --- src/qtoolkit/core/data_objects.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qtoolkit/core/data_objects.py b/src/qtoolkit/core/data_objects.py index 527faea..5a42759 100644 --- a/src/qtoolkit/core/data_objects.py +++ b/src/qtoolkit/core/data_objects.py @@ -204,7 +204,7 @@ def __post_init__(self): def _check_no_values(self) -> bool: """ - Check if all the attributes are None or empty + Check if all the attributes are None or empty. """ for f in fields(self): if self.__getattribute__(f.name): From 3432967feae548cec773239eb4fc5c844b9f2580 Mon Sep 17 00:00:00 2001 From: Guido Petretto Date: Thu, 22 Feb 2024 10:45:40 +0100 Subject: [PATCH 5/8] Update tests/core/test_data_objects.py Co-authored-by: David Waroquiers --- tests/core/test_data_objects.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/core/test_data_objects.py b/tests/core/test_data_objects.py index b36521f..99c2196 100644 --- a/tests/core/test_data_objects.py +++ b/tests/core/test_data_objects.py @@ -241,7 +241,8 @@ def test_no_process_placement(self): with pytest.raises( UnsupportedResourcesError, match=r"When process_placement is None either define only nodes " - r"plus processes_per_node or only processes", + r"plus processes_per_node or only processes to get a default value. " + r"Otherwise all the fields must be empty.", ): QResources(project="xxx") From c5e5c7239e06880af0a1ea17761680e70cfb589b Mon Sep 17 00:00:00 2001 From: Guido Petretto Date: Thu, 22 Feb 2024 11:53:46 +0100 Subject: [PATCH 6/8] test for shellio --- src/qtoolkit/io/base.py | 8 ++++---- src/qtoolkit/io/shell.py | 4 +++- tests/io/test_shell.py | 16 +++++++++++++++- 3 files changed, 22 insertions(+), 6 deletions(-) diff --git a/src/qtoolkit/io/base.py b/src/qtoolkit/io/base.py index 3beaf98..b935949 100644 --- a/src/qtoolkit/io/base.py +++ b/src/qtoolkit/io/base.py @@ -172,12 +172,12 @@ def check_convert_qresources(self, resources: QResources) -> dict: Also checks that passed values are declared to be handled by the corresponding subclass. """ - not_none = set() + not_empty = set() for field in fields(resources): - if getattr(resources, field.name) is not None: - not_none.add(field.name) + if getattr(resources, field.name): + not_empty.add(field.name) - unsupported_options = not_none.difference(self.supported_qresources_keys) + unsupported_options = not_empty.difference(self.supported_qresources_keys) if unsupported_options: msg = f"Keys not supported: {', '.join(sorted(unsupported_options))}" diff --git a/src/qtoolkit/io/shell.py b/src/qtoolkit/io/shell.py index 770febd..e045384 100644 --- a/src/qtoolkit/io/shell.py +++ b/src/qtoolkit/io/shell.py @@ -258,7 +258,9 @@ def _convert_qresources(self, resources: QResources) -> dict: Only an empty QResources is accepted in ShellIO. """ if not resources.check_empty(): - raise UnsupportedResourcesError # pragma: no cover + raise UnsupportedResourcesError( + "Only empty QResources is supported" + ) # pragma: no cover return {} @property diff --git a/tests/io/test_shell.py b/tests/io/test_shell.py index 254ef9e..66f56d5 100644 --- a/tests/io/test_shell.py +++ b/tests/io/test_shell.py @@ -204,10 +204,24 @@ def test_check_convert_qresources(self, shell_io): qr = QResources(processes=1) with pytest.raises( UnsupportedResourcesError, - match=r"Keys not supported: process_placement, processes, scheduler_kwargs", + match=r"Keys not supported: process_placement, processes", ): shell_io.check_convert_qresources(qr) + qr = QResources() + assert shell_io.check_convert_qresources(qr) == {} + + def test_convert_qresources(self, shell_io): + qr = QResources(processes=1) + with pytest.raises( + UnsupportedResourcesError, + match=r"Only empty QResources is supported", + ): + shell_io._convert_qresources(qr) + + qr = QResources() + assert shell_io._convert_qresources(qr) == {} + def test_header(self, shell_io): # check that the required elements are properly handled in header template options = { From 2544441e192d2132e919b251a0cbc7cf96ac3809 Mon Sep 17 00:00:00 2001 From: Guido Petretto Date: Mon, 26 Feb 2024 15:49:21 +0100 Subject: [PATCH 7/8] change slurm conversion --- src/qtoolkit/io/slurm.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qtoolkit/io/slurm.py b/src/qtoolkit/io/slurm.py index 08f48d3..2195219 100644 --- a/src/qtoolkit/io/slurm.py +++ b/src/qtoolkit/io/slurm.py @@ -131,8 +131,8 @@ def qstate(self) -> QState: SlurmState.PENDING: QState.QUEUED, SlurmState.PREEMPTED: QState.FAILED, SlurmState.RESV_DEL_HOLD: QState.QUEUED_HELD, - SlurmState.REQUEUE_FED: QState.QUEUED, - SlurmState.REQUEUE_HOLD: QState.QUEUED, + SlurmState.REQUEUE_FED: QState.REQUEUED, # ambiguous conversion. Could also be QUEUED, + SlurmState.REQUEUE_HOLD: QState.REQUEUED, # QUEUED_HELD or SUSPENDED SlurmState.RESIZING: QState.RUNNING, SlurmState.REVOKED: QState.FAILED, SlurmState.RUNNING: QState.RUNNING, From 3ffe3ac2bc9143631aab75ab4d3092ce8edbbc6f Mon Sep 17 00:00:00 2001 From: Guido Petretto Date: Mon, 26 Feb 2024 15:55:35 +0100 Subject: [PATCH 8/8] fix docstring --- src/qtoolkit/core/data_objects.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/qtoolkit/core/data_objects.py b/src/qtoolkit/core/data_objects.py index 5a42759..9700f31 100644 --- a/src/qtoolkit/core/data_objects.py +++ b/src/qtoolkit/core/data_objects.py @@ -59,16 +59,16 @@ class QState(QTKEnum): defined. UNDETERMINED: The job status cannot be determined. This is a permanent - issue, not being solvable by asking again for the job state. + issue, not being solvable by asking again for the job state. QUEUED: The job is queued for being scheduled and executed. QUEUED HELD: The job has been placed on hold by the system, the - administrator, or the submitting user. + administrator, or the submitting user. RUNNING: The job is running on an execution host. SUSPENDED: The job has been suspended by the user, the system or the - administrator. + administrator. REQUEUED: The job was re-queued by the DRM system, and is eligible to run. REQUEUED HELD: The job was re-queued by the DRM system, and is currently - placed on hold by the system, the administrator, or the submitting user. + placed on hold by the system, the administrator, or the submitting user. DONE: The job finished without an error. FAILED: The job exited abnormally before finishing.