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

Introduces PassManager drawer #2445

Merged
merged 16 commits into from
May 23, 2019
Merged

Conversation

maddy-tod
Copy link
Contributor

Summary

Fixes #2317

This introduces a drawer for the PassManager. It takes the pass manager to be drawn, a filename to write to and optionally a style dictionary.

Details and comments

The drawer creates a box for each flow controller and labels it with the name of the controller. It then adds any passes belonging to this controller into the box. The colour of the box for each pass is decided from the style dict, with the default being different colours for analysis and transformation passes. Any inputs to the pass are then added in circles, with any optional parameters having a dashed edge.

This is an example, taken as a subsection of the graph created when the level1 pass manager is drawn.
image

@nonhermitian
Copy link
Contributor

The fact that this relies on pydot, and thus GraphViz, means the ability for the end user to execute this code is beyond our packaging capacbilities; It requires the user to go and install binaries themselves. Is it not possible to use Matplotlib here? The flow of a pass manager looks like a directed, possibly cyclic, graph.

@maddy-tod
Copy link
Contributor Author

pydot and GraphViz are already used by the DAG drawer, so I don't think this should be an issue. I can have a look at doing it using matplot if that would be preferable, but as it is drawing a graph structure I think that GraphViz is a good choice.

@nonhermitian
Copy link
Contributor

I would argue the dag drawer should go to mpl as well. However, others probably have different opinions.

@ajavadia
Copy link
Member

Yes matplotlib is preferable since it does not introduce non-python dependencies. However drawing graphs such as the DAG is a not a very easy task. Graphviz uses a non-trivial algorithm for finding good coordinates for the nodes such that the output is visually appealing.
It is a good project for someone to port these to matplotlib, but for now relying on a graph library is easier.
I should also say that only a pass developer needs to see the DAG. The normal circuit representation suffices for everyone else. That's why the dependency is not a hard dependency.
This pass manager graph may have more users. It may also be slightly simpler because the size of the graph is typically not more than 5-6 nodes.

Copy link
Contributor

@lcapelluto lcapelluto left a comment

Choose a reason for hiding this comment

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

Nice feature!

@jaygambetta
Copy link
Member

I think this is nice. I dont mind it using GraphViz as @ajavadia says this is not a simply algorithm to draw. How does it error if I dont have it installed.

@maddy-tod
Copy link
Contributor Author

@jaygambetta it raises an ImportError with instructions about how to install GraphViz and pydot, seen here.

@jaygambetta
Copy link
Member

great.

@nonhermitian nonhermitian merged commit 2d14c1e into Qiskit:master May 23, 2019
mtreinish added a commit to mtreinish/qiskit-core that referenced this pull request May 23, 2019
This commit fixes the graphviz check in the pass manager visualization
added recently in Qiskit#2445. The check was added to determine if the dot
command was in the default PATH by trying to subprocess out and run
'dot -V' however the way in which subprocess was called results in both
the dot version string and a blank line being printed if graphviz is
actually installed. This happens on import time which we definitely
don't want to do (and breaks test discovery in stestr).
mtreinish added a commit that referenced this pull request May 24, 2019
This commit fixes the graphviz check in the pass manager visualization
added recently in #2445. The check was added to determine if the dot
command was in the default PATH by trying to subprocess out and run
'dot -V' however the way in which subprocess was called results in both
the dot version string and a blank line being printed if graphviz is
actually installed. This happens on import time which we definitely
don't want to do (and breaks test discovery in stestr).
lia-approves pushed a commit to edasgupta/qiskit-terra that referenced this pull request Jul 30, 2019
* First go at pass manager drawer - draws the passes into boxes based on their flow controllers

* Added in comments and coloring the nodes

* Added in drawing the arguments

* Added in testing

* Refactoring

* Fixed cyclic import

* Made filename non optional

* CHANGELOG

* Added in checks to see if GraphViz is installed - will skip tests if not

* Renames pss to pass_ and moved an import basedon review

* Moved import to fix lint
lia-approves pushed a commit to edasgupta/qiskit-terra that referenced this pull request Jul 30, 2019
This commit fixes the graphviz check in the pass manager visualization
added recently in Qiskit#2445. The check was added to determine if the dot
command was in the default PATH by trying to subprocess out and run
'dot -V' however the way in which subprocess was called results in both
the dot version string and a blank line being printed if graphviz is
actually installed. This happens on import time which we definitely
don't want to do (and breaks test discovery in stestr).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

PassManager.passes() needs a drawer
5 participants