From 7c732f4bbddde09316747865682e0b951d691f4d Mon Sep 17 00:00:00 2001 From: Mark Dawson Date: Fri, 19 Apr 2024 16:07:08 +0100 Subject: [PATCH 1/6] added cyclepoint grouping with subgraphs added toggle button and styling added e2e test use initialOptions prop to save view state fix nodes undefined error added towncrier entry review amends review amends fix broken e2e tests --- changes.d/1763.feat.md | 1 + src/components/cylc/GraphSubgraph.vue | 48 +++++++++++ src/views/Graph.vue | 112 +++++++++++++++++++++++--- tests/e2e/specs/graph.cy.js | 19 +++++ 4 files changed, 167 insertions(+), 13 deletions(-) create mode 100644 changes.d/1763.feat.md create mode 100644 src/components/cylc/GraphSubgraph.vue diff --git a/changes.d/1763.feat.md b/changes.d/1763.feat.md new file mode 100644 index 000000000..0cbbc5d7b --- /dev/null +++ b/changes.d/1763.feat.md @@ -0,0 +1 @@ +Added graph view feature to group nodes by cycle point \ No newline at end of file diff --git a/src/components/cylc/GraphSubgraph.vue b/src/components/cylc/GraphSubgraph.vue new file mode 100644 index 000000000..a8368a475 --- /dev/null +++ b/src/components/cylc/GraphSubgraph.vue @@ -0,0 +1,48 @@ + + + diff --git a/src/views/Graph.vue b/src/views/Graph.vue index adee7a8a4..803499a18 100644 --- a/src/views/Graph.vue +++ b/src/views/Graph.vue @@ -86,6 +86,14 @@ along with this program. If not, see . /> + + + + + @@ -105,6 +113,7 @@ import { import SubscriptionQuery from '@/model/SubscriptionQuery.model' // import CylcTreeCallback from '@/services/treeCallback' import GraphNode from '@/components/cylc/GraphNode.vue' +import GraphSubgraph from '@/components/cylc/GraphSubgraph.vue' import ViewToolbar from '@/components/cylc/ViewToolbar.vue' import { posToPath, @@ -118,7 +127,8 @@ import { mdiArrowCollapse, mdiArrowExpand, mdiRefresh, - mdiFileRotateRight + mdiFileRotateRight, + mdiVectorSelection } from '@mdi/js' // NOTE: Use TaskProxies not nodesEdges{nodes} to list nodes as this is what @@ -219,6 +229,7 @@ export default { components: { GraphNode, + GraphSubgraph, ViewToolbar }, @@ -251,11 +262,19 @@ export default { */ const spacing = useInitialOptions('spacing', { props, emit }, 1.5) + /** + * The group by cycle point toggle state. + * If true the graph nodes will be grouped by cycle point + * @type {import('vue').Ref} + */ + const groupCycle = useInitialOptions('groupCycle', { props, emit }, false) + return { jobTheme: useJobTheme(), transpose, autoRefresh, - spacing + spacing, + groupCycle } }, @@ -268,6 +287,7 @@ export default { // the nodes end edges we render to the graph graphNodes: [], graphEdges: [], + subgraphs: {}, // the svg transformations to apply to each node to apply the layout // generated by graphviz nodeTransformations: {}, @@ -361,6 +381,13 @@ export default { icon: mdiArrowCollapse, action: 'callback', callback: this.decreaseSpacing + }, + { + title: 'Group by cycle point', + icon: mdiVectorSelection, + action: 'toggle', + value: this.groupCycle, + key: 'groupCycle' } ] } @@ -483,7 +510,20 @@ export default { } return ret }, - getDotCode (nodeDimensions, nodes, edges) { + /** + * Get the nodes binned by cycle point + * + * @param {Object[]} nodes + * @returns {{ [dateTime: string]: Object[] nodes }} mapping of node to their cycle point. + */ + getCycles (nodes) { + if (!this.groupCycle) return + return nodes.reduce((x, y) => { + (x[y.node.cyclePoint] ||= []).push(y) + return x + }, {}) + }, + getDotCode (nodeDimensions, nodes, edges, cycles) { // return GraphViz dot code for the given nodes, edges and dimensions const ret = ['digraph {'] let spacing = this.spacing @@ -526,6 +566,27 @@ export default { ] `) } + + if (this.groupCycle) { + // Loop over the subgraphs + Object.keys(cycles).forEach((key, i) => { + // Loop over the nodes that are included in the subraph + const nodeFormattedArray = cycles[key].map(a => `"${a.id}"`) + ret.push(` + subgraph cluster_margin_${i} + { + margin=100.0 + label="margin" + subgraph cluster_${i} {${nodeFormattedArray};\n + label = "${key}";\n + fontsize = "70px" + style=dashed + margin=60.0 + } + }`) + }) + } + if (this.transpose) { // left-right orientation // route edges from anywhere on the node of the source task to anywhere @@ -542,6 +603,8 @@ export default { } } ret.push('}') + console.log("ret.join('\n')") + console.log(ret.join('\n')) return ret.join('\n') }, hashGraph (nodes, edges) { @@ -602,6 +665,8 @@ export default { return } + const cycles = this.getCycles(nodes) + // compute the graph ID const graphID = this.hashGraph(nodes, edges) if (this.graphID === graphID) { @@ -646,7 +711,7 @@ export default { // layout the graph try { - await this.layout(nodes, edges, nodeDimensions) + await this.layout(nodes, edges, nodeDimensions, cycles) } catch (e) { // something went wrong, allow the layout to retry later this.graphID = null @@ -689,26 +754,42 @@ export default { * @param {Object[]} edges * @param {{ [id: string]: SVGRect }} nodeDimensions */ - async layout (nodes, edges, nodeDimensions) { + async layout (nodes, edges, nodeDimensions, cycles) { // generate the GraphViz dot code - const dotCode = this.getDotCode(nodeDimensions, nodes, edges) + const dotCode = this.getDotCode(nodeDimensions, nodes, edges, cycles) // run the layout algorithm const jsonString = (await this.graphviz).layout(dotCode, 'json') const json = JSON.parse(jsonString) + this.subgraphs = {} // update graph node positions for (const obj of json.objects) { - const [x, y] = obj.pos.split(',') - const bbox = nodeDimensions[obj.name] - // translations: - // 1. The graphviz node coordinates - // 2. Centers the node on this coordinate - // TODO convert (2) to maths OR fix it to avoid recomputation? - this.nodeTransformations[obj.name] = ` + if (obj.bb) { + // if the object is a subgraph + const [left, bottom, right, top] = obj.bb.split(',') + this.subgraphs[obj.name] = { + x: left, + y: -top, + width: right - left, + height: top - bottom, + label: obj.label + } + } else { + console.log("obj.name") + console.log(obj.name) + // else the object is a node + const [x, y] = obj.pos.split(',') + const bbox = nodeDimensions[obj.name] + // translations: + // 1. The graphviz node coordinates + // 2. Centers the node on this coordinate + // TODO convert (2) to maths OR fix it to avoid recomputation? + this.nodeTransformations[obj.name] = ` translate(${x}, -${y}) translate(-${bbox.width / 2}, -${bbox.height / 2}) ` + } } // update edge paths this.graphEdges = json.edges?.map(edge => posToPath(edge.pos)) ?? [] @@ -741,6 +822,11 @@ export default { if (!this.autoRefresh) { this.updateTimer() } + }, + groupCycle () { + // refresh the graph when group by cycle point option is changed + this.graphID = null + this.refresh() } } } diff --git a/tests/e2e/specs/graph.cy.js b/tests/e2e/specs/graph.cy.js index c12ee1f57..56b9232dd 100644 --- a/tests/e2e/specs/graph.cy.js +++ b/tests/e2e/specs/graph.cy.js @@ -117,6 +117,18 @@ describe('Graph View', () => { .should('have.length', 7) }) + it('should group by cycle point', () => { + cy.visit('/#/graph/one') + waitForGraphLayout() + cy + .get('[data-cy="control-groupCycle"] > .v-btn') + .click() + cy + .get('.c-graph:first') + .find('.c-graph-subgraph > rect') + .should('be.visible') + }) + it('remembers autorefresh setting when switching between workflows', () => { cy.visit('/#/workspace/one') addView('Graph') @@ -130,4 +142,11 @@ describe('Graph View', () => { waitForGraphLayout() checkRememberToolbarSettings('[data-cy=control-transpose]', 'not.have.class', 'have.class') }) + + it('remembers cycle point grouping setting when switching between workflows', () => { + cy.visit('/#/workspace/one') + addView('Graph') + waitForGraphLayout() + checkRememberToolbarSettings('[data-cy="control-groupCycle"]', 'not.have.class', 'have.class') + }) }) From 2deb24080c4cd42e4b561b29bfc243caee2b4515 Mon Sep 17 00:00:00 2001 From: Mark Dawson Date: Fri, 10 May 2024 15:32:35 +0100 Subject: [PATCH 2/6] code tidy up --- src/views/Graph.vue | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/views/Graph.vue b/src/views/Graph.vue index 803499a18..d35f93e36 100644 --- a/src/views/Graph.vue +++ b/src/views/Graph.vue @@ -603,8 +603,6 @@ export default { } } ret.push('}') - console.log("ret.join('\n')") - console.log(ret.join('\n')) return ret.join('\n') }, hashGraph (nodes, edges) { @@ -776,8 +774,6 @@ export default { label: obj.label } } else { - console.log("obj.name") - console.log(obj.name) // else the object is a node const [x, y] = obj.pos.split(',') const bbox = nodeDimensions[obj.name] From d3de703415aef1f72df9826408d93d3bde1994ca Mon Sep 17 00:00:00 2001 From: Mark Dawson Date: Tue, 14 May 2024 10:47:57 +0100 Subject: [PATCH 3/6] review amends --- src/components/cylc/GraphSubgraph.vue | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/components/cylc/GraphSubgraph.vue b/src/components/cylc/GraphSubgraph.vue index a8368a475..aac449a10 100644 --- a/src/components/cylc/GraphSubgraph.vue +++ b/src/components/cylc/GraphSubgraph.vue @@ -17,9 +17,10 @@ :y="labelYPosition" font-family="Roboto" alignment-baseline="middle" text-anchor="middle" - font-size="70px" + font-size="60px" fill="black" - stroke-width=1.5 + stroke-width=5 + paint-order="stroke" stroke="white" > {{ subgraph.label }} From 8a0b1d6a25081921cd350d921ffdaa3ad5f079d5 Mon Sep 17 00:00:00 2001 From: Ronnie Dutta <61982285+MetRonnie@users.noreply.github.com> Date: Wed, 15 May 2024 15:27:27 +0100 Subject: [PATCH 4/6] Subgraph improvements (#5) * Tidy * Mock data: update subscription data for graph view * Graph view: handle negative coords when converting graphviz edge to SVG --- src/services/mock/json/workflows/multi.json | 4 ++ src/utils/graph-utils.js | 47 ++++++++++++++------- src/views/Graph.vue | 34 ++++++++------- tests/unit/utils/graph-utils.spec.js | 16 +++++-- 4 files changed, 67 insertions(+), 34 deletions(-) diff --git a/src/services/mock/json/workflows/multi.json b/src/services/mock/json/workflows/multi.json index 871918d17..89c6a3721 100644 --- a/src/services/mock/json/workflows/multi.json +++ b/src/services/mock/json/workflows/multi.json @@ -141,9 +141,11 @@ { "id": "~user/other/multi/run2//1/foo", "state": "waiting", + "cyclePoint": "1", "isHeld": false, "isQueued": true, "isRunahead": false, + "name": "foo", "task": { "meanElapsedTime": 0, "__typename": "Task" @@ -157,9 +159,11 @@ { "id": "~user/other/multi/run2//2/foo", "state": "waiting", + "cyclePoint": "2", "isHeld": false, "isQueued": false, "isRunahead": false, + "name": "foo", "task": { "meanElapsedTime": 0, "__typename": "Task" diff --git a/src/utils/graph-utils.js b/src/utils/graph-utils.js index 76a369be1..40bd39a0d 100644 --- a/src/utils/graph-utils.js +++ b/src/utils/graph-utils.js @@ -1,4 +1,4 @@ -/** +/* * Copyright (C) NIWA & British Crown (Met Office) & Contributors. * * This program is free software: you can redistribute it and/or modify @@ -15,25 +15,42 @@ * along with this program. If not, see . */ +/** + * Convert graphviz edge bezier curve in dot format to SVG path . + * + * @param {string} pos - `pos` attribute of a graph edge in dot format. + * @returns {string} The SVG path. + */ export function posToPath (pos) { + // pos starts with `e,` followed by a list of coordinates + const parts = pos.substring(2).split(' ') // the last point comes first, followed by the others in order I.E: // -1, 0, 1, 2, ... -3, -2 - const parts = pos.substring(2).split(' ').map(x => x.split(',')) - const [last] = parts.splice(0, 1) - let path = null - for (const part of parts) { - if (!path) { - path = `M${part[0]} -${part[1]} C` - } else { - path = path + ` ${part[0]} -${part[1]},` - } - } - path = path + ` L ${last[0]} -${last[1]}` - return path + const [last, first] = parts.splice(0, 2) + const path = parts.reduce( + (acc, part) => `${acc} ${getCoord(part)},`, + `M${getCoord(first)} C` + ) + return `${path} L ${getCoord(last)}` } -/* TODO: everything! */ -// eslint-disable-next-line no-extend-native +/** + * Convert dotcode `pos` coordinate to SVG path coordinate. + * + * @param {string} posCoord - A coordinate in dot format. + * @returns {string} + */ +export function getCoord (posCoord) { + const [x, y] = posCoord.split(',').map(parseFloat) + return `${x} ${-y}` +} + +/** + * Calculate a non-cryptographic hash value for a given string. + * + * @param {string} string + * @returns {number} + */ export function nonCryptoHash (string) { let hash = 0 let i diff --git a/src/views/Graph.vue b/src/views/Graph.vue index d35f93e36..847785bb6 100644 --- a/src/views/Graph.vue +++ b/src/views/Graph.vue @@ -86,13 +86,12 @@ along with this program. If not, see . /> - - - - + + @@ -514,7 +513,7 @@ export default { * Get the nodes binned by cycle point * * @param {Object[]} nodes - * @returns {{ [dateTime: string]: Object[] nodes }} mapping of node to their cycle point. + * @returns {{ [dateTime: string]: Object[] }=} mapping of cycle points to nodes */ getCycles (nodes) { if (!this.groupCycle) return @@ -764,14 +763,17 @@ export default { // update graph node positions for (const obj of json.objects) { if (obj.bb) { - // if the object is a subgraph - const [left, bottom, right, top] = obj.bb.split(',') - this.subgraphs[obj.name] = { - x: left, - y: -top, - width: right - left, - height: top - bottom, - label: obj.label + // if the object is a subgraph + if (obj.label !== 'margin') { + // ignore the margins in the dot-code which do not need DOM elements + const [left, bottom, right, top] = obj.bb.split(',') + this.subgraphs[obj.name] = { + x: left, + y: -top, + width: right - left, + height: top - bottom, + label: obj.label + } } } else { // else the object is a node diff --git a/tests/unit/utils/graph-utils.spec.js b/tests/unit/utils/graph-utils.spec.js index 0f8b56199..4cc556ab6 100644 --- a/tests/unit/utils/graph-utils.spec.js +++ b/tests/unit/utils/graph-utils.spec.js @@ -1,4 +1,4 @@ -/** +/* * Copyright (C) NIWA & British Crown (Met Office) & Contributors. * * This program is free software: you can redistribute it and/or modify @@ -39,11 +39,21 @@ describe('Graph functionality', () => { 'L 211.5 -156.5' ) }) + it('Handles negative coordinates', () => { + expect(posToPath( + 'e,1,1 -2,-2 3,-3 -1,-0' + )).to.equal( + 'M-2 2 C 3 3, -1 0, L 1 -1' + ) + }) }) describe('nonCryptoHash', () => { - it('Converts a string to a stable hash', () => { - expect(nonCryptoHash('foo')).to.equal(101574) + it.each([ + ['foo', 101574], + ['', 0], + ])('Converts a string to a stable hash: %o -> %i', (str, expected) => { + expect(nonCryptoHash(str)).to.equal(expected) }) }) }) From 6abbdec6f55fd5c9d48d1fb63ca9ff064b24d6fe Mon Sep 17 00:00:00 2001 From: Mark Dawson Date: Wed, 15 May 2024 16:01:06 +0100 Subject: [PATCH 5/6] Added comments and copyright notice --- src/components/cylc/GraphSubgraph.vue | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/components/cylc/GraphSubgraph.vue b/src/components/cylc/GraphSubgraph.vue index aac449a10..c81a04cf6 100644 --- a/src/components/cylc/GraphSubgraph.vue +++ b/src/components/cylc/GraphSubgraph.vue @@ -1,3 +1,23 @@ + + +