diff --git a/docs/further.md b/docs/further.md index 3a0091a..18545b1 100644 --- a/docs/further.md +++ b/docs/further.md @@ -87,14 +87,14 @@ The following limits can be defined for each partition: | Parameter | Type | Description | Default | | ----------------------- | --------- | ---------------------------------- | --------- | -| `max_runtime` | int | Maximum walltime in minutes | unlimited | +| `max_runtime` | int/str | Maximum walltime | unlimited | | `max_mem_mb` | int | Maximum total memory in MB | unlimited | | `max_mem_mb_per_cpu` | int | Maximum memory per CPU in MB | unlimited | | `max_cpus_per_task` | int | Maximum CPUs per task | unlimited | | `max_nodes` | int | Maximum number of nodes | unlimited | | `max_tasks` | int | Maximum number of tasks | unlimited | | `max_tasks_per_node` | int | Maximum tasks per node | unlimited | -| `max_threads` | int | Maximum threads per node | unlimited | +| `max_threads` | int | Maximum threads per node | unlimited | | `max_gpu` | int | Maximum number of GPUs | 0 | | `available_gpu_models` | list[str] | List of available GPU models | none | | `max_cpus_per_gpu` | int | Maximum CPUs per GPU | unlimited | @@ -103,6 +103,19 @@ The following limits can be defined for each partition: | `available_constraints` | list[str] | List of available node constraints | none | | `cluster` | str | Cluster name in multi-cluster setup | none | +Note: the `max_runtime` definition may contain + - Numeric values (assumed to be in minutes): 120, 120.5 + - Snakemake-style time strings: "6d", "12h", "30m", "90s", "2d12h30m" + - SLURM time formats: + - "minutes" (e.g., "60") + - "minutes:seconds" (interpreted as hours:minutes, e.g., "60:30") + - "hours:minutes:seconds" (e.g., "1:30:45") + - "days-hours" (e.g., "2-12") + - "days-hours:minutes" (e.g., "2-12:30") + - "days-hours:minutes:seconds" (e.g., "2-12:30:45") + +They are all auto-converted to minutes. Seconds are rounded to the nearest value in minutes. + ##### Example Partition Configuration ```yaml diff --git a/snakemake_executor_plugin_slurm/partitions.py b/snakemake_executor_plugin_slurm/partitions.py index 43a45f0..f77fca8 100644 --- a/snakemake_executor_plugin_slurm/partitions.py +++ b/snakemake_executor_plugin_slurm/partitions.py @@ -8,6 +8,7 @@ JobExecutorInterface, ) from snakemake_interface_executor_plugins.logging import LoggerExecutorInterface +from .utils import parse_time_to_minutes def read_partition_file(partition_file: Path) -> List["Partition"]: @@ -222,6 +223,15 @@ class PartitionLimits: # Node features/constraints available_constraints: Optional[List[str]] = None + def __post_init__(self): + """Convert max_runtime to minutes if specified as a time string""" + # Check if max_runtime is a string or needs conversion + # isinf() only works on numeric types, so check type first + if isinstance(self.max_runtime, str) or ( + isinstance(self.max_runtime, (int, float)) and not isinf(self.max_runtime) + ): + self.max_runtime = parse_time_to_minutes(self.max_runtime) + @dataclass class Partition: diff --git a/snakemake_executor_plugin_slurm/utils.py b/snakemake_executor_plugin_slurm/utils.py index e7e9300..f833a89 100644 --- a/snakemake_executor_plugin_slurm/utils.py +++ b/snakemake_executor_plugin_slurm/utils.py @@ -1,8 +1,10 @@ # utility functions for the SLURM executor plugin +import math import os import re from pathlib import Path +from typing import Union from snakemake_interface_executor_plugins.jobs import ( JobExecutorInterface, @@ -10,6 +12,125 @@ from snakemake_interface_common.exceptions import WorkflowError +def round_half_up(n): + return int(math.floor(n + 0.5)) + + +def parse_time_to_minutes(time_value: Union[str, int, float]) -> int: + """ + Convert a time specification to minutes (integer). This function + is intended to handle the partition definitions for the max_runtime + value in a partition config file. + + Supports: + - Numeric values (assumed to be in minutes): 120, 120.5 + - Snakemake-style time strings: "6d", "12h", "30m", "90s", "2d12h30m" + - SLURM time formats: + - "minutes" (e.g., "60") + - "minutes:seconds" (interpreted as hours:minutes, e.g., "60:30") + - "hours:minutes:seconds" (e.g., "1:30:45") + - "days-hours" (e.g., "2-12") + - "days-hours:minutes" (e.g., "2-12:30") + - "days-hours:minutes:seconds" (e.g., "2-12:30:45") + + Args: + time_value: Time specification as string, int, or float + + Returns: + Time in minutes as integer (fractional minutes are rounded) + + Raises: + WorkflowError: If the time format is invalid + """ + # If already numeric, return as integer minutes (rounded) + if isinstance(time_value, (int, float)): + return round_half_up(time_value) # implicit conversion to int + + # Convert to string and strip whitespace + time_str = str(time_value).strip() + + # Try to parse as plain number first + try: + return round_half_up(float(time_str)) # implicit conversion to int + except ValueError: + pass + + # Try SLURM time formats first (with colons and dashes) + # Format: days-hours:minutes:seconds or variations + if "-" in time_str or ":" in time_str: + try: + days = 0 + hours = 0 + minutes = 0 + seconds = 0 + + # Split by dash first (days separator) + if "-" in time_str: + parts = time_str.split("-") + if len(parts) != 2: + raise ValueError("Invalid format with dash") + days = int(parts[0]) + time_str = parts[1] + + # Split by colon (time separator) + time_parts = time_str.split(":") + + if len(time_parts) == 1: + # Just hours (after dash) or just minutes + if days > 0: + hours = int(time_parts[0]) + else: + minutes = int(time_parts[0]) + elif len(time_parts) == 2: + # was: days-hours:minutes + hours = int(time_parts[0]) + minutes = int(time_parts[1]) + elif len(time_parts) == 3: + # was: hours:minutes:seconds + hours = int(time_parts[0]) + minutes = int(time_parts[1]) + seconds = int(time_parts[2]) + else: + raise ValueError("Too many colons in time format") + + # Convert everything to minutes + total_minutes = days * 24 * 60 + hours * 60 + minutes + seconds / 60.0 + return round_half_up(total_minutes) # implicit conversion to int + + except (ValueError, IndexError): + # If SLURM format parsing fails, try Snakemake style below + pass + + # Parse Snakemake-style time strings (e.g., "6d", "12h", "30m", "90s", "2d12h30m") + # Pattern matches: optional number followed by unit (d, h, m, s) + pattern = r"(\d+(?:\.\d+)?)\s*([dhms])" + matches = re.findall(pattern, time_str.lower()) + + if not matches: + raise WorkflowError( + f"Invalid time format: '{time_value}'. " + f"Expected formats:\n" + f" - Numeric value in minutes: 120\n" + f" - Snakemake style: '6d', '12h', '30m', '90s', '2d12h30m'\n" + f" - SLURM style: 'minutes', 'minutes:seconds', 'hours:minutes:seconds',\n" + f" 'days-hours', 'days-hours:minutes', 'days-hours:minutes:seconds'" + ) + + total_minutes = 0.0 + for value, unit in matches: + num = float(value) + if unit == "d": + total_minutes += num * 24 * 60 + elif unit == "h": + total_minutes += num * 60 + elif unit == "m": + total_minutes += num + elif unit == "s": + total_minutes += num / 60 + + return round_half_up(total_minutes) + + def delete_slurm_environment(): """ Function to delete all environment variables diff --git a/tests/test_time_conversion.py b/tests/test_time_conversion.py new file mode 100644 index 0000000..c49413f --- /dev/null +++ b/tests/test_time_conversion.py @@ -0,0 +1,374 @@ +import pytest +import tempfile +import yaml +from pathlib import Path +from math import inf + +from snakemake_interface_common.exceptions import WorkflowError +from snakemake_executor_plugin_slurm.utils import parse_time_to_minutes +from snakemake_executor_plugin_slurm.partitions import ( + PartitionLimits, + read_partition_file, +) + + +class TestTimeConversion: + """Test time string conversion to minutes""" + + def test_parse_time_numeric_int(self): + """Test parsing of plain integer (minutes)""" + assert parse_time_to_minutes(120) == 120 + assert parse_time_to_minutes(0) == 0 + assert parse_time_to_minutes(1440) == 1440 + + def test_parse_time_numeric_float(self): + """Test parsing of plain float (minutes) - rounds to integer""" + assert parse_time_to_minutes(120.5) == 121 # Rounded to nearest minute + assert parse_time_to_minutes(0.5) == 1 # Rounded to 1 minute + assert parse_time_to_minutes(120.4) == 120 # Rounded down + + def test_parse_time_numeric_string(self): + """Test parsing of numeric strings (minutes)""" + assert parse_time_to_minutes("120") == 120 + assert parse_time_to_minutes("120.5") == 121 # Rounded to nearest minute + assert parse_time_to_minutes("0") == 0 + + def test_parse_time_days(self): + """Test parsing of day notation""" + assert parse_time_to_minutes("1d") == 1440 # 1 day = 1440 minutes + assert parse_time_to_minutes("6d") == 8640 # 6 days = 8640 minutes + assert parse_time_to_minutes("0.5d") == 720 # 0.5 days = 720 minutes + + def test_parse_time_hours(self): + """Test parsing of hour notation""" + assert parse_time_to_minutes("1h") == 60 + assert parse_time_to_minutes("12h") == 720 + assert parse_time_to_minutes("0.5h") == 30 + + def test_parse_time_minutes(self): + """Test parsing of minute notation""" + assert parse_time_to_minutes("30m") == 30 + assert parse_time_to_minutes("90m") == 90 + assert parse_time_to_minutes("1m") == 1 + + def test_parse_time_seconds(self): + """Test parsing of second notation""" + assert parse_time_to_minutes("60s") == 1 # 60 seconds = 1 minute + assert ( + parse_time_to_minutes("90s") == 2 + ) # 90 seconds = 1.5 minutes, rounded to 2 + assert ( + parse_time_to_minutes("30s") == 1 + ) # 30 seconds = 0.5 minutes, rounded to 1 + + def test_parse_time_combined(self): + """Test parsing of combined time notations""" + # 2 days + 12 hours + 30 minutes = 2880 + 720 + 30 = 3630 minutes + assert parse_time_to_minutes("2d12h30m") == 3630 + # 1 day + 30 minutes = 1440 + 30 = 1470 minutes + assert parse_time_to_minutes("1d30m") == 1470 + # 6 hours + 30 seconds = 360 + 0.5 = 360.5 minutes, rounded to 361 + assert parse_time_to_minutes("6h30s") == 361 + + def test_parse_time_whitespace(self): + """Test parsing with whitespace""" + assert parse_time_to_minutes(" 6d ") == 8640 + assert parse_time_to_minutes("2d 12h 30m") == 3630 + + def test_parse_time_case_insensitive(self): + """Test that parsing is case insensitive""" + assert parse_time_to_minutes("6D") == 8640 + assert parse_time_to_minutes("12H") == 720 + assert parse_time_to_minutes("30M") == 30 + assert parse_time_to_minutes("90S") == 2 # Rounded + + def test_parse_time_invalid_format(self): + """Test that invalid formats raise WorkflowError""" + with pytest.raises(WorkflowError, match="Invalid time format"): + parse_time_to_minutes("invalid") + + with pytest.raises(WorkflowError, match="Invalid time format"): + parse_time_to_minutes("6x") + + with pytest.raises(WorkflowError, match="Invalid time format"): + parse_time_to_minutes("abc123") + + def test_parse_time_slurm_minutes_only(self): + """Test SLURM format: minutes only""" + assert parse_time_to_minutes("60") == 60 + assert parse_time_to_minutes("120") == 120 + assert parse_time_to_minutes("1440") == 1440 + + def test_parse_time_slurm_minutes_seconds(self): + """Test SLURM format: minutes:seconds + Note: In SLURM, MM:SS format is ambiguous. For partition time limits, + it's more commonly interpreted as hours:minutes (HH:MM) since limits + are typically longer durations. MM:SS is mainly used for elapsed time output. + """ + # Interpreted as hours:minutes for partition limits + assert parse_time_to_minutes("60:30") == 3630 # 60 hours + 30 minutes + assert parse_time_to_minutes("1:30") == 90 # 1 hour + 30 minutes + assert parse_time_to_minutes("0:45") == 45 # 0 hours + 45 minutes + + def test_parse_time_slurm_hours_minutes_seconds(self): + """Test SLURM format: hours:minutes:seconds""" + assert parse_time_to_minutes("1:30:00") == 90 # 1 hour 30 minutes + assert ( + parse_time_to_minutes("2:15:30") == 136 + ) # 2h 15m 30s = 135.5 min, rounded to 136 + assert parse_time_to_minutes("12:00:00") == 720 # 12 hours + assert ( + parse_time_to_minutes("0:45:30") == 46 + ) # 45 minutes 30 seconds = 45.5, rounded to 46 + + def test_parse_time_slurm_days_hours(self): + """Test SLURM format: days-hours""" + assert parse_time_to_minutes("1-0") == 1440 # 1 day + assert parse_time_to_minutes("2-12") == 3600 # 2 days + 12 hours + assert parse_time_to_minutes("6-0") == 8640 # 6 days + + def test_parse_time_slurm_days_hours_minutes(self): + """Test SLURM format: days-hours:minutes""" + assert parse_time_to_minutes("1-0:30") == 1470 # 1 day + 30 minutes + assert ( + parse_time_to_minutes("2-12:30") == 3630 + ) # 2 days + 12 hours + 30 minutes + assert parse_time_to_minutes("0-6:15") == 375 # 6 hours + 15 minutes + + def test_parse_time_slurm_days_hours_minutes_seconds(self): + """Test SLURM format: days-hours:minutes:seconds""" + assert parse_time_to_minutes("1-0:0:0") == 1440 # 1 day exactly + assert ( + parse_time_to_minutes("2-12:30:45") == 3631 + ) # 2d 12h 30m 45s = 3630.75, rounded to 3631 + assert ( + parse_time_to_minutes("0-1:30:30") == 91 + ) # 1h 30m 30s = 90.5, rounded to 91 + # 7 days + 23 hours + 59 minutes + 59 seconds + # = 7*24*60 + 23*60 + 59 + 59/60 = 10080 + 1380 + 59 + 0.98333 = 11519.98333, + # rounded to 11520 + assert parse_time_to_minutes("7-23:59:59") == 11520 + + +class TestPartitionLimitsTimeConversion: + """Test that PartitionLimits correctly converts max_runtime""" + + def test_partition_limits_numeric_runtime(self): + """Test PartitionLimits with numeric runtime""" + limits = PartitionLimits(max_runtime=1440) + assert limits.max_runtime == 1440 + + def test_partition_limits_string_runtime(self): + """Test PartitionLimits with string time format""" + limits = PartitionLimits(max_runtime="6d") + assert limits.max_runtime == 8640 + + def test_partition_limits_combined_runtime(self): + """Test PartitionLimits with combined time format""" + limits = PartitionLimits(max_runtime="2d12h") + assert limits.max_runtime == 3600 # 2880 + 720 = 3600 + + def test_partition_limits_infinity_runtime(self): + """Test PartitionLimits with infinite runtime (default)""" + limits = PartitionLimits() + assert limits.max_runtime == inf + + def test_partition_limits_infinity_unchanged(self): + """Test that infinity runtime is not converted""" + limits = PartitionLimits(max_runtime=inf) + assert limits.max_runtime == inf + + +class TestPartitionFileTimeConversion: + """Test that partition files correctly parse time formats""" + + def test_partition_file_with_time_strings(self): + """Test reading partition file with time string formats""" + config = { + "partitions": { + "short": { + "max_runtime": "12h", + "max_mem_mb": 64000, + }, + "medium": { + "max_runtime": "2d", + "max_mem_mb": 128000, + }, + "long": { + "max_runtime": "6d", + "max_mem_mb": 256000, + }, + } + } + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f: + yaml.dump(config, f) + temp_path = Path(f.name) + + try: + partitions = read_partition_file(temp_path) + + # Check that partitions were created + assert len(partitions) == 3 + + # Check that time strings were converted correctly + short_partition = next(p for p in partitions if p.name == "short") + assert short_partition.limits.max_runtime == 720 # 12h = 720 minutes + + medium_partition = next(p for p in partitions if p.name == "medium") + assert medium_partition.limits.max_runtime == 2880 # 2d = 2880 minutes + + long_partition = next(p for p in partitions if p.name == "long") + assert long_partition.limits.max_runtime == 8640 # 6d = 8640 minutes + finally: + temp_path.unlink() + + def test_partition_file_with_numeric_runtime(self): + """Test that numeric runtime still works""" + config = { + "partitions": { + "numeric": { + "max_runtime": 1440, + "max_mem_mb": 64000, + }, + } + } + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f: + yaml.dump(config, f) + temp_path = Path(f.name) + + try: + partitions = read_partition_file(temp_path) + assert len(partitions) == 1 + assert partitions[0].limits.max_runtime == 1440 + finally: + temp_path.unlink() + + def test_partition_file_with_combined_time(self): + """Test partition file with combined time format""" + config = { + "partitions": { + "combined": { + "max_runtime": "2d12h30m", + "max_mem_mb": 128000, + }, + } + } + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f: + yaml.dump(config, f) + temp_path = Path(f.name) + + try: + partitions = read_partition_file(temp_path) + assert len(partitions) == 1 + # 2d = 2880, 12h = 720, 30m = 30 → total = 3630 minutes + assert partitions[0].limits.max_runtime == 3630 + finally: + temp_path.unlink() + + def test_partition_file_with_invalid_time(self): + """Test that invalid time format in partition file raises error""" + config = { + "partitions": { + "invalid": { + "max_runtime": "invalid_time", + "max_mem_mb": 64000, + }, + } + } + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f: + yaml.dump(config, f) + temp_path = Path(f.name) + + try: + with pytest.raises(WorkflowError, match="Invalid time format"): + read_partition_file(temp_path) + finally: + temp_path.unlink() + + def test_partition_file_with_slurm_time_formats(self): + """Test reading partition file with SLURM time formats""" + config = { + "partitions": { + "short_minutes": { + "max_runtime": "60", # 60 minutes + "max_mem_mb": 32000, + }, + "medium_hms": { + "max_runtime": "12:00:00", # 12 hours in h:m:s + "max_mem_mb": 64000, + }, + "long_dh": { + "max_runtime": "6-0", # 6 days in d-h format + "max_mem_mb": 128000, + }, + "very_long_dhms": { + "max_runtime": "7-12:30:00", # 7 days 12h 30m in d-h:m:s + "max_mem_mb": 256000, + }, + } + } + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f: + yaml.dump(config, f) + temp_path = Path(f.name) + + try: + partitions = read_partition_file(temp_path) + + # Check that partitions were created + assert len(partitions) == 4 + + # Check that SLURM time formats were converted correctly + short_partition = next(p for p in partitions if p.name == "short_minutes") + assert short_partition.limits.max_runtime == 60 + + medium_partition = next(p for p in partitions if p.name == "medium_hms") + assert medium_partition.limits.max_runtime == 720 # 12 hours + + long_partition = next(p for p in partitions if p.name == "long_dh") + assert long_partition.limits.max_runtime == 8640 # 6 days + + very_long_partition = next( + p for p in partitions if p.name == "very_long_dhms" + ) + # 7 days + 12 hours + 30 minutes = 7*24*60 + 12*60 + 30 = 10830 minutes + assert very_long_partition.limits.max_runtime == 10830 + finally: + temp_path.unlink() + + def test_partition_file_mixed_time_formats(self): + """Test partition file with mixed Snakemake and SLURM formats""" + config = { + "partitions": { + "snakemake_style": { + "max_runtime": "6d", + "max_mem_mb": 64000, + }, + "slurm_style": { + "max_runtime": "6-0", + "max_mem_mb": 64000, + }, + "numeric": { + "max_runtime": 8640, + "max_mem_mb": 64000, + }, + } + } + + with tempfile.NamedTemporaryFile(mode="w", suffix=".yaml", delete=False) as f: + yaml.dump(config, f) + temp_path = Path(f.name) + + try: + partitions = read_partition_file(temp_path) + assert len(partitions) == 3 + + # All three should result in the same runtime (6 days = 8640 minutes) + for partition in partitions: + assert partition.limits.max_runtime == 8640 + finally: + temp_path.unlink()