Skip to content

Commit

Permalink
fix walltime conversion issue
Browse files Browse the repository at this point in the history
add test for walltime conversion
  • Loading branch information
bgunnar5 committed Apr 3, 2023
1 parent e769bd4 commit 9d4d8fc
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 3 deletions.
1 change: 1 addition & 0 deletions merlin/examples/workflows/slurm/slurm_test.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ description:

batch:
type: slurm
walltime: 10:00:00

env:
variables:
Expand Down
7 changes: 6 additions & 1 deletion merlin/spec/merlinspec.json
Original file line number Diff line number Diff line change
Expand Up @@ -280,7 +280,12 @@
"launch_args": {"type": "string", "minLength": 1},
"worker_launch": {"type": "string", "minLength": 1},
"nodes": {"type": "integer", "minimum": 1},
"walltime": {"type": "string", "pattern": "^(?:(?:([0-9][0-9]|2[0-3]):)?([0-5][0-9]):)?([0-5][0-9])$"}
"walltime": {
"anyOf": [
{"type": "string", "pattern": "^(?:(?:([0-9][0-9]|2[0-3]):)?([0-5][0-9]):)?([0-5][0-9])$"},
{"type": "integer"}
]
}
}
}
}
16 changes: 15 additions & 1 deletion merlin/spec/specification.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,14 @@
import os
import shlex
from copy import deepcopy
from datetime import timedelta
from io import StringIO

import yaml
from maestrowf.specification import YAMLSpecification

from merlin.spec import all_keys, defaults
from merlin.utils import repr_timedelta


LOG = logging.getLogger(__name__)
Expand Down Expand Up @@ -179,6 +181,18 @@ def load_spec_from_string(cls, string, needs_IO=True, needs_verification=False):
spec.verify()
LOG.debug("Merlin spec verified.")

# Convert the walltime value back to HMS if PyYAML messed with it
for _, section in spec.yaml_sections.items():
# Section is a list for the study block
if isinstance(section, list):
for step in section:
if "walltime" in step and isinstance(step["walltime"], int):
step["walltime"] = repr_timedelta(timedelta(seconds=step["walltime"]))
# Section is a dict for all other blocks
if isinstance(section, dict):
if "walltime" in section and isinstance(section["walltime"], int):
section["walltime"] = repr_timedelta(timedelta(seconds=section["walltime"]))

return spec

@classmethod
Expand Down Expand Up @@ -315,7 +329,7 @@ def verify_batch_block(self, schema):
try:
err_msg = "Walltime must be of the form SS, MM:SS, or HH:MM:SS."
walltime = self.batch["walltime"]
if len(walltime) > 2:
if isinstance(walltime, str) and len(walltime) > 2:
# Walltime must have : if it's not of the form SS
if ":" not in walltime:
raise ValueError(err_msg)
Expand Down
4 changes: 3 additions & 1 deletion tests/integration/test_definitions.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,9 +228,11 @@ def define_tests(): # pylint: disable=R0914,R0915
"conditions": [HasReturnCode(), HasRegex(celery_slurm_regex)],
"run type": "local",
},
# TODO if we ever implement a way to modify yaml files for testing, use that for testing walltime here
"run-workers echo slurm_test": {
"cmds": f"{workers} {slurm} --echo",
"conditions": [HasReturnCode(), HasRegex(celery_slurm_regex)],
# Making sure walltime isn't set to an integer w/ last two conditions here
"conditions": [HasReturnCode(), HasRegex(celery_slurm_regex), HasRegex(r"36000", negate=True), HasRegex(r"10:00:00")],
"run type": "local",
},
"run-workers echo lsf_test": {
Expand Down

0 comments on commit 9d4d8fc

Please sign in to comment.