cylc / cylc-flow

Cylc: a workflow engine for cycling systems.
https://cylc.github.io
GNU General Public License v3.0
334 stars 94 forks source link

Chain offset expression with time units does not work #5047

Closed MetRonnie closed 11 months ago

MetRonnie commented 2 years ago

Describe the bug

initial cycle point = 2000
final cycle point = +PT6H+PT1S
ERROR - could not convert string to float: '6H+PT1'
    Traceback (most recent call last):
      File "~/github/cylc-flow/cylc/flow/cycling/iso8601.py", line 868, in interval_parse
        return _interval_parse(interval_string)
      File "~/github/cylc-flow/cylc/flow/cycling/iso8601.py", line 889, in _interval_parse
        return WorkflowSpecifics.interval_parser.parse(interval_string)
      File "~/github/isodatetime/metomi/isodatetime/parsers.py", line 605, in parse
        raise ISO8601SyntaxError("duration", expression)
    metomi.isodatetime.exceptions.ISO8601SyntaxError: Invalid ISO 8601 duration representation: +PT6H+PT1S
    During handling of the above exception, another exception occurred:
    Traceback (most recent call last):
      File "~/github/cylc-flow/cylc/flow/cycling/iso8601.py", line 871, in interval_parse
        return -1 * _interval_parse(interval_string.replace("-", "", 1))
      File "~/github/cylc-flow/cylc/flow/cycling/iso8601.py", line 889, in _interval_parse
        return WorkflowSpecifics.interval_parser.parse(interval_string)
      File "~/github/isodatetime/metomi/isodatetime/parsers.py", line 605, in parse
        raise ISO8601SyntaxError("duration", expression)
    metomi.isodatetime.exceptions.ISO8601SyntaxError: Invalid ISO 8601 duration representation: +PT6H+PT1S
    During handling of the above exception, another exception occurred:
    Traceback (most recent call last):
      File "~/github/cylc-flow/cylc/flow/scheduler.py", line 645, in start
        await self.configure()
      File "~/github/cylc-flow/cylc/flow/scheduler.py", line 396, in configure
        self.load_flow_file()
      File "~/github/cylc-flow/cylc/flow/scheduler.py", line 1085, in load_flow_file
        self.config = WorkflowConfig(
      File "~/github/cylc-flow/cylc/flow/config.py", line 381, in __init__
        self.process_final_cycle_point()
      File "~/github/cylc-flow/cylc/flow/config.py", line 714, in process_final_cycle_point
        self.final_point = get_point_relative(
      File "~/github/cylc-flow/cylc/flow/cycling/loader.py", line 99, in get_point_relative
        return POINT_RELATIVE_GETTERS[cycling_type](*args, **kwargs)
      File "~/github/cylc-flow/cylc/flow/cycling/iso8601.py", line 855, in get_point_relative
        interval = ISO8601Interval(str(interval_parse(offset_string)))
      File "~/github/cylc-flow/cylc/flow/cycling/iso8601.py", line 873, in interval_parse
        return _interval_parse(interval_string.replace("+", "", 1))
      File "~/github/cylc-flow/cylc/flow/cycling/iso8601.py", line 889, in _interval_parse
        return WorkflowSpecifics.interval_parser.parse(interval_string)
      File "~/github/isodatetime/metomi/isodatetime/parsers.py", line 575, in parse
        value = float(value)
    ValueError: could not convert string to float: '6H+PT1'
CRITICAL - Workflow shutting down - could not convert string to float: '6H+PT1'

It is treating everything between the first PT and the final S as the number of seconds.

Release version(s) and/or repository branch(es) affected? 8.0.0

Pull requests welcome! This is an Open Source project - please consider contributing a bug fix yourself (please read CONTRIBUTING.md before starting any work though).

MetRonnie commented 2 years ago

Not such a crucial bug as you can do PT1H1S instead, hence tagged against 8.x

wxtim commented 1 year ago

Interestingly now had a userbitten by this because the offset is created by a fairly complex macro.

{% macro format_final(run_periods, fmt='PT1S') -%}
    {%- set ENDYR = '+P%dY'%(run_periods[0]) if run_periods[0] > 0 else '' -%}
    {%- set ENDMO = '+P%dM'%(run_periods[1]) if run_periods[1] > 0 else '' -%}
    {%- set ENDDA = '+P%dD'%(run_periods[2]) if run_periods[2] > 0 else '' -%}
    {%- set ENDHR = '+PT%dH'%(run_periods[3]) if run_periods[3] > 0 else '' -%}
    {%- set ENDMI = '+PT%dM'%(run_periods[4]) if run_periods[4] > 0 else '' -%}
    {%- set ENDSE = '+PT%dS'%(run_periods[5]) if run_periods[5] > 0 else '' -%}
    {{ENDYR}}{{ENDMO}}{{ENDDA}}{{ENDHR}}{{ENDMI}}{{ENDSE}}-{{fmt}}
{%- endmacro %}

Interim suggested fix was:

-    {{ENDYR}}{{ENDMO}}{{ENDDA}}{{ENDHR}}{{ENDMI}}{{ENDSE}}-{{fmt}}
+    {%- set END = ENDYR + ENDMO + ENDDA + ENDHR + ENDMI + ENDSE -%}
+    PT{{END[1:] | duration_as('s') | int - fmt | duration_as('s') | int }}S