cylc-ui icon indicating copy to clipboard operation
cylc-ui copied to clipboard

addded family grouping with subgraphs

Open markgrahamdawson opened this issue 1 year ago • 3 comments

Partly addresses issue https://github.com/cylc/cylc-ui/issues/1130 The grouping of nodes by cycle point is completed in this pr https://github.com/cylc/cylc-ui/pull/1763

----Notes on work----

Some ideas for a unified approach to grouping/collapsing cycles/families. I'm suggesting unifying the handling of cycles and families (note, cycles represent the "root" family so they are essentially the same thing).

Grouping/Ungrouping - Drawing dashed boxes around a cycle/family.

Collapsing/Expanding - Reducing a family down to a single node.

Limitations of the Cylc 7 approach:

  • Once you expand a family it's gone, the tasks which belong to the expanded family are mixed in with other tasks in the graph, you can not tell what family they belong to. This is an issue if the user wants to examine a component within the workflow.
  • No visibility of the inheritance hierarchy (i.e. what can we expand/collapse).
  • No visibility of what you have expanded/collapsed (i.e. where are we in the hierarchy).

Note, for simplicity, this approach groups/collapses all instances of selected families rather than managing this at a per-cycle level. I think this is probably more aligned with expectations, but does represent a minor limitation, e.g. there's no ability to collapse all but one cycle. The ability to expand/collapse specific cycle instances would be a reasonable enhancement.

Design Sketch image

Had a quick discussion on this (more to come):

  • Can't really think of a valid use case for collapsing all cycles (users would do this in the tree view if they wanted to), so perhaps treat cycles differently (i.e. collapse per-cycle rather than all cycles) and remove from the menus.
  • Better expand/collapse icon (obviously).
  • The Cylc 7 default of only expanding the cycle point (i.e. show top-level families only) is a reasonable protection for viewing large workflows. We might want to continue with this, or perhaps do something smart (e.g. only collapse families if there are lots of tasks on load).

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.

markgrahamdawson avatar May 24 '24 15:05 markgrahamdawson

Still to do (in order of priority).

  1. Work out how to implement nested families ~~2. Get graph status showing on graph nodes~~
  2. When switching to another workflow and returning the graph fails to load
  3. Expand collapse icons on each node (and subgraph) on the graph ~~5. If collapsed by family+ grouped by family - dont draw graph around collapsed node (see cycle point functionality)~~
  4. The edges and nodes are now let variables (so they can be updated) - is this ok? Might want to change them to something else?

markgrahamdawson avatar Jul 05 '24 08:07 markgrahamdawson

Im struggling with grouping by families due to the fact that they can be nested ...

The context

Collapsing and Expanding nodes is easier as the .dot file just needs the node/s added/removed ....

"~mdawson/runtime-tutorial-families-two/run1//20240724T0000Z/get_observations_aldergrove" [
            label=<
              <TABLE HEIGHT="132.447509765625">
                <TR>
                  <TD PORT="in" WIDTH="100"></TD>
                </TR>
                <TR>
                  <TD PORT="task" WIDTH="100" HEIGHT="132.447509765625">icon</TD>
                  <TD WIDTH="628.4224853515625">~mdawson/runtime-tutorial-families-two/run1//20240724T0000Z/get_observations_aldergrove</TD>
                </TR>
                <TR>
                  <TD PORT="out" WIDTH="100"></TD>
                </TR>
              </TABLE>
            >
          ]

and the edge (relationship between the node and other nodes) defined... "~mdawson/runtime-tutorial-families-two/run1//20240724T0300Z/get_observations_aldergrove":out -> "~mdawson/runtime-tutorial-families-two/run1//20240724T0300Z/consolidate_observations":in

This means I dont have to directly deal with a hierarchy (nested structure) as things are just being added/removed.

For grouping the syntax is a little different, using subgraphs ...

                  subgraph cluster_margin_family16
                  {
                    margin=100.0
                    label="margin"
                    subgraph cluster_family16 {"~mdawson/runtime-tutorial-families-two/run1//20240724T0000Z/get_observations_shetland","~mdawson/runtime-tutorial-families-two/run1//20240724T0000Z/get_observations_aldergrove";

                      label = "GET_OBSERVATIONS_NORTH20240724T0000Z";

                      fontsize = "70px"
                      style=dashed
                      margin=60.0
                  }
                }

For grouping by cycle point there are no nested cycle points (doesnt make sense) so its just a case of making subgraph for each cycle. The subgraphs do need to account for the fact that the nodes may have been expanded or collapsed but that can be managed by calculating what nodes need to be included from the nodes variable - which is up-to-date with what has been expanded/collapsed. Also understanding the hierarchical relationship is easier because its contained in the node id whether it has been expanded or collapsed - it will always have a cycle associated with it.

The problem

The problem is with nested grouping which is relevant for

  1. family groups inside cycle groups
  2. family groups inside family groups.

The way this is represented in the .dot code is by having subgraphs within subgraphs...

subgraph FAMILY {
  "~mdawson/run-name/run1/cycle/SUBFAMILY", "~mdawson/run-name/run1/cycle/task1", "~mdawson/run-name/run1/cycle/task2", ;
  label = Family
  subgraph SUBFAMILY {
    "~mdawson/run-name/run1/cycle/task1", "~mdawson/run-name/run1/cycle/task2", ;
    label = SubFamily
  }
}

image

The above is an simple example of some graphviz code for a simple nested family situation. Below is an example for a more complicated one... image

subgraph FAMILY {
  "~mdawson/run-name/run1/cycle/SUBFAMILY_A", "~mdawson/run-name/run1/cycle/SUBFAMILY_B", "~mdawson/run-name/run1/cycle/SUBFAMILY_A1", "~mdawson/run-name/run1/cycle/SUBFAMILY_A2", "~mdawson/run-name/run1/cycle/SUBFAMILY_B1", "~mdawson/run-name/run1/cycle/SUBFAMILY_B2", "~mdawson/run-name/run1/cycle/Task_y", "~mdawson/run-name/run1/cycle/Task_x", "~mdawson/run-name/run1/cycle/Task_m", "~mdawson/run-name/run1/cycle/Task_n", "~mdawson/run-name/run1/cycle/Task_g", "~mdawson/run-name/run1/cycle/Task_h", "~mdawson/run-name/run1/cycle/Task_i", "~mdawson/run-name/run1/cycle/Task_j" ;
  label = Family
  subgraph SUBFAMILY_A {
 "~mdawson/run-name/run1/cycle/SUBFAMILY_A1", "~mdawson/run-name/run1/cycle/SUBFAMILY_A2", "~mdawson/run-name/run1/cycle/Task_y", "~mdawson/run-name/run1/cycle/Task_x", "~mdawson/run-name/run1/cycle/Task_m", "~mdawson/run-name/run1/cycle/Task_n" ;
    label = SubFamily_A
        subgraph SUBFAMILY_A1 { "~mdawson/run-name/run1/cycle/Task_y", "~mdawson/run-name/run1/cycle/Task_x" ;
          label = SubFamily_A1
      }
      subgraph SUBFAMILY_A2 { "~mdawson/run-name/run1/cycle/Task_m", "~mdawson/run-name/run1/cycle/Task_n" ;
          label = SubFamily_A1
      }
  }
  subgraph SUBFAMILY_B {
 "~mdawson/run-name/run1/cycle/SUBFAMILY_B1", "~mdawson/run-name/run1/cycle/SUBFAMILY_B2", "~mdawson/run-name/run1/cycle/Task_g", "~mdawson/run-name/run1/cycle/Task_h", "~mdawson/run-name/run1/cycle/Task_i", "~mdawson/run-name/run1/cycle/Task_j" ;
    label = SubFamily_B
        subgraph SUBFAMILY_B1 { "~mdawson/run-name/run1/cycle/Task_g", "~mdawson/run-name/run1/cycle/Task_h" ;
          label = SubFamily_B1
      }
      subgraph SUBFAMILY_B2 { "~mdawson/run-name/run1/cycle/Task_i", "~mdawson/run-name/run1/cycle/Task_j" ;
          label = SubFamily_B1
      }
  }
}

The subgraphs can be n layers deep so that needs to be handled programatically (cant be hard coded). At the moment the graphviz .dot code is being written as an array of strings that all gets added to - pushing new values onto the end. And then using the join method on the array to make one big string.

I have thought about giving each node a ranking in terms of how 'deep' it is in the hierarchy then ranking from most deep to least deep then looping through ... but this wont work because (as in the example above) you would miss out a lot of the graph

markgrahamdawson avatar Jul 29 '24 12:07 markgrahamdawson

I think this is a problem that warrants recursion as it's tricky to unroll as an iterative loop.

Here's an idea of what that could look like (Python syntax):

  • First, go through every task and create a dotcode entry for each (this is the bit that includes the <TABLE /> label).
  • Then go through every family in inheritance order and build the nested subgraphs for each, inserting the task entries we have just built into the relevant subgraph when we get to it.
  • Then do the '\n'.join(dotcode) bit.
from random import random

TASKS = {
    'foo': {
        'name': 'foo',
        'parent': 'FOO',
    },
    'FOO': {
        'name': 'FOO',
        'parent': 'root'
    },
    'bar': {
        'name': 'bar',
        'parent': 'BAR1',
    },
    'baz': {
        'name': 'baz',
        'parent': 'BAR2',
    },
    'BAR1': {
        'name': 'BAR1',
        'parent': 'BAR',
    },
    'BAR2': {
        'name': 'BAR2',
        'parent': 'BAR',
    },
    'root': {
        'name': 'root',
        'parent': None,
    },
}

TREE = {
    'root': {
        'FOO': None,
        'BAR': {
            'BAR1': None,
            'BAR2': None,
        },
    },
}

def add_subgraph(dotcode, pointer, graph_sections):
    for key, value in pointer.items():
        dotcode.append(
            f'subgraph cluster_{str(random())[2:]} {{'
            f'\nlabel = "{key}"'
        )

        if value:
            add_subgraph(dotcode, value, graph_sections)

        if key in graph_sections:
            dotcode.extend(graph_sections[key])

        dotcode.append('}')

    return dotcode

def get_dotcode(tasks):
    graph_sections = {}

    for task in tasks.values():
        parent = task['parent']
        if not parent:
            continue
        section = graph_sections.setdefault(parent, [])
        section.append(f'{task["name"]} [title="{task["name"]}"]')

    dotcode = ['digraph {']
    add_subgraph(dotcode, TREE['root'], graph_sections)
    return dotcode


for item in get_dotcode(TASKS):
    print(item)
digraph {

  subgraph cluster_23300787190407446 {
    label = "FOO"

    foo [title="foo"]
  }

  subgraph cluster_5025488657295563 {
    label = "BAR"

    subgraph cluster_2135762450670372 {
      label = "BAR1"

      bar [title="bar"]
    }

    subgraph cluster_4413670667138756 {
      label = "BAR2"

      baz [title="baz"]
    }

  BAR1 [title="BAR1"]
  BAR2 [title="BAR2"]
}

I haven't taken cycles into account in this solution, you'll need to add a for cycle in cycles loop at the top of this.

This solution will also add entries for families which have no tasks, so, you'll need some fancy logic for removing empty families, and any families that contain only empty families.

oliver-sanders avatar Jul 31 '24 15:07 oliver-sanders

@wxtim, could you join me on this review.

These things can be moved to follow up:

  • Consider unifying the toolbar items (i.e. fewer buttons).
  • Adding expand/collapse icons to families / cycles in the graph itself.
  • Default collapse state (like the Cylc 7 GUI).
  • Expand/collapse all buttons (or similar).

Note, the graph view is a pain to test.

oliver-sanders avatar Nov 08 '24 16:11 oliver-sanders

@markgrahamdawson - Observed bug with

[scheduler]
    allow implicit tasks = True

[scheduling]
    initial cycle point = 1
    cycling mode = integer
    [[graph]]
        R1 = wednesday => FAM1:succeed-any

[runtime]
    [[root]]
        script = sleep 1000
    [[FAM1]]
    [[FAM1a]]
        inherit = FAM1

    [[wednesday]]
    [[henry, geoffry, richard, edward]]
        inherit = FAM1
    [[richard, edward]]
        inherit = FAM1a

Collapsing FAM1a removes the dependency arrows: image

Collapsing FAM1 also removes dependency arrows image

wxtim avatar Nov 11 '24 11:11 wxtim

[minor point] @oliver-sanders Observed that collapsed by Cycle point cycles probably don't need both of the Cycle point labels:

image

wxtim avatar Nov 11 '24 11:11 wxtim

ycle point cycles probably don't need both o

Would we rather keep the big one or the small one?

markgrahamdawson avatar Nov 11 '24 11:11 markgrahamdawson

Would we rather keep the big one or the small one?

We should keep the big one.

Easy to do, but it'll need some if type logic in the GraphNode component. Can do as follow up.

oliver-sanders avatar Nov 11 '24 11:11 oliver-sanders

[tangential to , but exacerbated by this PR]

We were talking about the funky lines GraphViz sometimes comes up with.

I suspect that these are the result of GraphViz trying to thread edges between tasks and subgraphs.

We currently apply spacing to subgraphs by nesting them inside another subgraph. There is a GraphViz margin attribute which might work for us?

oliver-sanders avatar Nov 11 '24 12:11 oliver-sanders

Hi @markgrahamdawson

I'm still getting missing dependency arrows, though it was harder to reproduce:

[scheduler]
    allow implicit tasks = True
    cycle point format = %Y

[scheduling]
    initial cycle point = 1971
    [[graph]]
        P1Y = """
            X[-P1Y]:succeed-all => start_cycle
            start_cycle => X:succeed-all => _dummy_ => Y
        """

[runtime]
    [[root]]
        script = sleep 1000
    [[X, Y]]

    [[x]]
        inherit = X
    [[y]]
        inherit = Y

I had collapset 1972 and X

image

wxtim avatar Nov 13 '24 15:11 wxtim

I can't break it. 🚀

wxtim avatar Nov 15 '24 11:11 wxtim

[tangential to , but exacerbated by this PR]

We were talking about the funky lines GraphViz sometimes comes up with.

I suspect that these are the result of GraphViz trying to thread edges between tasks and subgraphs.

We currently apply spacing to subgraphs by nesting them inside another subgraph. There is a GraphViz margin attribute which might work for us?

On this one we, we do use that margin attribute - line 855 in the src/views/Graph.vue

markgrahamdawson avatar Nov 15 '24 12:11 markgrahamdawson

Got some conflicts that I suggest resolving by merging master into this branch

MetRonnie avatar Jan 14 '25 16:01 MetRonnie

Testing on a real, complex workflow, unfortunately I got this error when I tried to collapse a family:

Graph.vue:1589 TypeError: Cannot read properties of undefined (reading 'width')
    at Proxy.layout (Graph.vue:1660:29)
    at async Proxy.refresh (Graph.vue:1584:9)

MetRonnie avatar Feb 05 '25 11:02 MetRonnie

@wxtim This is normally as a result of an uncaught error which will be visible in the console

MetRonnie avatar Mar 25 '25 11:03 MetRonnie

I've left a workflow running all day in the background with this display on and haven't noted any indication of slowdown.

wxtim avatar Mar 26 '25 16:03 wxtim

unately, edges are missing between collapsed famil

Hopefully fixed with b108b6f04935ab2d740de00cc3fad24f0c0400d3 image

markgrahamdawson avatar Mar 28 '25 13:03 markgrahamdawson

unately, edges are missing between collapsed famil

Hopefully fixed with b108b6f

I have unfortunately found an similar issue with cycles

Collapsed families and grouped by cycle image

Collapsed families and grouped by cycle... and then collapse by a cycle

image

however two collapsed cycles do have edges between each other

image

markgrahamdawson avatar Mar 28 '25 13:03 markgrahamdawson

unately, edges are missing between collapsed famil

Hopefully fixed with b108b6f

I have unfortunately found an similar issue with cycles

Hopefully fixed with 73d05f50086e96bff0b60b0b97721c4b78c3fccf

image

markgrahamdawson avatar Mar 31 '25 12:03 markgrahamdawson