Fix latex drawer symmetrical gates display #6877

enavarro51 · 2021-08-05T15:52:50Z

Summary

Fixes #6050
Continuation of #6061

Details and comments

This PR is a continuation of #6061 and resolves several merge conflicts with that PR. It corrects the spacing of the names for symmetrical gates in the latex drawer.

…fix6050

…o fix6050

enavarro51 · 2021-08-09T22:56:26Z

@TharrmashasthaPV I was wondering if you could have a look at this PR. It was started by @jsistos and I'm finishing it off. It accomplishes the goal, but you commented we are trying to eliminate adding header info in latex source. So is there a way to accomplish the same thing without the added \newlength? Thanks.

jsistos · 2021-08-10T07:06:12Z

Hello,
I have been unavailable these days as I was on vacation. I'm glad to see this issue is being looked at again. I'll be happy to contribute again if needed @enavarro51 .

enavarro51 · 2021-08-10T14:03:36Z

@jsistos Glad to see you back. I've basically just cleared up the merge conflicts from #6061 and I believe everything is running properly. The one remaining issue is what I mentioned in my comment to @TharrmashasthaPV above. There was a major cleanup of the header code for the latex drawer in #6483 related to issue #6447. So I think we are trying to keep anything out of the header other than the \usepackage items. Is there a way to adjust your code so that we don't have \newlength in the header?

If it's a simple fix, just let me know and I can finish it off here, or you can take it from here. Once it's finished, I'll let @javabster know and we'll see if we can get a quick approval. Thanks a lot.

TharrmashasthaPV

@TharrmashasthaPV I was wondering if you could have a look at this PR. It was started by @jsistos and I'm finishing it off. It accomplishes the goal, but you commented we are trying to eliminate adding header info in latex source. So is there a way to accomplish the same thing without the added \newlength? Thanks.

@enavarro51 I think we might have a hack for that.

TharrmashasthaPV · 2021-08-11T03:57:55Z

qiskit/visualization/latex.py

+        self._latex[wire_max][col] += (
+            " \\cds{-1}{\\settowidth{\\glen}{\\ensuremath{%s}}"
+            " \\hspace{0.5em}\\hspace{\\glen}\\ensuremath{%s}}" % (gate_text, gate_text)
+        )


As far as I could check, I think \hphantom latex command also creates empty space as required. So I think on replacing lines 533-536 by

self._latex[wire_max][col] += ( " \\cds{-1}{\hspace{0.5em}\hphantom{\\ensuremath{%s}}" "\\ensuremath{%s}}" % (gate_text, gate_text) )

we should still get the required spacing correctly.

TharrmashasthaPV · 2021-08-11T04:17:04Z

qiskit/visualization/latex.py

@@ -328,7 +330,7 @@ def _get_image_depth(self):
                # which take 4 columns
                base_type = None if not hasattr(op, "base_gate") else op.base_gate
                if isinstance(op, RZZGate) or isinstance(base_type, (U1Gate, PhaseGate, RZZGate)):
-                    column_width = 4
+                    column_width = 5


I was just trying out some gates. I feel in the below circuit the gap between the first and the second layer of gates is large. I think this trouble arises whenever there is a large gate in the same layer as that of the symmetric gate.

from math import pi qc = QuantumCircuit(4) qc.u3(0.23123414,0.1223412343,0.3412342345,0) qc.cu1(0.123456,1,2) qc.rzz(pi,3,1) qc.cp(pi/3,0,1) qc.swap(0,1) qc.draw('latex)

Do you think we can we do something about this?

We could go back to 4 columns, but then the ZZ gate may overlap with the next layer of gates in case there aren't wider gates giving some extra space. (I'm not entirely sure, but I think ZZ is the only gate with such issue).

As @enavarro51 said in #6061, changing the number of columns based on the gate text may add unnecessary complexness. My take had been trying to use a smaller font for the labels, but I was unable to figure it out.

Although, if only RZZ allows for the overlap to happen, then maybe we could just split the column counts (5 for RZZ, 4 otherwise). The issue would still remain for RZZ gates in the same layer as other (large enough) gates, though.

I think this is a bit of a messy problem. I agree with @TharrmashasthaPV that the spacing is too wide. The problem is that qcircuit takes care of spacing for gates, but doesn't know what to do with this arbitrary side text, so it deals with it as if it is just the 2 bullets and a vertical line in terms of horizontal space.

There's a simple (kind of) solution which is just to restrict the floats to 1 decimal and lower case ZZ (which is what it used to be), then set cols to increment by 3 instead of 5. Not perfect, but maybe acceptable?

But then you run into this.

Ultimately this should work the same as gates, where the sidetext is a special gate that expands the space to the next column based on the width of the text. Maybe need a \newcommand?

maybe \phantom could help to normalize the space?

Forcing the amount of decimals sounds like a too drastic solution to me.

I've been thinking about this issue a fair bit, tried using \phantom but that didn't seem to resolve much. I think we need a way to dynamically change the number of columns depending on the length of the gate_text. When the actual gate is drawn, is it always placed in the centre of the columns? If yes then as we increase the number of columns we will also inadvertently be increasing the space on the left of the gate. Maybe another solution would be to have gates with side text not placed in the centre of the columns but on the left hand side.

1ucian0 · 2022-01-31T21:21:43Z

Hmm... this one got too deep in my todo-list. Does anybody now the situation here?

enavarro51 · 2022-01-31T21:38:59Z

Hey, @1ucian0. I believe it is as it appears to be - stuck. Everybody is in agreement on what the problem is, but so far none of the solutions works for all cases -

wide gate in same layer with small sidetext and other layers to the right
wide sidetext in same layer with narrow gates and other layers to the right

Maybe there's a LaTex whiz out there who has some new ideas.

coveralls · 2022-01-31T21:59:06Z

Pull Request Test Coverage Report for Build 1774968902

3 of 3 (100.0%) changed or added relevant lines in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 83.154%

Totals
Change from base Build 1774688493:	0.0%
Covered Lines:	51908
Relevant Lines:	62424

💛 - Coveralls

1ucian0 · 2022-02-04T17:20:24Z

Thanks for the update! The use of \phantom{gate_text} should be that "way to dynamically change the number of columns depending on the length of the gate_text" that @javabster was searching for. See https://www.tutorialspoint.com/tex_commands/phantom.htm

jakelishman · 2023-03-06T15:20:45Z

This is rather old sorry, and I'm obviously missing the full context, but in general I would expect the \phantom-based solution to this to require "drawing" the gate label twice - once normally as side-text, and once within the "gate text" field that qcircuit expects, with the latter wrapped in \phantom and forced-left-aligned.

jsistos and others added 16 commits March 19, 2021 18:08

Fixed symmetrical gates bug in latex drawer

f114213

Merge branch 'master' of https://github.com/Qiskit/qiskit-terra into …

c1d97ad

…fix6050

fixed tests with symmetrical gates

485120b

Merge branch 'master' into fix6050

9727a5a

Added variable hspace to latex output

1c1f580

Merge branch 'master' of https://github.com/Qiskit/qiskit-terra into …

440f190

…fix6050

Merge branch 'fix6050' of https://github.com/jsistos/qiskit-terra int…

ed9b4ba

…o fix6050

fixed lint

cf58afd

Updated number of symgate columns to 5

7b98474

Merge branch 'master' into fix6050

43fcbed

Merge branch 'master' into fix6050

2d95216

Merge branch 'master' into fix6050

359ebb7

Merge branch 'master' into fix6050

e16ef50

Merge branch 'master' into fix6050

795491a

Fix pull conflicts

32b5bca

Fix ref images

8cb5d83

enavarro51 requested review from nkanazawa1989, nonhermitian and a team as code owners August 5, 2021 15:52

Lint

207bc43

TharrmashasthaPV reviewed Aug 11, 2021

View reviewed changes

enavarro51 and others added 4 commits August 19, 2021 13:06

Fix latex merge conflicts

d1b8026

Lint

8172c4a

Fix merge conflict

5e7d0df

Merge branch 'main' into fix6050_2

831921c

enavarro51 mentioned this pull request Feb 15, 2022

Classical conditions misalign in both ASCII and MPL output #7532

Closed

HuangJunye added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Jun 21, 2022

jakelishman mentioned this pull request Mar 6, 2023

Latex drawer now graphs symmetrical gates and their names together #6061

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix latex drawer symmetrical gates display #6877

Fix latex drawer symmetrical gates display #6877

enavarro51 commented Aug 5, 2021

enavarro51 commented Aug 9, 2021

jsistos commented Aug 10, 2021

enavarro51 commented Aug 10, 2021

TharrmashasthaPV left a comment

TharrmashasthaPV Aug 11, 2021

TharrmashasthaPV Aug 11, 2021

jsistos Aug 11, 2021

jsistos Aug 12, 2021

enavarro51 Aug 14, 2021

1ucian0 Sep 29, 2021

javabster Oct 8, 2021

1ucian0 commented Jan 31, 2022

enavarro51 commented Jan 31, 2022

coveralls commented Jan 31, 2022

1ucian0 commented Feb 4, 2022

jakelishman commented Mar 6, 2023

Fix latex drawer symmetrical gates display #6877

Are you sure you want to change the base?

Fix latex drawer symmetrical gates display #6877

Conversation

enavarro51 commented Aug 5, 2021

Summary

Details and comments

enavarro51 commented Aug 9, 2021

jsistos commented Aug 10, 2021

enavarro51 commented Aug 10, 2021

TharrmashasthaPV left a comment

Choose a reason for hiding this comment

TharrmashasthaPV Aug 11, 2021

Choose a reason for hiding this comment

TharrmashasthaPV Aug 11, 2021

Choose a reason for hiding this comment

jsistos Aug 11, 2021

Choose a reason for hiding this comment

jsistos Aug 12, 2021

Choose a reason for hiding this comment

enavarro51 Aug 14, 2021

Choose a reason for hiding this comment

1ucian0 Sep 29, 2021

Choose a reason for hiding this comment

javabster Oct 8, 2021

Choose a reason for hiding this comment

1ucian0 commented Jan 31, 2022

enavarro51 commented Jan 31, 2022

coveralls commented Jan 31, 2022

Pull Request Test Coverage Report for Build 1774968902

💛 - Coveralls

1ucian0 commented Feb 4, 2022

jakelishman commented Mar 6, 2023