Latex drawer tested using notebook in binder #6450

TharrmashasthaPV · 2021-05-21T04:55:46Z

Summary

Fixes #6371 .

Details and comments

Following #4544 , this PR enable testing Latex drawer using notebook in Binder. The flow of the testing follows exactly from #4544 . The binder of this PR can be accessed at https://mybinder.org/v2/gh/TharrmashasthaPV/qiskit-terra/binder_test_branch?filepath=test%2Fipynb%2Flatex_tester.ipynb. The release notes have been added.

Files added/modified

Renamed test/ipynb/mpl_tester.ipynb to test/iplynb/snapshot_tester.ipynb - The notebook that will run in Binder that includes tests for both mpl and latex drawers.
Added apt.txt - This file installs the required latex dependencies.
Modified postBuild - This file now also collect and installs the latex qcircuit.sty package.
Added test/ipynb/latex/references - This folder contains all the reference images.
Added test/ipynb/latex/test_circuit_latex_drawer.py - The file that contains all the latex tests.

…t-terra into issue6371

1ucian0 · 2021-08-04T11:54:48Z

test/ipynb/snapshot_tester.ipynb

@@ -3,35 +3,61 @@
  {
   "cell_type": "code",
   "execution_count": null,
-   "id": "fc250437",


It is possible to "clean" the notebook with nb-clean. In this way, we can keep the minimum file in the repo.

Yeah, sure. I'll do that 😃

1ucian0

Awesome work.

For a following PR, would be possible to have a py version of snapshot_tester.ipynb? With #6427 , CI could run snapshot_tester.py. What do you think?

1ucian0 · 2021-08-04T12:13:19Z

CONTRIBUTING.md

 new tests where necessary. You can do this as follows:

 1. Make sure you have pushed your latest changes to your remote branch.
-2. Go to link: `https://mybinder.org/v2/gh/<github_user>/<repo>/<branch>?urlpath=apps/test/ipynb/mpl_tester.ipynb`. For example, if your GitHub username is `username`, your forked repo has the same name the original, and your branch is `my_awesome_new_feature`, you should visit https://mybinder.org/v2/gh/username/qiskit-terra/my_awesome_new_feature?urlpath=apps/test/ipynb/mpl_tester.ipynb.
+2. Go to link: `https://mybinder.org/v2/gh/<github_user>/<repo>/<branch>?urlpath=apps/test/ipynb/snapshot_tester.ipynb`. For example, if your GitHub username is `username`, your forked repo has the same name the original, and your branch is `my_awesome_new_feature`, you should visit https://mybinder.org/v2/gh/username/qiskit-terra/my_awesome_new_feature?urlpath=apps/test/ipynb/snapshot_tester.ipynb


I was not able to run https://mybinder.org/v2/gh/TharrmashasthaPV/qiskit-terra/binder_test_branch?filepath=apps/test/ipynb/snapshot_tester.ipynb . Maybe binder is in a bad day? Maybe I'm doing something wrong?

There seems to be a small error in the link you tried. 😃 We use either ...filepath=test/ipynb/snapshot_tester.py or ...urlpath=apps/test/ipynb/snapshot_tester.py in the link. One correct link is https://mybinder.org/v2/gh/TharrmashasthaPV/qiskit-terra/binder_test_branch?urlpath=apps/test/ipynb/snapshot_tester.ipynb. This link seems to be working for me.

I also was struggling to run them a few days ago. I did manage to run them in the end but they were very slow. Also wasn't sure if it was a binder issue, or maybe something to do with the changes here

I just tested running branches with and without the change of this PR and I kind of found them both to take similar duration (around 3 mins) to open a server for the first time a branch is run after a commit. So, I think it is a binder issue.

TharrmashasthaPV · 2021-08-06T08:04:13Z

Awesome work.

For a following PR, would be possible to have a py version of snapshot_tester.ipynb? With #6427 , CI could run snapshot_tester.py. What do you think?

Thanks! I think currently we are just saving the output images at
https://github.com/Qiskit/qiskit-terra/blob/0cdacec2ee3b26a96b1d7bbb0def6b7eb66ac5b4/azure-pipelines.yml#L255-L256
but not comparing them in CI. I think it certainly would be better to have comparison in CI and so I too feel we should have a snapshot_tester.py. I'll be sure to push it in a following PR.

…aPV/qiskit-terra into binder_test_branch

…nder_test_branch

javabster

LGTM thanks Tharrma! 🚀

1ucian0 · 2021-11-17T13:15:46Z

postBuild

@@ -11,6 +11,10 @@
 pip install matplotlib pylatexenc pillow appmode seaborn
 pip install .

+# Install QCircuit for LaTeX drawer
+mkdir -p $(kpsewhich -var-value=TEXMFHOME)/tex/latex/qcircuit/
+wget https://mirrors.ctan.org/graphics/qcircuit/qcircuit.sty -P $(kpsewhich -var-value=TEXMFHOME)/tex/latex/qcircuit/


Suggested change

wget https://mirrors.ctan.org/graphics/qcircuit/qcircuit.sty -P $(kpsewhich -var-value=TEXMFHOME)/tex/latex/qcircuit/

tlmgr install qcircuit

1ucian0 · 2021-11-17T13:18:09Z

test/ipynb/mpl/__init__.py

@@ -10,7 +10,7 @@
 # copyright notice, and modified files need to carry a notice indicating
 # that they have been altered from the originals.

-"""Image comparison test for MPL graph drawer.
+"""Image comparison test for MPL drawer.


Suggested change

"""Image comparison test for MPL drawer.

"""Image comparison test for MPL circuit drawer.

I think just MPL drawer is correct, as it tests both the graphs and the circuits, then each subfolder has it's own __init.py__ where it specifies if it's testing the graphs or circuits

In that case...

Suggested change

"""Image comparison test for MPL drawer.

"""Image comparison test for MPL drawers.

coveralls · 2021-11-17T13:43:18Z

Pull Request Test Coverage Report for Build 1493871580

0 of 1 (0.0%) changed or added relevant line in 1 file are covered.
667 unchanged lines in 34 files lost coverage.
Overall coverage decreased (-0.4%) to 82.099%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/visualization/utils.py	0	1	0.0%

Files with Coverage Reduction	New Missed Lines	%
qiskit/dagcircuit/dagnode.py	1	76.35%
qiskit/pulse/utils.py	1	97.37%
qiskit/transpiler/passes/utils/check_map.py	1	95.24%
qiskit/visualization/qcstyle.py	1	34.62%
qiskit/circuit/quantumcircuit.py	2	94.19%
qiskit/pulse/library/waveform.py	2	95.74%
qiskit/transpiler/passes/layout/vf2_layout.py	2	98.08%
qiskit/transpiler/passes/routing/stochastic_swap.py	3	96.58%
qiskit/circuit/tools/pi_check.py	4	87.04%
qiskit/transpiler/passes/utils/gate_direction.py	4	80.0%

Totals
Change from base Build 1468573388:	-0.4%
Covered Lines:	49421
Relevant Lines:	60197

💛 - Coveralls

jakelishman · 2023-03-06T15:25:04Z

In principle, despite its age, this PR still looks mostly appropriate to me, and not too far off merging. I suspect that the conflicts are mostly a minor change in this PR conflicting with another minor styling change in the LaTeX output since this PR was opened.

@1ucian0 - you assigned yourself before. Can you finish off driving this over the line? If you write the small changes you think were required, I can do the final review.

1ucian0 · 2023-04-25T16:37:57Z

This can probably be closed as we are moving to CI visual testing in #9961

TharrmashasthaPV and others added 30 commits May 7, 2021 03:11

Added binder tests for latex

20fdbd2

path correction

88ff31f

Minor fix

6fb992e

Minor fix 2

ea0899f

Fixed minor error 3

0a8d042

minor error fix 4

bb24b28

minor error fix 5

c2a149b

minor error fix 5

8ad11e1

error fix 6

589c237

error fix 7

2567802

error fix 8

0fe9ae1

error fix 9

5238f98

Fix 10

37042d4

Fix 11

f9c1ac3

Added apt.txt

764be02

Merge branch 'issue6371' of https://github.com/TharrmashasthaPV/qiski…

7289ce2

…t-terra into issue6371

Update apt.txt

3ae7f24

updated apt.txt 2

cc9971b

Updated apt.txt again

d727933

Updating apt-txt trial 3

f7b027b

Testing with apt.txt

23a8bbc

updated postbuild 1

86ce4ba

Create envirenment.yml

1b367dd

updated env.yml

f499859

updated apt

3defef9

update apt again

82a4681

Update postBuild

0593da4

Update postbuild1

af39dd7

update postbuild2

67d31d6

new postbuild

6eaad0c

1ucian0 reviewed Aug 4, 2021

View reviewed changes

removed reno, envionment.yml and cleaned ipynb

6b660bf

TharrmashasthaPV dismissed javabster’s stale review via 6b660bf August 6, 2021 07:42

TharrmashasthaPV and others added 9 commits August 6, 2021 13:34

Merge branch 'main' into binder_test_branch

c1cf62e

small fix

525fbe8

Merge branch 'main' into binder_test_branch

3500d61

Resolved conflicts

80c465b

nb-clean

0cd29f6

Merge branch 'main' into binder_test_branch

ea8b3c0

Added missing images

bd336d2

Merge branch 'binder_test_branch' of https://github.com/Tharrmashasth…

3bf7058

…aPV/qiskit-terra into binder_test_branch

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

dd837f9

…nder_test_branch

javabster previously approved these changes Oct 18, 2021

View reviewed changes

javabster added the Changelog: New Feature Include in the "Added" section of the changelog label Oct 18, 2021

1ucian0 added Changelog: None Do not include in changelog and removed Changelog: New Feature Include in the "Added" section of the changelog labels Oct 18, 2021

Merge branch 'main' into binder_test_branch

a2f3cdc

1ucian0 reviewed Nov 17, 2021

View reviewed changes

Tidied up package installs

b51d07e

javabster dismissed their stale review via b51d07e November 23, 2021 08:25

1ucian0 added the Community PR PRs from contributors that are not 'members' of the Qiskit repo label Jun 14, 2022

1ucian0 self-assigned this Jun 14, 2022

1ucian0 closed this Apr 25, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Latex drawer tested using notebook in binder #6450

Latex drawer tested using notebook in binder #6450

TharrmashasthaPV commented May 21, 2021 •

edited

Loading

1ucian0 Aug 4, 2021

TharrmashasthaPV Aug 6, 2021

1ucian0 left a comment •

edited

Loading

1ucian0 Aug 4, 2021

TharrmashasthaPV Aug 6, 2021

javabster Aug 6, 2021 •

edited

Loading

TharrmashasthaPV Aug 6, 2021

TharrmashasthaPV commented Aug 6, 2021

javabster left a comment

1ucian0 Nov 17, 2021

1ucian0 Nov 17, 2021

javabster Nov 22, 2021

1ucian0 Dec 9, 2021

coveralls commented Nov 17, 2021 •

edited

Loading

jakelishman commented Mar 6, 2023

1ucian0 commented Apr 25, 2023

	wget https://mirrors.ctan.org/graphics/qcircuit/qcircuit.sty -P $(kpsewhich -var-value=TEXMFHOME)/tex/latex/qcircuit/
	tlmgr install qcircuit

	"""Image comparison test for MPL drawer.
	"""Image comparison test for MPL circuit drawer.

	"""Image comparison test for MPL drawer.
	"""Image comparison test for MPL drawers.

Latex drawer tested using notebook in binder #6450

Latex drawer tested using notebook in binder #6450

Conversation

TharrmashasthaPV commented May 21, 2021 • edited Loading

Summary

Details and comments

Files added/modified

Choose a reason for hiding this comment

Choose a reason for hiding this comment

1ucian0 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javabster Aug 6, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

TharrmashasthaPV commented Aug 6, 2021

javabster 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

coveralls commented Nov 17, 2021 • edited Loading

Pull Request Test Coverage Report for Build 1493871580

💛 - Coveralls

jakelishman commented Mar 6, 2023

1ucian0 commented Apr 25, 2023

TharrmashasthaPV commented May 21, 2021 •

edited

Loading

1ucian0 left a comment •

edited

Loading

javabster Aug 6, 2021 •

edited

Loading

coveralls commented Nov 17, 2021 •

edited

Loading