Skip to content

Commit

Permalink
fix: proper interpretation of standard resources given as strings (e.…
Browse files Browse the repository at this point in the history
…g. runtime as '5m'). Avoid the need to set additional quotes around size or timespan resources. Improved error messages for resource handling. (#2716)

### Description

Eventually, human-readable resource parsing for runtimes from
workflow-profiles ought to be enabled.

### QC

* [ x] The PR contains a test case for the changes, or the changes are
already covered by an existing test case.
* [ ] The documentation (`docs/`) is updated to reflect the changes or
this is not necessary (e.g. if the change does neither modify the
language nor the behavior or functionalities of Snakemake).

---------

Co-authored-by: Johannes Koester <johannes.koester@uni-due.de>
  • Loading branch information
cmeesters and johanneskoester committed Feb 23, 2024
1 parent 2f7d4b5 commit b6636e9
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 20 deletions.
17 changes: 12 additions & 5 deletions snakemake/cli.py
Expand Up @@ -1683,17 +1683,24 @@ def parse_args(argv):
if args.profile:
profiles.append(args.profile)
if workflow_profile:
workflow_profile_stmt = f" and workflow specific profile {workflow_profile}"
workflow_profile_stmt = f" {'and ' if profiles else ''}workflow specific profile {workflow_profile}"
profiles.append(workflow_profile)
else:
workflow_profile_stmt = ""

print(
f"Using profile{'s' if len(profiles) > 1 else ''} "
f"{' and '.join(map(str, profiles))}{workflow_profile_stmt} for setting default command line arguments.",
file=sys.stderr,
fmt_profiles = profiles if not workflow_profile else profiles[:-1]
profile_stmt = (
f"Using profile{'s' if len(profiles) > 1 else ''} {' and '.join(map(str, fmt_profiles))}"
if fmt_profiles
else "Using"
)

if profiles:
print(
f"{profile_stmt}{workflow_profile_stmt} for setting default command line arguments.",
file=sys.stderr,
)

parser = get_argument_parser(profiles=profiles)
args = parser.parse_args(argv)

Expand Down
30 changes: 28 additions & 2 deletions snakemake/resources.py
Expand Up @@ -12,6 +12,7 @@
is_file_not_found_error,
)
from snakemake.common.tbdstring import TBDString
from snakemake.logging import logger


@dataclass
Expand Down Expand Up @@ -537,6 +538,8 @@ def generic_callable(val, threads_arg, **kwargs):
except NameError:
return val
except Exception as e:
if is_humanfriendly_resource(val):
return val
if not is_file_not_found_error(e, kwargs["input"]):
# Missing input files are handled by the caller
raise WorkflowError(
Expand All @@ -545,7 +548,10 @@ def generic_callable(val, threads_arg, **kwargs):
" String arguments may need additional "
"quoting. E.g.: --default-resources "
"\"tmpdir='/home/user/tmp'\" or "
"--set-resources \"compute1:slurm_extra='--nice=100'\".",
"--set-resources \"somerule:someresource='--nice=100'\". "
"This also holds for setting resources inside of a profile, where "
"you might have to enclose them in single and double quotes, "
"i.e. someresource: \"'--nice=100'\".",
e,
)
raise e
Expand Down Expand Up @@ -639,8 +645,28 @@ def infer_resources(name, value, resources: dict):
and not isinstance(value, TBDString)
):
try:
resources["runtime"] = max(int(round(parse_timespan(value) / 60)), 1)
parsed = max(int(round(parse_timespan(value) / 60)), 1)
except InvalidTimespan:
raise WorkflowError(
f"Cannot parse runtime value into minutes for setting runtime resource: {value}"
)
logger.debug(f"Inferred runtime value of {parsed} minutes from {value}")
resources["runtime"] = parsed


def is_humanfriendly_resource(value):
from humanfriendly import parse_size, parse_timespan, InvalidTimespan, InvalidSize

try:
parse_size(value)
return True
except InvalidSize:
pass

try:
parse_timespan(value)
return True
except InvalidTimespan:
pass

return False
28 changes: 15 additions & 13 deletions snakemake/rules.py
Expand Up @@ -992,21 +992,23 @@ def apply(name, res, threads=None):

if value is not None:
resources[name] = value
# Infer standard resources from eventual human readable forms.
infer_resources(name, value, resources)

# infer additional resources
for mb_item, mib_item in (
("mem_mb", "mem_mib"),
("disk_mb", "disk_mib"),
):
if (
name == mb_item
and mib_item not in self.resources.keys()
and isinstance(value, int)
if not isinstance(value, TBDString):
# Infer standard resources from eventual human readable forms.
infer_resources(name, value, resources)

# infer additional resources
for mb_item, mib_item in (
("mem_mb", "mem_mib"),
("disk_mb", "disk_mib"),
):
# infer mem_mib (memory in Mebibytes) as additional resource
resources[mib_item] = mb_to_mib(value)
if (
name == mb_item
and mib_item not in self.resources.keys()
and isinstance(value, int)
):
# infer mem_mib (memory in Mebibytes) as additional resource
resources[mib_item] = mb_to_mib(value)

resources = Resources(fromdict=resources)
return resources
Expand Down
Empty file.
@@ -0,0 +1,12 @@
rule a:
output: "a.out"
#resources:
# runtime="5m"
shell: """
if [ {resources.runtime} -eq 5 ]; then
touch {output}
else
echo "Runtime is not 300 seconds"
exit 1
fi
"""
@@ -0,0 +1,3 @@
set-resources:
a:
runtime: 5m
10 changes: 10 additions & 0 deletions tests/tests.py
Expand Up @@ -1918,6 +1918,16 @@ def test_no_workflow_profile():
)


@skip_on_windows # not platform dependent
def test_runtime_conversion_from_workflow_profile():
test_path = dpath("test_runtime_conversion_from_workflow_profile")
run(
test_path,
snakefile="workflow/Snakefile",
shellcmd=f"snakemake -c1",
)


@skip_on_windows
def test_localrule():
run(dpath("test_localrule"), targets=["1.txt", "2.txt"])
Expand Down

0 comments on commit b6636e9

Please sign in to comment.