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

validate: do not check for circular/cyclic graph if size is above limit #4020

Merged
merged 6 commits into from
Jan 10, 2021

Conversation

MetRonnie
Copy link
Member

@MetRonnie MetRonnie commented Dec 31, 2020

These changes close #3869

Cyclic graph validation can be quite slow and is currently included as part of --strict

This PR (last one of 2020!) introduces an option --check-circular to cylc validate. If the number of tasks is less than the limit (100), checking for circular graph will always happen. If the number is above the limit, the check will be skipped (with a warning) unless --check-circular is used.

Requirements check-list

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • Appropriate tests are included (unit).
  • Appropriate change log entry included.
  • (master branch) I have opened a documentation PR at cylc/cylc-doc/pull/XXXX.
  • No dependency changes.

@MetRonnie MetRonnie added this to the cylc-8.0.0 milestone Dec 31, 2020
@MetRonnie MetRonnie self-assigned this Dec 31, 2020
cylc/flow/config.py Outdated Show resolved Hide resolved
cylc/flow/config.py Outdated Show resolved Hide resolved
@oliver-sanders oliver-sanders modified the milestones: cylc-8.0.0, cylc-8.0a3 Jan 4, 2021
Limit is on number of tasks.

Provide an option "--check-circular" to always perform this check. It
also is always done when "--strict" is used.
@MetRonnie
Copy link
Member Author

  • tests/k/restart/21-task-elapsed.t passes locally for me
  • tests/f/rose-conf/04-opts-set-from-env.t doesn't run locally for me as I haven't installed cylc-rose, but I imagine the problem is not to do with this PR

@dpmatthews dpmatthews removed their request for review January 5, 2021 11:30
Comment on lines 57 to 62
parser.add_option(
"--check-circular",
help="Check for circular dependencies in graphs when the number of "
"tasks is greater than 100. This can be slow when the number of "
"tasks is high.",
action="store_true", default=False, dest="check_circular")
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could also add the abbreviation --cc if that's worth doing?

CHANGES.md Outdated Show resolved Hide resolved
@hjoliver hjoliver merged commit 07d5b6b into cylc:master Jan 10, 2021
@MetRonnie MetRonnie deleted the cyclic-validation branch January 11, 2021 21:45
@hjoliver hjoliver modified the milestones: cylc-8.0a3, cylc-8.0b0 Feb 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

validate: add separate option for cyclic graph detection
3 participants