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

dump: restrict window to n=0 #5600

Merged
merged 1 commit into from
Dec 19, 2023
Merged

Conversation

oliver-sanders
Copy link
Member

@oliver-sanders oliver-sanders commented Jun 23, 2023

  • This restores the old cylc dump behaviour of displaying only the pool contents.

This also fixes a bug where many n=0 tasks were reported as being in the n=1 window.

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).
  • Applied any dependency changes to both setup.cfg (and conda-environment.yml if present).
  • Tests are included (or explain why tests are not needed).
  • CHANGES.md entry included if this is a change that can affect users
  • Cylc-Doc pull request opened if required at cylc/cylc-doc/pull/XXXX.
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

@oliver-sanders oliver-sanders added this to the cylc-8.2.0 milestone Jun 23, 2023
@oliver-sanders oliver-sanders self-assigned this Jun 23, 2023
@hjoliver
Copy link
Member

(Looks good, thanks. )

@oliver-sanders
Copy link
Member Author

Unfortunately, I spotted that the depth field which carries the n-window number for a task in the schema, doesn't appear to be giving the right answer.

I managed to fix it for many cases, but a few remain, especially when pushing out the graph-extent. I've written this up as a new issue here - #5609

So sadly, cylc dump isn't quite the task-pool reporting tool it once was, but we need to get this bug fixed because it will be needed for n-window filtering in the GUI so we aught to get this sorted out soon.

Copy link
Member

@MetRonnie MetRonnie left a comment

Choose a reason for hiding this comment

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

In #5609 there seems to be some confusion about whether "depth" refers to number of graph edges or family inheritance depth, is it the same here?

cylc/flow/data_store_mgr.py Outdated Show resolved Hide resolved
@oliver-sanders oliver-sanders marked this pull request as draft June 30, 2023 12:13
@oliver-sanders
Copy link
Member Author

In #5609 there seems to be some confusion about whether "depth" refers to number of graph edges or family inheritance

Yep, drafting this for now.

@oliver-sanders oliver-sanders added the BLOCKED This can't happen until something else does label Jul 3, 2023
@oliver-sanders
Copy link
Member Author

Blocked by #5609

@oliver-sanders oliver-sanders modified the milestones: cylc-8.2.0, cylc-8.x Jul 3, 2023
@oliver-sanders oliver-sanders removed the BLOCKED This can't happen until something else does label Nov 2, 2023
@oliver-sanders oliver-sanders modified the milestones: cylc-8.x, cylc-8.3.0 Nov 2, 2023
* This restores the old `cylc dump` behaviour of displaying only the
  pool contents.
@oliver-sanders oliver-sanders requested review from wxtim and removed request for hjoliver December 19, 2023 11:16
@MetRonnie MetRonnie merged commit 3320c05 into cylc:master Dec 19, 2023
24 of 27 checks passed
@oliver-sanders oliver-sanders deleted the dump-n-0 branch December 19, 2023 14:00
wxtim added a commit to wxtim/cylc that referenced this pull request Jan 9, 2024
* upstream/master: (350 commits)
  Apply suggestions from code review [skip ci]
  Added missing union type import (cylc#5906)
  set CYLC_DEBUG env var when set-verbosity DEBUG (cylc#5854)
  coverage: exclude report-timings and set 90% as the lower limit
  lint: Warn users that `CYLC_VERSION={{CYLC_VERSION}}` is deprecated (cylc#5890)
  Add a new entry_point for xtriggers (cylc#5831)
  Update copyright year
  Tests: fix cached datetime arithmetic contamination between tests using different calendar modes
  Replace cyclers functional reftests with integration reftests
  Simplify integration reftest
  Replace pre-initial functional reftests with integration reftests
  Integration tests: add a simpler reftest fixture
  auto update syntax files
  id: catch traceback in ID parsing
  actions: downgrade macos runners to macos 11 (cylc#5892)
  Feat.lint obsolete vars (cylc#5879)
  dump: restrict window to n=0 (cylc#5600)
  Add chained offset logic for FCP (cylc#5885)
  Test.example replace a reftest (cylc#5860)
  GH Actions artifact name fixes
  ...
wxtim added a commit to wxtim/cylc that referenced this pull request Jan 11, 2024
* upstream/master: (81 commits)
  Apply suggestions from code review [skip ci]
  Added missing union type import (cylc#5906)
  coverage: exclude report-timings and set 90% as the lower limit
  lint: Warn users that `CYLC_VERSION={{CYLC_VERSION}}` is deprecated (cylc#5890)
  Add a new entry_point for xtriggers (cylc#5831)
  Tests: fix cached datetime arithmetic contamination between tests using different calendar modes
  Replace cyclers functional reftests with integration reftests
  Simplify integration reftest
  Replace pre-initial functional reftests with integration reftests
  Integration tests: add a simpler reftest fixture
  auto update syntax files
  Feat.lint obsolete vars (cylc#5879)
  dump: restrict window to n=0 (cylc#5600)
  Test.example replace a reftest (cylc#5860)
  GH Actions artifact name fixes
  Bump actions/upload-artifact from 3 to 4
  Bump actions/download-artifact from 3 to 4
  tests/i: update tui screenshot
  Update .github/workflows/test_fast.yml
  actions: add non-utc job to fast tests
  ...
wxtim added a commit to wxtim/cylc that referenced this pull request Jan 12, 2024
* upstream/master: (81 commits)
  Apply suggestions from code review [skip ci]
  Added missing union type import (cylc#5906)
  coverage: exclude report-timings and set 90% as the lower limit
  lint: Warn users that `CYLC_VERSION={{CYLC_VERSION}}` is deprecated (cylc#5890)
  Add a new entry_point for xtriggers (cylc#5831)
  Tests: fix cached datetime arithmetic contamination between tests using different calendar modes
  Replace cyclers functional reftests with integration reftests
  Simplify integration reftest
  Replace pre-initial functional reftests with integration reftests
  Integration tests: add a simpler reftest fixture
  auto update syntax files
  Feat.lint obsolete vars (cylc#5879)
  dump: restrict window to n=0 (cylc#5600)
  Test.example replace a reftest (cylc#5860)
  GH Actions artifact name fixes
  Bump actions/upload-artifact from 3 to 4
  Bump actions/download-artifact from 3 to 4
  tests/i: update tui screenshot
  Update .github/workflows/test_fast.yml
  actions: add non-utc job to fast tests
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong :( small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants