From a2273aa25d35b345c47c41009b23aa48b085e05c Mon Sep 17 00:00:00 2001 From: meesters Date: Mon, 1 Dec 2025 10:27:27 +0100 Subject: [PATCH 1/7] fix: validator check for integers was broken. --- snakemake_executor_plugin_slurm/validation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/snakemake_executor_plugin_slurm/validation.py b/snakemake_executor_plugin_slurm/validation.py index 497ad6a..7529c3b 100644 --- a/snakemake_executor_plugin_slurm/validation.py +++ b/snakemake_executor_plugin_slurm/validation.py @@ -132,7 +132,7 @@ def validate_executor_settings(settings, logger=None): if settings.init_seconds_before_status_checks is not None: if ( not isinstance(settings.init_seconds_before_status_checks, int) - or settings.init_seconds_before_status_checks > 0 + or settings.init_seconds_before_status_checks < 0 ): raise WorkflowError( "init-seconds-before-status-checks must be a positive integer." From 47a5d7cab6ba266c66aae026000c13b38bfa4997 Mon Sep 17 00:00:00 2001 From: meesters Date: Mon, 1 Dec 2025 11:13:34 +0100 Subject: [PATCH 2/7] fix: stricter checking --- snakemake_executor_plugin_slurm/validation.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/snakemake_executor_plugin_slurm/validation.py b/snakemake_executor_plugin_slurm/validation.py index 7529c3b..610d8a0 100644 --- a/snakemake_executor_plugin_slurm/validation.py +++ b/snakemake_executor_plugin_slurm/validation.py @@ -132,7 +132,7 @@ def validate_executor_settings(settings, logger=None): if settings.init_seconds_before_status_checks is not None: if ( not isinstance(settings.init_seconds_before_status_checks, int) - or settings.init_seconds_before_status_checks < 0 + or settings.init_seconds_before_status_checks < 1 ): raise WorkflowError( "init-seconds-before-status-checks must be a positive integer." From 77902fbd9377a7174eed0828a38fd2079c4fa69f Mon Sep 17 00:00:00 2001 From: meesters Date: Mon, 8 Dec 2025 12:38:11 +0100 Subject: [PATCH 3/7] feat: partition selection with cluster confinment working --- snakemake_executor_plugin_slurm/__init__.py | 29 +++++++++++++++++-- snakemake_executor_plugin_slurm/partitions.py | 20 ++++++++----- .../submit_string.py | 11 +++++-- 3 files changed, 48 insertions(+), 12 deletions(-) diff --git a/snakemake_executor_plugin_slurm/__init__.py b/snakemake_executor_plugin_slurm/__init__.py index 2abb5ba..a490dec 100644 --- a/snakemake_executor_plugin_slurm/__init__.py +++ b/snakemake_executor_plugin_slurm/__init__.py @@ -868,10 +868,35 @@ def get_partition_arg(self, job: JobExecutorInterface): else raises an error - implicetly. """ partition = None + + # Check if a specific partition is requested if job.resources.get("slurm_partition"): - partition = job.resources.slurm_partition - elif self._partitions: + # But also check if there's a cluster requirement that might override it + job_cluster = ( + job.resources.get("slurm_cluster") + or job.resources.get("cluster") + or job.resources.get("clusters") + ) + + if job_cluster and self._partitions: + # If a cluster is specified, verify the partition exists and matches + # Otherwise, use auto-selection to find a partition for that cluster + partition_obj = next( + (p for p in self._partitions if p.name == job.resources.slurm_partition), + None + ) + if partition_obj and partition_obj.partition_cluster and partition_obj.partition_cluster != job_cluster: + # Partition exists but is for a different cluster - use auto-selection + partition = get_best_partition(self._partitions, job, self.logger) + else: + partition = job.resources.slurm_partition + else: + partition = job.resources.slurm_partition + + # If no partition was selected yet, try auto-selection + if not partition and self._partitions: partition = get_best_partition(self._partitions, job, self.logger) + # we didnt get a partition yet so try fallback. if not partition: if self._fallback_partition is None: diff --git a/snakemake_executor_plugin_slurm/partitions.py b/snakemake_executor_plugin_slurm/partitions.py index 668141f..cb1ce62 100644 --- a/snakemake_executor_plugin_slurm/partitions.py +++ b/snakemake_executor_plugin_slurm/partitions.py @@ -39,12 +39,16 @@ def read_partition_file(partition_file: Path) -> List["Partition"]: raise KeyError("Partition name cannot be empty") # Extract optional cluster name from partition config - cluster = partition_config.pop("cluster", None) + cluster = None + for key in ("slurm_cluster", "cluster", "clusters"): + if key in partition_config: + cluster = partition_config.pop(key) + break out.append( Partition( name=partition_name, - cluster=cluster, + partition_cluster=cluster, limits=PartitionLimits(**partition_config), ) ) @@ -241,7 +245,7 @@ class Partition: name: str limits: PartitionLimits - cluster: Optional[str] = None + partition_cluster: Optional[str] = None def score_job_fit(self, job: JobExecutorInterface) -> Optional[float]: """ @@ -269,14 +273,14 @@ def score_job_fit(self, job: JobExecutorInterface) -> Optional[float]: # Accept multiple possible resource names for cluster specification job_cluster = ( job.resources.get("slurm_cluster") - or job.resources.get("clusters") or job.resources.get("cluster") + or job.resources.get("clusters") ) - if job_cluster is not None: - # Job specifies a cluster - partition must match - if self.cluster is not None and self.cluster != job_cluster: - return None # Partition is for a different cluster + # If either partition or job specifies a cluster, they must match for scoring + if self.partition_cluster is not None or job_cluster is not None: + if self.partition_cluster != job_cluster: + return 0 # Cluster mismatch: score is 0 for resource_key, limit in numerical_resources.items(): job_requirement = job.resources.get(resource_key, 0) diff --git a/snakemake_executor_plugin_slurm/submit_string.py b/snakemake_executor_plugin_slurm/submit_string.py index 0e1498e..0ffeb15 100644 --- a/snakemake_executor_plugin_slurm/submit_string.py +++ b/snakemake_executor_plugin_slurm/submit_string.py @@ -42,8 +42,15 @@ def get_submit_command(job, params): # "- p '{partition_name}'" call += f" {params.partition}" - if job.resources.get("clusters"): - call += f" --clusters {safe_quote(job.resources.clusters)}" + # Add cluster specification if provided + # Check for cluster first (singular), then fall back to clusters (plural) for backwards compatibility + cluster_val = ( + job.resources.get("cluster") + or job.resources.get("clusters") + or job.resources.get("slurm_cluster") + ) + if cluster_val: + call += f" --cluster {safe_quote(cluster_val)}" if job.resources.get("runtime"): call += f" -t {safe_quote(job.resources.runtime)}" From b7cce6d16209dfdff139734be496ac590bacfd3c Mon Sep 17 00:00:00 2001 From: meesters Date: Mon, 8 Dec 2025 12:41:55 +0100 Subject: [PATCH 4/7] fix: linting --- snakemake_executor_plugin_slurm/__init__.py | 22 ++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/snakemake_executor_plugin_slurm/__init__.py b/snakemake_executor_plugin_slurm/__init__.py index a490dec..5481d4c 100644 --- a/snakemake_executor_plugin_slurm/__init__.py +++ b/snakemake_executor_plugin_slurm/__init__.py @@ -868,7 +868,7 @@ def get_partition_arg(self, job: JobExecutorInterface): else raises an error - implicetly. """ partition = None - + # Check if a specific partition is requested if job.resources.get("slurm_partition"): # But also check if there's a cluster requirement that might override it @@ -877,26 +877,34 @@ def get_partition_arg(self, job: JobExecutorInterface): or job.resources.get("cluster") or job.resources.get("clusters") ) - + if job_cluster and self._partitions: # If a cluster is specified, verify the partition exists and matches # Otherwise, use auto-selection to find a partition for that cluster partition_obj = next( - (p for p in self._partitions if p.name == job.resources.slurm_partition), - None + ( + p + for p in self._partitions + if p.name == job.resources.slurm_partition + ), + None, ) - if partition_obj and partition_obj.partition_cluster and partition_obj.partition_cluster != job_cluster: + if ( + partition_obj + and partition_obj.partition_cluster + and partition_obj.partition_cluster != job_cluster + ): # Partition exists but is for a different cluster - use auto-selection partition = get_best_partition(self._partitions, job, self.logger) else: partition = job.resources.slurm_partition else: partition = job.resources.slurm_partition - + # If no partition was selected yet, try auto-selection if not partition and self._partitions: partition = get_best_partition(self._partitions, job, self.logger) - + # we didnt get a partition yet so try fallback. if not partition: if self._fallback_partition is None: From 588f02091bfd312af5a0c9d67f68284bf0140b2f Mon Sep 17 00:00:00 2001 From: meesters Date: Mon, 8 Dec 2025 14:51:54 +0100 Subject: [PATCH 5/7] docs: updated documentation with a hint on cluster, clusters, slurm_cluster equal treating --- docs/further.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/further.md b/docs/further.md index 18545b1..181b403 100644 --- a/docs/further.md +++ b/docs/further.md @@ -70,6 +70,8 @@ The SLURM executor plugin supports automatic partition selection based on job re *Jobs that explicitly specify a `slurm_partition` resource will bypass automatic selection and use the specified partition directly.* +> **Note:** Partition selection supports specifying the target cluster using any of the resource keys `cluster`, `clusters`, or `slurm_cluster` in your workflow profile or the partition configuration file. All three are treated equivalently by the plugin. + ##### Partition Limits Specification To enable automatic partition selection, create a YAML configuration file that defines the available partitions and their resource limits. This file should be structured as follows: From 80007805b18d22ef8205224406905de3ed31beb8 Mon Sep 17 00:00:00 2001 From: meesters Date: Mon, 8 Dec 2025 15:53:56 +0100 Subject: [PATCH 6/7] fix: testing --- snakemake_executor_plugin_slurm/partitions.py | 13 +++-- tests/test_partition_selection.py | 55 +++++++++++++++++-- 2 files changed, 58 insertions(+), 10 deletions(-) diff --git a/snakemake_executor_plugin_slurm/partitions.py b/snakemake_executor_plugin_slurm/partitions.py index cb1ce62..79b9b92 100644 --- a/snakemake_executor_plugin_slurm/partitions.py +++ b/snakemake_executor_plugin_slurm/partitions.py @@ -64,7 +64,7 @@ def get_best_partition( for p in candidate_partitions: score = p.score_job_fit(job) logger.debug(f"Partition '{p.name}' score for job {job.name}: {score}") - if score is not None: + if score is not None and score > 0: scored_partitions.append((p, score)) if scored_partitions: @@ -277,10 +277,15 @@ def score_job_fit(self, job: JobExecutorInterface) -> Optional[float]: or job.resources.get("clusters") ) - # If either partition or job specifies a cluster, they must match for scoring - if self.partition_cluster is not None or job_cluster is not None: + # Enforce strict cluster eligibility: + # - If the job specifies a cluster, only partitions with a matching cluster are eligible + # - If the job does not specify a cluster, only partitions without a cluster are eligible + if job_cluster is not None: if self.partition_cluster != job_cluster: - return 0 # Cluster mismatch: score is 0 + return None # Not eligible + else: + if self.partition_cluster is not None: + return None # Not eligible for resource_key, limit in numerical_resources.items(): job_requirement = job.resources.get(resource_key, 0) diff --git a/tests/test_partition_selection.py b/tests/test_partition_selection.py index e88c21d..8db413c 100644 --- a/tests/test_partition_selection.py +++ b/tests/test_partition_selection.py @@ -591,8 +591,8 @@ def test_cluster_specification_via_slurm_cluster( # Verify cluster field is read correctly # Find partitions by name instead of assuming order partition_map = {p.name: p for p in partitions} - assert partition_map["normal-small"].cluster == "normal" - assert partition_map["deviating-small"].cluster == "deviating" + assert partition_map["normal-small"].partition_cluster == "normal" + assert partition_map["deviating-small"].partition_cluster == "deviating" # Job targeting 'normal' cluster job = mock_job(threads=16, mem_mb=32000, slurm_cluster="normal") @@ -664,10 +664,54 @@ def test_cluster_mismatch_excludes_partitions( finally: temp_path.unlink() + def test_job_with_cluster_does_not_select_partition_without_cluster( + self, multicluster_partition_config, temp_yaml_file, mock_job, mock_logger + ): + """ + Test that jobs with a cluster requirement do not select partitions without a cluster. + """ + config = dict(multicluster_partition_config) + config["partitions"]["no-cluster"] = { + "max_runtime": 360, + "max_threads": 32, + "max_mem_mb": 64000, + } + temp_path = temp_yaml_file(config) + try: + partitions = read_partition_file(temp_path) + job = mock_job(threads=16, mem_mb=32000, slurm_cluster="normal") + selected_partition = get_best_partition(partitions, job, mock_logger) + # Should select a partition with cluster 'normal', not 'no-cluster' + assert selected_partition in ["normal-small", "normal-large"] + finally: + temp_path.unlink() + + def test_job_without_cluster_can_select_partition_without_cluster( + self, multicluster_partition_config, temp_yaml_file, mock_job, mock_logger + ): + """ + Test that jobs without cluster requirement can select partitions without a cluster. + """ + config = dict(multicluster_partition_config) + config["partitions"]["no-cluster"] = { + "max_runtime": 360, + "max_threads": 32, + "max_mem_mb": 64000, + } + temp_path = temp_yaml_file(config) + try: + partitions = read_partition_file(temp_path) + job = mock_job(threads=16, mem_mb=32000) + selected_partition = get_best_partition(partitions, job, mock_logger) + # Should be able to select 'no-cluster' partition + assert selected_partition in ["normal-small", "normal-large", "no-cluster"] + finally: + temp_path.unlink() + def test_job_without_cluster_uses_any_partition( self, multicluster_partition_config, temp_yaml_file, mock_job, mock_logger ): - """Test that jobs without cluster specification can use any partition.""" + """Test that jobs without cluster specification can use any partition without a cluster assignment.""" temp_path = temp_yaml_file(multicluster_partition_config) try: @@ -677,9 +721,8 @@ def test_job_without_cluster_uses_any_partition( job = mock_job(threads=16, mem_mb=32000) selected_partition = get_best_partition(partitions, job, mock_logger) - # Should select a partition (any cluster is acceptable) - assert selected_partition is not None - assert mock_logger.info.call_count >= 1 + # Should return None since all partitions have a cluster assignment + assert selected_partition is None finally: temp_path.unlink() From 4ca2f13d2cb229030ca8f5e959621af77866e1b0 Mon Sep 17 00:00:00 2001 From: meesters Date: Mon, 8 Dec 2025 15:59:10 +0100 Subject: [PATCH 7/7] fix: formatting --- snakemake_executor_plugin_slurm/__init__.py | 3 ++- snakemake_executor_plugin_slurm/partitions.py | 6 ++++-- snakemake_executor_plugin_slurm/submit_string.py | 3 ++- tests/test_partition_selection.py | 11 ++++++++--- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/snakemake_executor_plugin_slurm/__init__.py b/snakemake_executor_plugin_slurm/__init__.py index 5481d4c..0ca00f6 100644 --- a/snakemake_executor_plugin_slurm/__init__.py +++ b/snakemake_executor_plugin_slurm/__init__.py @@ -894,7 +894,8 @@ def get_partition_arg(self, job: JobExecutorInterface): and partition_obj.partition_cluster and partition_obj.partition_cluster != job_cluster ): - # Partition exists but is for a different cluster - use auto-selection + # Partition exists but is for a different cluster: + # use auto-selection partition = get_best_partition(self._partitions, job, self.logger) else: partition = job.resources.slurm_partition diff --git a/snakemake_executor_plugin_slurm/partitions.py b/snakemake_executor_plugin_slurm/partitions.py index 79b9b92..4be329b 100644 --- a/snakemake_executor_plugin_slurm/partitions.py +++ b/snakemake_executor_plugin_slurm/partitions.py @@ -278,8 +278,10 @@ def score_job_fit(self, job: JobExecutorInterface) -> Optional[float]: ) # Enforce strict cluster eligibility: - # - If the job specifies a cluster, only partitions with a matching cluster are eligible - # - If the job does not specify a cluster, only partitions without a cluster are eligible + # - If the job specifies a cluster, only partitions with a matching cluster + # are eligible + # - If the job does not specify a cluster, only partitions without a cluster + # are eligible if job_cluster is not None: if self.partition_cluster != job_cluster: return None # Not eligible diff --git a/snakemake_executor_plugin_slurm/submit_string.py b/snakemake_executor_plugin_slurm/submit_string.py index 0ffeb15..112eb46 100644 --- a/snakemake_executor_plugin_slurm/submit_string.py +++ b/snakemake_executor_plugin_slurm/submit_string.py @@ -43,7 +43,8 @@ def get_submit_command(job, params): call += f" {params.partition}" # Add cluster specification if provided - # Check for cluster first (singular), then fall back to clusters (plural) for backwards compatibility + # Check for cluster first (singular), then fall back to clusters (plural) + # for backwards compatibility cluster_val = ( job.resources.get("cluster") or job.resources.get("clusters") diff --git a/tests/test_partition_selection.py b/tests/test_partition_selection.py index 8db413c..c673bd8 100644 --- a/tests/test_partition_selection.py +++ b/tests/test_partition_selection.py @@ -668,7 +668,8 @@ def test_job_with_cluster_does_not_select_partition_without_cluster( self, multicluster_partition_config, temp_yaml_file, mock_job, mock_logger ): """ - Test that jobs with a cluster requirement do not select partitions without a cluster. + Test that jobs with a cluster requirement do not select partitions + without a cluster. """ config = dict(multicluster_partition_config) config["partitions"]["no-cluster"] = { @@ -690,7 +691,8 @@ def test_job_without_cluster_can_select_partition_without_cluster( self, multicluster_partition_config, temp_yaml_file, mock_job, mock_logger ): """ - Test that jobs without cluster requirement can select partitions without a cluster. + Test that jobs without cluster requirement can select partitions + without a cluster. """ config = dict(multicluster_partition_config) config["partitions"]["no-cluster"] = { @@ -711,7 +713,10 @@ def test_job_without_cluster_can_select_partition_without_cluster( def test_job_without_cluster_uses_any_partition( self, multicluster_partition_config, temp_yaml_file, mock_job, mock_logger ): - """Test that jobs without cluster specification can use any partition without a cluster assignment.""" + """ + Test that jobs without cluster specification can use any partition + without a cluster assignment. + """ temp_path = temp_yaml_file(multicluster_partition_config) try: