Add a cirq.plot() function and SupportsPlot protocol

[pingyeh]

This implements what I proposed in #1890 (comment).

• A SupportsPlot protocol containing a single _plot_ method. Think of it as a graphic version of __str__. It should only plot on the axes given in the arg, and never call show().
• A cirq.plot function that calls _plot_ with an axes instance. Think of it as a graphic version of print. It creates a new figure and axes if no axes is given, passes the axes to _plot_ for rendering, and calls show. It is intended for interactive use.

I migrated Heatmap, CrossEntropyResult, RabiResult and RandomizedBenchMarkResult to SupportsPlot protocol in this PR. TomographyResult looks a bit more complicated so I left a TODO.

Two demo codes and plots are shown below.

[Strilanc]

What is the advantage of this over just calling object.plot()?

For example, the advantage of cirq.unitary is that it can fallback to other methods (such as decompose) and works on native python types such as a list (of operations). The advantage of cirq.approx_equal is all of the fallback logic around trying both objects and using __eq__, which would be tedious to add to every object.

[pingyeh]

As described in my proposal #1890 (comment), this design solves the tension between 2 usecases:

1. People want to show figures in interactive sessions, with less keystrokes if possible.
2. People don't want to see figures popping up in tests.

This design makes _plot_ method plot onto the Axes passed in, so there is no figures popping up in tests. The cirq.plot function provides a easy shorthand to show the graph in interactive sessions while re-using the logic in _plot_ methods.

In this design, _plot_ is a library which should never call show. I think it is a better solution than show(warn=False).

@dabacon, @mpharrigan, what do you think?

[Strilanc]

But couldn't that also be solved by having the object have a plot method that took the axes? I don't see the user's incentive to have the method global.

[pingyeh]

You have to do 2 things for a plot method to work both as a library and in interactive sessions:

1. Create an axes if one is not passed in.
2. Call show with warn=False so it doesn't cause warnings in tests.

(1) is repeated logic in every plot method. The global cirq.plot method put that logic in one place. I like the DRY perspective of that.

(2) is likely not the best solution, either. We just should not call show in library codes.

[mpharrigan]

I don't think it's much overhead to have

if ax is None:
  ax = plt.gca()

I think this is pretty standard for libraries that build on matplotlib. I still don't think anything should call show :) interactive sessions in jupyter notebook don't require a call to show

[pingyeh]

I'm hoping that cirq libraries only use object-based matplotlib codes instead of the codes that use "current" or "default" figures like gca() or gcf(), because they produce side effects.

[mpharrigan]

If someone doesn't pass in an ax, they're presumably already using matplotlib's stateful mode. Cirq would only use object-y methods. The translation between the two would happen with the one call to ax = plt.gca().

Off the top of my head, I thought of these libraries which offer plotting, and they follow this idiom:

• https://networkx.github.io/documentation/stable/_modules/networkx/drawing/nx_pylab.html#draw
• https://github.com/mwaskom/seaborn/blob/c73055b2a9d9830c6fbbace07127c370389d04dd/seaborn/categorical.py#L2953

[Strilanc]

Ping, are you aware of libraries that offer plotting following a different idiom? I would strongly prefer to follow existing patterns when possible.

[Strilanc]

For now I think the verdict is that we keep plotting as a per-object method. If we find more things that have to be common to the plotting methods, such as e.g. supporting writing to a file in some standard way, then it will make sense to extract a common method.

I am particularly wary of the fact that plotting is something that is important outside of cirq, and so cirq cannot really impose a standard way to do it. The benefits would need to be pretty large to override this (e.g. as they are with approx equal).

[pingyeh]

Let me bring your attention to the Coding Styles recommended by matplotlib.

It recommends that the figure and axes be created via functions in the plt module, then use the object-oriented codes in user's plotting functions, where an Axes object, data to be plotted, and keyword arguments (for tuning the look of the plot) are passed in. No calls to plt functions from the user function.

We all like to group data to be plotted into classes. The user's plotting functions become a plotting method of the class, so the data args are unnecessary. What remains is exactly the SupportsPlot protocol in this PR.

@Strilanc seems to focus on the cirq.plot, which is not my focus. I can remove it from the PR. The key is the SupportsPlot protocol and removing plot.show in plotting methods.

I'm open to naming plot methods _plot_ or plot. The args and behavior are what I really care about.

[Strilanc]

Yes, please remove cirq.plot. But note that there is no purpose in defining a SupportsPlot protocol if there's no corresponding method consuming it, so the protocol should also be removed.

How about we add a show=False argument to the plot methods?

[pingyeh]

Do you mean something like this?

    def plot(self, ax, show=False, **kwargs):
        ...
        if show:
            ax.get_figure().show()

No, I don't want to. Library codes should not call show(), as discussed in #1890 and indicated in the matplotlib coding style.

If we agree that all plot methods should take (an Axes, a keyword args) and not call show, I can drop this PR and start migrating the existing plot methods and their call sites throughout the codebase.

[mpharrigan]

Let me bring your attention to the Coding Styles recommended by matplotlib.

TIL! Thanks @pingyeh. I still don't think anyone follows this though... They have a lot of gall to suggest we all "do as they say" :) For fun, I was quickly able to find an instance within matplotlib that uses if ax is None: ax = gca(): https://matplotlib.org/3.1.1/_modules/matplotlib/pyplot.html#colorbar . Hilariously, the documentation for this parameter is: "Parent axes from which space for a new colorbar axes will be stolen". Makes me chuckle how far from their own advice it is

[pingyeh]

It is a function in pyplot, so it's supposed to do that.

We certainly do not follow recommendation blindly. But this recommendation is something I agree with quite strongly for reasons I wrote above, e.g., lack of side effects, easier to test, etc.

[Strilanc]

Please drop the global plot method. Write the class instance plot method according to the best practices you outlined.

[pingyeh]

Please drop the global plot method. Write the class instance plot method according to the best practices you outlined.

Sent #2286 for review.

[Strilanc]

Obsoleted by the introduction of conventions around plotting methods.