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

Added Gantt chart #1466

Merged
merged 48 commits into from
Mar 8, 2024
Merged

Added Gantt chart #1466

merged 48 commits into from
Mar 8, 2024

Conversation

ChrisPaulBennett
Copy link
Contributor

@ChrisPaulBennett ChrisPaulBennett commented Sep 4, 2023

Partially addresses #181

This is the first iteration of the Gantt chart view.
We wanted to add a few various other features (zooming, scaling of the data and pagination) but I will be going on parental leave earlier than expected. So would like to get some version out there for us to get some much needed feedback.
This also contains some small additions to the box plot.
The view itself is a full working state, just looks a bit squished when viewing a big suite.

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.

@hjoliver

This comment was marked as resolved.

@oliver-sanders oliver-sanders added the data workflows team Work pertaining to the analysis/gantt/etc views label Oct 12, 2023
@ChrisPaulBennett

This comment was marked as resolved.

@ChrisPaulBennett ChrisPaulBennett marked this pull request as ready for review November 28, 2023 15:26
@ChrisPaulBennett
Copy link
Contributor Author

ChrisPaulBennett commented Nov 28, 2023

That was a bit more complicated than I had envisaged. But it is now ready for review.

This branch needs changes to UI server for the query to work properly. This is currently in a pull request
cylc/cylc-uiserver#530

package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
oliver-sanders

This comment was marked as resolved.

src/views/Analysis.vue Outdated Show resolved Hide resolved
src/components/cylc/gantt/GanttChart.vue Show resolved Hide resolved
@MetRonnie

This comment was marked as resolved.

@MetRonnie

This comment was marked as resolved.

@MetRonnie
Copy link
Member

MetRonnie commented Dec 19, 2023

Ooh, also could do with a refresh button I think

src/components/cylc/gantt/GanttChart.vue Outdated Show resolved Hide resolved
src/components/cylc/gantt/GanttChart.vue Outdated Show resolved Hide resolved
src/components/cylc/gantt/GanttChart.vue Outdated Show resolved Hide resolved
src/components/cylc/gantt/GanttChart.vue Outdated Show resolved Hide resolved
src/components/cylc/gantt/GanttChart.vue Outdated Show resolved Hide resolved
src/components/cylc/gantt/GanttChart.vue Outdated Show resolved Hide resolved
Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

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

Nice, looking forward to this one.

Refreshing:

It could definitely do with a refresh button for now. For future work we can subscribe to the live data and have it update automatically which would be pretty cool.

Too many tasks:

We do need to protect this new view against large and or long running workflows to avoid issues. I tried running the OS35 global to see how the gantt view coped, it took a while:

Screenshot from 2024-01-04 14-46-07

But it did eventually load, although the resulting chart wasn't the most usable:

Screenshot from 2024-01-04 14-50-47

But once I used task name filtering to whittle down the number of rows, it worked rather well:

Screenshot from 2024-01-04 14-58-00

I would suggest restricting the number of tasks displayed to a configured value (maybe default ~30) and displaying a message explaining that there are too many tasks to display, please use the task filter to reduce the number. Or alternatively, doing something similar to #1510 and requiring users to manually specify the tasks they're interested in.

Too many cycles:

We have the same issue with long-running workflows where there may have been a large number of cycles. E.G. This workflow has racked up a few cycles:

Screenshot from 2024-01-04 14-58-48

Some workflows run for several months and cycle as often as 5 mins which will probably cause issues. I'm not sure how best to counter against this issue, ideas:

  • Max cycle limit (i.e. show the most recent 30 cycles and display a message if there are more than 30).
  • Get the UI Server to intelligently pick the time window to display and only load earlier / later cycles on request.

Simulation mode:

Cylc supports a simulation mode (e.g. cylc play --mode=simulation).
in which jobs are simulated rather than run. Simulated jobs don't add their timings to the DB which means the gantt view doesn't work with simulation mode. This is ok I think, but we might want to detect this so we can display a message explaining what's going on rather than a blank tab.

Alternatively, we could just get Cylc to stick the timings into the database? Will ask the team.

src/views/Gantt.vue Outdated Show resolved Hide resolved
@ChrisPaulBennett

This comment was marked as resolved.

@oliver-sanders

This comment was marked as resolved.

@ChrisPaulBennett

This comment was marked as resolved.

@oliver-sanders

This comment was marked as resolved.

@MetRonnie
Copy link
Member

We do need to protect this new view against large and or long running workflows to avoid issues.

How about paginating, showing the most recent tasks on the first page?

@ChrisPaulBennett
Copy link
Contributor Author

We do need to protect this new view against large and or long running workflows to avoid issues.

How about paginating, showing the most recent tasks on the first page?

Pagination was originally going to be in this pull request. But it ended up being a fairly involved process so we thought it best to do it separately. (Actually some of the issue's in this pull request were due to the pagination efforts being removed).
As soon as this PR is in I will begin pagination.

src/views/Gantt.vue Outdated Show resolved Hide resolved
src/components/cylc/gantt/filter.js Outdated Show resolved Hide resolved
Copy link
Member

@oliver-sanders oliver-sanders left a comment

Choose a reason for hiding this comment

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

LGTM, tested, task selection is good, platform filters work. Thanks!

Would be great if you could put together a screenshot or GIF for the changes section in cylc-doc, we'll announce every entry in this section one by one.

@MetRonnie, please squash merge.

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.

Spotted an issue with the filtering: ChrisPaulBennett#4

@oliver-sanders oliver-sanders requested a review from MetRonnie March 5, 2024 13:20
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.

I'm happy with this now 👍 I did notice a couple more issues which I don't know if you want to address now or later.

Edit: address later - #1698

@MetRonnie MetRonnie merged commit c130b07 into cylc:master Mar 8, 2024
8 checks passed
MetRonnie added a commit that referenced this pull request Mar 27, 2024
@MetRonnie MetRonnie modified the milestones: 2.4.0, 2.5.0 Mar 27, 2024
@ChrisPaulBennett ChrisPaulBennett deleted the gantt-chart-view branch July 11, 2024 12:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data workflows team Work pertaining to the analysis/gantt/etc views
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants