Enhanced graphviz functionality #2596

chrisjsewell · 2019-03-13T22:12:15Z

So I don't expect this to be merged any time soon,
since all the tests would need to be rewritten (plus style fixes).
But the basic idea is I've:

Used the python-graphviz package which;
- simplifies the code and make it more 'pythonic'.
- has a _repr_svg_() method for rendering in the notebook.
Rewritten the function as a class, which
- maintains a cache of the added nodes and graphs (as sets)
- splits up the function and allows for multiple additions to the same graph
- has separate _style_ functions for different node classes

Here are two examples of using it in the notebook.

The first is a replication of the original function, with some extras.

The second is new functionality, that was the original reason I started writing this.
It visualizes a root node and filtered connecting nodes, without any intermediate connections.

chrisjsewell · 2019-03-13T23:14:01Z

Oh and I've just noticed the actual name of the file would need to be change
(from graphviz.py), so there is not an import clash

chrisjsewell · 2019-03-13T23:32:18Z

and if you were to get really fancy;
plugins could provide something like aiida.graph.style entry_points, to define how any of their node classes are styled in the graph (e.g. adding class specific information to the label, like I have done for CifData)

ltalirz · 2019-03-14T19:59:08Z

@chrisjsewell very cool!
I would propose to schedule a discussion on this during the plugin migration week unless others want to move forward with this even before.
mentioning @giovannipizzi @sphuber

giovannipizzi · 2019-03-15T07:08:09Z

Very nice! Indeed let's discuss in person - I'd also like to brainstorm if we can unify in a single place the routines to describe a node (e.g. we now have get_desc()that should probably move out of the Node subclasses, we should unify with the generation of text-only (for Graphviz), HTML string (for Materials Cloud), information on what to visualise (for Materials Cloud), ...

I.e., we should probably have a "visualiser" class that for every class know what an how to visualise it (and I see some possibilities to merge with @chrisjsewell idea of 'style' entry points).
We should also look at how jupyter provides multiple output formats when displaying classes (text, html, ...) to get inspiration.

chrisjsewell · 2019-04-17T22:59:42Z

@ltalirz @sphuber @giovannipizzi I have updated this module to be compatible with the current develop branch (i.e. v1.0.0b2). I have also streamlined (to some extent) the mapping of node/link -> visualisation, implementing four default mapping functions:

LinkType's -> graphviz edge style dict, e.g. {"style": "dashed", "color": "blue"}
Data.class_node_type -> graphviz node style dict, e.g. {"shape": "diamond", "color": "blue"} (defaulting to node.get_style_default() if available)
Data.class_node_type -> a data-type specific sub-label (defaulting to node.get_description())
ProcessNode.class_node_type -> graphviz node style dict, similar to Data, but also colour by success/failure

The default mapping functions can also be overridden by the user, on instantiation of the Graph class, (by providing alternate functions), making it easy to customise.

Below, I attach a notebook and screenshot, replicating your AddAndMultiplyWorkChain example:

Provenance_Graphviz.ipynb.txt

coveralls · 2019-04-18T03:52:37Z

Coverage decreased (-0.1%) to 71.544% when pulling 07fe0f5 on chrisjsewell:enhanced-graphviz into 3a35710 on aiidateam:develop.

coveralls · 2019-04-18T03:52:38Z

Coverage increased (+0.1%) to 74.435% when pulling 4753cf1 on chrisjsewell:enhanced-graphviz into 19ed82e on aiidateam:develop.

chrisjsewell · 2019-07-04T07:36:03Z

does this seem reasonable to you as well

Yeh as I've mentioned, I added the functionality, for it to be fairly easy to change styles, at the user level. So I'm not necessarily to bothered what the defaults are, as I may choose not to use them :)
Personally, the edges are fine, but I'd prefer a shape difference between Work processes and Calc processes, and I think there's probably a case for two color "modes":

Process analysis: Data = white, Failed Processes = red, Running Processes = orange, Successful processes = green.
Presentational: with distinct colors for different node types.

You could maybe mix them (using different background/font colors), but that gets a bit messy.

Perhaps I should add an easy way of switching?

chrisjsewell · 2019-07-04T09:44:48Z

Ok, in b0c2be0, I have applied @ltalirz's style format. I have also merged default_process_styles and default_data_styles into default_node_styles, and added a pstate_node_styles.

From aiida/backends/tests/tools/visualization/test_graph.py, you can create (just uncomment the render line):

test_graph_graphviz_source:

graph = graph_mod.Graph()
graph.recurse_descendants(nodes.pd0)

test_graph_graphviz_source_pstate:

graph = graph_mod.Graph(node_style_fn=pstate_node_styles)
graph.recurse_descendants(nodes.pd0)

and obviously, you can just copy one of these functions, and make your own style very easily.

ltalirz

Dear @chrisjsewell, thank you so much for implementing all these changes!

Here come my final comments, after this I'm happy to merge:

I'm now getting the following warnings - not quite sure why?

verdi graph generate 31317b2e-d126-4fdc-a2a8-80f1a9aa773b
Info: Initiating graphviz engine: dot
Info: Recursing ancestors, max depth=None
Info: Recursing descendants, max depth=None
Warning: gvrender_set_style: unsupported style rectangle - ignoring
Warning: gvrender_set_style: unsupported style rectangle - ignoring
Warning: gvrender_set_style: unsupported style rectangle - ignoring
Warning: gvrender_set_style: unsupported style rectangle - ignoring
Warning: gvrender_set_style: unsupported style rectangle - ignoring
Warning: gvrender_set_style: unsupported style rectangle - ignoring
Warning: gvrender_set_style: unsupported style rectangle - ignoring
Warning: gvrender_set_style: unsupported style rectangle - ignoring
Warning: gvrender_set_style: unsupported style rectangle - ignoring
Warning: gvrender_set_style: unsupported style rectangle - ignoring
Warning: gvrender_set_style: unsupported style rectangle - ignoring
Warning: gvrender_set_style: unsupported style rectangle - ignoring
Warning: gvrender_set_style: unsupported style rectangle - ignoring
Warning: gvrender_set_style: unsupported style rectangle - ignoring
Success: Output file: 32.dot.pdf

Please add a uuid/pk switch at the command line as well. I would personally even vote to make uuid the default but that is not an issue as long as one can easily switch (something like --id-label)
Could we move to a color different from white for workfunctions? Something that looks related to the color of the WorkChain?
My style, by default, removed the borders of the shapes because I feel they are just adding lines that aren't really needed (?)
Do we still want to add the hint on the status of process nodes by adding a red/yellow/green border for those?

chrisjsewell · 2019-07-05T08:11:32Z

Thanks @ltalirz

I'm now getting the following warnings

Oops, there was a typo in specifying styles for WorkFunctionNodes (the only process type I didn't include in the tests!), hopefully fixed in 0b5087f

Could we move to a color different from white for workfunctions?

It was white because of the above error, should be the same color as WorkChains now

Please add a uuid/pk switch at the command line

added in 9076f2b

My style, by default, removed the borders of the shapes

fixed in a8f6009

Do we still want to add the hint on the status of process nodes by adding a red/yellow/green border for those?

I leave that up to you, @giovannipizzi and @sphuber?

ltalirz · 2019-07-05T12:27:46Z

@chrisjsewell Thanks, for me it's good to merge
Let's give @sphuber and @giovannipizzi a few hours to reply - otherwise I'll go ahead.

I would simply "squash & merge" if that's OK with you.
If you like to keep any topical commits separated, I would ask you to rebase into, say, no more than 5.

chrisjsewell · 2019-07-05T12:30:02Z

Yep "squash & merge" will be fine thanks @ltalirz

sphuber

Just some minor comments/questions. Other than that I'd be happy to merge this. Great work @chrisjsewell and thanks a lot for all the reviewing @ltalirz ! 👍

aiida/cmdline/commands/cmd_graph.py

sphuber · 2019-07-05T14:16:13Z

aiida/cmdline/commands/cmd_graph.py

+def generate(root_node, link_types, id_label, ancestor_depth, descendant_depth, process_out, process_in, engine,
+             verbose, output_format, show):
+    """
+    Generate a graph from a ROOT_NODE (specified by pk or uuid).


I think technically the node label could also be used. Granted it won't be that common. I think it would maybe be better to use the term "identifier". This is used across the CLI to mean one of PK, UUID, or label.

fixed in 6e599e9

aiida/cmdline/commands/cmd_graph.py

sphuber · 2019-07-05T14:20:08Z

aiida/cmdline/commands/cmd_devel.py

-        'aiida.transports',
-        'aiida.tools.dbimporters.plugins',
+        'aiida.cmdline.utils', 'aiida.cmdline.params.types', 'aiida.cmdline.params.options', 'aiida.common',
+        'aiida.schedulers', 'aiida.transports', 'aiida.tools.dbimporters.plugins', 'aiida.tools.visualization'


I think all the tests are actually in the module aiida.backends correct? And I don't see any in aiida.tools.visualization, so it is not necessary to add it here

fixed in 6e599e9

sphuber · 2019-07-05T14:23:22Z

aiida/tools/visualization/graph.py

+        "process.calculation.calcjob.CalcJobNode.": {
+            "shape": "ellipse",
+            "style": "filled",
+            # "pencolor": "black",


If we are not using the pencolor, maybe get rid of the commented out code

fixed in 6e599e9

sphuber · 2019-07-05T14:24:44Z

aiida/tools/visualization/graph.py

+            node_style['fillcolor'] = '#de707fff'  # red
+        elif node.is_finished_ok:
+            node_style['fillcolor'] = '#8cd499ff'  # green
+        else:  # specifically look for waiting state?


Note that this conditional will hit the states CREATED and RUNNING as well. Think that the same color as WAITING is fine, just thought I'd mention it. Maybe update the comment to include them

fixed in 6e599e9

sphuber · 2019-07-05T14:26:16Z

aiida/tools/visualization/graph.py

+    """
+    # pylint: disable=too-many-branches
+
+    class_node_type = node.class_node_type


I take it this code can be cleaned up when we address #3087 and we define a node method that will provide this type of descriptor?

Yes indeed. Basically all the node classes should supply their own (default) description, and this function (which is injected into the Graph class) can just be used by the user, to override any descriptions they wish.

ltalirz · 2019-07-07T01:06:06Z

@sphuber Looks to me like your points have been addressed
I leave the final merge up to you

sphuber · 2019-07-07T06:59:07Z

Thanks a lot to both !

The `python-graphviz` package is employed to simplify the code that can build a visual representation of a sub set of the provenance graph. The main graphing function is rewritten as a class, which: * maintains a cache of the added nodes and graphs (as sets) * splits up the function and allows for multiple additions to the graph * has separate `_style_` functions for different node classes This gives larger control to define the graph that is to be visualized as well as the style to be used. The current style is adapted to resemble that of the documentation for a coherent look.

chrisjsewell added 3 commits February 28, 2019 15:57

CJS fix

62b398b

Merge remote-tracking branch 'upstream/develop' into develop

4a11d5c

add main code

1714c4d

chrisjsewell added 2 commits March 14, 2019 00:23

added add_origins_to_targets method

5d3ca0b

fix _cif_style

8ef11f2

chrisjsewell added 8 commits March 18, 2019 13:35

Merge remote-tracking branch 'upstream/develop' into develop

f44c16d

Merge remote-tracking branch 'upstream/develop' into develop

6e40e7d

Merge remote-tracking branch 'upstream/develop' into develop

2ef18e6

Merge remote-tracking branch 'upstream/develop' into develop

1089287

Merge remote-tracking branch 'upstream/develop' into develop

b236a65

Merge remote-tracking branch 'upstream/develop' into develop

104f4e4

Merge remote-tracking branch 'upstream/develop' into enhanced-graphviz

d5371a5

updated to be compatible with v1.0.0b2

35685d3

chrisjsewell added 7 commits April 18, 2019 01:29

test fixes

4d02467

convert docstrings to ReST style

2d85794

test fixes

4cdca70

test fixes

2ee6ca9

test fix

1f8a01c

docstring fixes

5a111b5

Update cmd_graph.py

07fe0f5

chrisjsewell added 3 commits April 18, 2019 06:11

default sub-label to node.get_description()

28cc1b7

move the process node styling to a separate function

695de89

attempt to get default style from node.get_style_default()

661eba4

chrisjsewell added 2 commits July 4, 2019 10:34

applied agreed style formats and added tests

b0c2be0

Merge remote-tracking branch 'upstream/develop' into enhanced-graphviz

7b7123e

chrisjsewell added 2 commits July 4, 2019 10:49

remove penwidth

e74d5e3

test fix

4753cf1

ltalirz suggested changes Jul 5, 2019

View reviewed changes

chrisjsewell added 2 commits July 5, 2019 08:52

fix WorkFunctionNode styles

0b5087f

Merge branch 'develop' into enhanced-graphviz

17244dc

chrisjsewell added 3 commits July 5, 2019 09:40

added --id-label option to verdi graph generate

9076f2b

remove node borders

a8f6009

yapf fix

2d156fb

ltalirz self-requested a review July 5, 2019 12:58

ltalirz previously approved these changes Jul 5, 2019

View reviewed changes

sphuber reviewed Jul 5, 2019

View reviewed changes

fixes for shuber review

6e599e9

chrisjsewell dismissed ltalirz’s stale review via 6e599e9 July 5, 2019 15:43

Fix yapf

b599361

sphuber approved these changes Jul 7, 2019

View reviewed changes

sphuber merged commit ad5d26f into aiidateam:develop Jul 7, 2019

chrisjsewell deleted the enhanced-graphviz branch July 8, 2019 03:52

chrisjsewell mentioned this pull request Jul 8, 2019

Add tutorial on aiida.tools.visualization.graph.Graph #3147

Merged

ltalirz mentioned this pull request Oct 21, 2019

verdi graph - Use UUIDs instead of pks #2636

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhanced graphviz functionality #2596

Enhanced graphviz functionality #2596

chrisjsewell commented Mar 13, 2019 •

edited by ltalirz

Loading

chrisjsewell commented Mar 13, 2019

chrisjsewell commented Mar 13, 2019 •

edited

Loading

ltalirz commented Mar 14, 2019

giovannipizzi commented Mar 15, 2019

chrisjsewell commented Apr 17, 2019 •

edited

Loading

coveralls commented Apr 18, 2019

coveralls commented Apr 18, 2019 •

edited

Loading

chrisjsewell commented Jul 4, 2019 •

edited

Loading

chrisjsewell commented Jul 4, 2019 •

edited

Loading

ltalirz left a comment

chrisjsewell commented Jul 5, 2019 •

edited

Loading

ltalirz commented Jul 5, 2019 •

edited

Loading

chrisjsewell commented Jul 5, 2019

sphuber left a comment

sphuber Jul 5, 2019

chrisjsewell Jul 5, 2019

sphuber Jul 5, 2019

chrisjsewell Jul 5, 2019

sphuber Jul 5, 2019

chrisjsewell Jul 5, 2019

sphuber Jul 5, 2019

chrisjsewell Jul 5, 2019

sphuber Jul 5, 2019

chrisjsewell Jul 5, 2019

ltalirz commented Jul 7, 2019

sphuber commented Jul 7, 2019

Enhanced graphviz functionality #2596

Enhanced graphviz functionality #2596

Conversation

chrisjsewell commented Mar 13, 2019 • edited by ltalirz Loading

chrisjsewell commented Mar 13, 2019

chrisjsewell commented Mar 13, 2019 • edited Loading

ltalirz commented Mar 14, 2019

giovannipizzi commented Mar 15, 2019

chrisjsewell commented Apr 17, 2019 • edited Loading

coveralls commented Apr 18, 2019

coveralls commented Apr 18, 2019 • edited Loading

chrisjsewell commented Jul 4, 2019 • edited Loading

chrisjsewell commented Jul 4, 2019 • edited Loading

ltalirz left a comment

Choose a reason for hiding this comment

chrisjsewell commented Jul 5, 2019 • edited Loading

ltalirz commented Jul 5, 2019 • edited Loading

chrisjsewell commented Jul 5, 2019

sphuber left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ltalirz commented Jul 7, 2019

sphuber commented Jul 7, 2019

chrisjsewell commented Mar 13, 2019 •

edited by ltalirz

Loading

chrisjsewell commented Mar 13, 2019 •

edited

Loading

chrisjsewell commented Apr 17, 2019 •

edited

Loading

coveralls commented Apr 18, 2019 •

edited

Loading

chrisjsewell commented Jul 4, 2019 •

edited

Loading

chrisjsewell commented Jul 4, 2019 •

edited

Loading

chrisjsewell commented Jul 5, 2019 •

edited

Loading

ltalirz commented Jul 5, 2019 •

edited

Loading