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

Labels should be added to mpl drawer #4580

Closed
enavarro51 opened this issue Jun 15, 2020 · 1 comment · Fixed by #4616
Closed

Labels should be added to mpl drawer #4580

enavarro51 opened this issue Jun 15, 2020 · 1 comment · Fixed by #4616
Labels
type: enhancement It's working, but needs polishing

Comments

@enavarro51
Copy link
Contributor

What is the expected enhancement?

Labels were added to text drawer in #4361, #4262, and #4420. Labels are user-supplied text that overwrites the names of gates and can be added to controls. This needs to be implemented for the mpl drawer. This issue is intended as a discussion to come to a consensus on design before we start coding.

image
The control labels can be placed on the highest control or below the lowest control if there are no controls above. And placed inside or outside the control line. In addition to what's shown here, some other possible questions would be,

  1. If there are no params, should the original gate name be placed in the gate box underneath the gate label in smaller text? Might help prevent confusion if someone did YGate(label='X')
  2. Or should the original gate name be placed in parens at the end of the label? This would work whether there are params or not and since most gate names are short would not take up too much space in most cases.
  3. What do we do with gate labels for swap, cz, cu1, rzz, cx's and possibly others that don't have a box to display in? Should we use sidetext for these?
  4. Currently all gate names are capitalized. Should all labels be capitalized or left as the user enters them?

One other thing. qc.cy(0, 1, label='Y Label') will always produce a control label. In the current implementation, the data structure is the same as
qc.append(YGate().control(1, label='Y Label'), [0, 1]).
If you want a gate label, you have to use a YGate form.

@enavarro51 enavarro51 added the type: enhancement It's working, but needs polishing label Jun 15, 2020
@enavarro51
Copy link
Contributor Author

New mpl drawer version

@1uciano @ajavadia Over the last several weeks (stuck in lockdown) I started going through the mpl drawer code and discovered there were a lot of redundancies, inefficiencies, and unused code. I decided to see how I might be able to restructure the code and also wanted to see if I could implement the text width calculations and gate and control labels.

After restructuring and adding the features, I now have a version that incorporates all the open issues, including text widths to determine gate and layer widths for a proportional font, and labels. Even with adding these features, the code is 140 lines shorter than the current master version.

I would like to do a PR, and I know you prefer incremental changes, but I feel like incremental changes will not produce an efficient, easily maintained mpl drawer in a reasonable time. I have tested the code extensively with many matplotlib backends and many circuits. Someone should probably still take some time to go through what I've done in some detail in order to approve it.

So should I submit a PR, which would list in detail all the changes I made, or would you prefer to stick with the current master and try to bring it up to a better standard over a longer period of time?

Thanks.

Below are images from the current master and from my revised version. isometry and initialize are not in the master image since these crash the drawer.
Current master
image

New version
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement It's working, but needs polishing
Projects
None yet
1 participant