Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bad recurrence exclusion warning #5659

Closed
hjoliver opened this issue Jul 31, 2023 · 7 comments · Fixed by #5664
Closed

Bad recurrence exclusion warning #5659

hjoliver opened this issue Jul 31, 2023 · 7 comments · Fixed by #5664
Assignees
Labels
could be better Not exactly a bug, but not ideal.
Milestone

Comments

@hjoliver
Copy link
Member

hjoliver commented Jul 31, 2023

See https://cylc.discourse.group/t/cannot-have-more-than-1-repetition-for-zero-duration-recurrence/705/13

[scheduler]
    cycle point format = %Y
[scheduling]
    initial cycle point = 3000
    [[graph]]
        P1Y ! ^ = "foo[-P1Y] => foo"
[runtime]
    [[foo]]

Validation:

$ cylc validate bug
WARNING - Cannot have more than 1 repetition for zero-duration recurrence 3000.
Valid for cylc-8.3.0.dev

The warning refers to the exclusion point ^.

@hjoliver hjoliver added the bug Something is wrong :( label Jul 31, 2023
@hjoliver hjoliver added this to the cylc-8.x milestone Jul 31, 2023
@oliver-sanders
Copy link
Member

oliver-sanders commented Jul 31, 2023

@hjoliver, is the issue that the error message is wrong (i.e. "Must have more than 1 repetition..."), or would we rather let Cylc silently ignore the null-recurrence?

@hjoliver
Copy link
Member Author

hjoliver commented Aug 1, 2023

Maybe I've misunderstood how exclusions are supposed to work, but it seems to me that this:

P1Y !^ = ...

should represent a single-point (^) exclusion - not a broken recurrence exclusion - in which case there should not be any warning message. ??

@oliver-sanders
Copy link
Member

I think I thought that P1Y was R1 for some reason.

Yes, P1Y!^ should work, T00!^ is a documented example, possibly a more recent breakage?

@oliver-sanders
Copy link
Member

oliver-sanders commented Aug 2, 2023

This warning is coming from parsing the exclusion itself, you'll get the same warning if you do:

[scheduling]
  [[graph]]
    ^ = foo

Because "^" gets replaced with "3000" and "3000" is not a valid expression.

The correct way of writing it is like this:

[scheduling]
  [[graph]]
    R1/^ = foo

And for the exclusion example:

[scheduling]
  [[graph]]
    P1Y ! R1/^ = foo

@oliver-sanders oliver-sanders self-assigned this Aug 2, 2023
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Aug 2, 2023
@oliver-sanders oliver-sanders added could be better Not exactly a bug, but not ideal. and removed bug Something is wrong :( labels Aug 2, 2023
@oliver-sanders
Copy link
Member

When we parse an exclusion, we first attempt to parse it as a sequence, then fall back to parsing it as a point.

It's this first attempt at parsing which causes the zero-duration warning. I've raised a PR to suppress this logging for exclusion parsing.

@oliver-sanders oliver-sanders modified the milestones: cylc-8.x, cylc-8.2.1 Aug 2, 2023
oliver-sanders added a commit to oliver-sanders/cylc-flow that referenced this issue Aug 2, 2023
@hjoliver
Copy link
Member Author

hjoliver commented Aug 7, 2023

The correct way of writing it is like this: ... And for the exclusion example:

[scheduling]
  [[graph]]
    P1Y ! R1/^ = foo

According to our documentation that is correct but not necessary, because single point exclusions are also supposed to be correct.

... [However, going by your last comment and new PR, you've come to the same conclusion yourself 👍 ]

@oliver-sanders
Copy link
Member

However, going by your last comment and new PR, you've come to the same conclusion yourself

...and being the author of that functionality...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
could be better Not exactly a bug, but not ideal.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants