Move GateTabulation and friends to cirq-core as requested in #4461 #4602

Ashalynd · 2021-10-27T18:50:25Z

cirq-google/cirq_google/__init__.py

cirq-core/cirq/optimizers/two_qubit_gate_compilation.py

tanujkhattar · 2021-11-17T20:06:42Z

@Ashalynd My work on adding approximate compilation method (described in #4362) is now blocked on getting this PR checked in. Would you be available to continue working on this, so we can get it in soon? Thanks!

Ashalynd · 2021-11-17T20:09:47Z

@Ashalynd My work on adding approximate compilation method (described in #4362) is now blocked on getting this PR checked in. Would you be available to continue working on this, so we can get it in soon? Thanks!

Yes, looking at finishing that right now.
Heads-up: I checked in what I have atm, mostly works but :

there was name clash between deprecated and new versions, so I renamed the new ones
not all tests seem to play nicely with deprecated classes.
saw some complaints about coverage (not sure if reproduces on github).

cirq-core/cirq/optimizers/two_qubit_gate_compilation.py

cirq-google/cirq_google/json_resolver_cache.py

cirq-google/cirq_google/optimizers/two_qubit_gates/gate_compilation.py

cirq-core/cirq/optimizers/two_qubit_gate_compilation.py

cirq-google/cirq_google/optimizers/convert_to_sycamore_gates.py

cirq-core/cirq/optimizers/two_qubit_gate_compilation.py

cirq-core/cirq/optimizers/two_qubit_gate_math_utils_test.py

tanujkhattar

Looking good. Left a final round of comments and we should be ready to merge after this. Thanks for the iterations!

cirq-google/cirq_google/optimizers/two_qubit_gates/gate_compilation.py

cirq-google/cirq_google/optimizers/two_qubit_gates/gate_compilation_test.py

cirq-core/cirq/transformers/heuristic_decompositions/__init__.py

cirq-core/cirq/transformers/__init__.py

tanujkhattar · 2022-01-05T21:33:24Z

cirq-core/cirq/protocols/json_test_data/TwoQubitGateTabulation.repr_inward

@@ -0,0 +1 @@
+cirq.TwoQubitGateTabulation(np.array([[(1+0j), 0j, 0j, 0j], [0j, (6.123233995736766e-17+0j), -1j, 0j], [0j, -1j, (6.123233995736766e-17+0j), 0j], [0j, 0j, 0j, (0.8660254037844387-0.49999999999999994j)]], dtype=np.complex128), np.array([[0.7853981633974483, 0.7853981633974483, 0.1308996938995748], [0.5455546451806152, 0.4274250500660468, 1.1102230246251565e-16], [0.482263980185593, 0.15541244226118112, -0.0], [0.08994436657892213, 0.0688663429298958, -0.0], [0.5337019863142523, 0.31800967733278185, -0.2565859360625904], [0.48438278888761155, 0.39121774851710633, 0.28728266960007875]], dtype=np.float64), [[],[(np.array([[(-0.8113361037323902+0.388228151720065j), (0.38855300559639544-0.20009795309891054j)], [(-0.38855300559639544-0.20009795309891054j), (-0.8113361037323902-0.388228151720065j)]], dtype=np.complex128), np.array([[(-0.15500592487333173-0.5209557609876208j), (-0.6259723589563565+0.5592288119996914j)], [(0.6259723589563565+0.5592288119996914j), (-0.15500592487333173+0.5209557609876208j)]], dtype=np.complex128))],[(np.array([[(-0.7591335658137299-0.5262321684934362j), (0.37132129473675946+0.09442685090928131j)], [(-0.37132129473675946+0.09442685090928131j), (-0.7591335658137299+0.5262321684934362j)]], dtype=np.complex128), np.array([[(0.004722472920167423+0.9809944588837683j), (0.18002103957975657+0.07224953423714474j)], [(-0.18002103957975657+0.07224953423714474j), (0.004722472920167423-0.9809944588837683j)]], dtype=np.complex128))],[(np.array([[(-0.0897362442086916+0.02445105905520244j), (-0.8503296766825128-0.5179662084918382j)], [(0.8503296766825128-0.5179662084918382j), (-0.0897362442086916-0.02445105905520244j)]], dtype=np.complex128), np.array([[(0.7441127742234152+0.6642425418602642j), (0.009545063892332065+0.07061810374004159j)], [(-0.009545063892332065+0.07061810374004159j), (0.7441127742234152-0.6642425418602642j)]], dtype=np.complex128))],[(np.array([[(-0.8113361037323902+0.388228151720065j), (0.38855300559639544-0.20009795309891054j)], [(-0.38855300559639544-0.20009795309891054j), (-0.8113361037323902-0.388228151720065j)]], dtype=np.complex128), np.array([[(-0.15500592487333173-0.5209557609876208j), (-0.6259723589563565+0.5592288119996914j)], [(0.6259723589563565+0.5592288119996914j), (-0.15500592487333173+0.5209557609876208j)]], dtype=np.complex128)),(np.array([[(-0.8113361037323902+0.388228151720065j), (0.38855300559639544-0.20009795309891054j)], [(-0.38855300559639544-0.20009795309891054j), (-0.8113361037323902-0.388228151720065j)]], dtype=np.complex128), np.array([[(-0.15500592487333173-0.5209557609876208j), (-0.6259723589563565+0.5592288119996914j)], [(0.6259723589563565+0.5592288119996914j), (-0.15500592487333173+0.5209557609876208j)]], dtype=np.complex128))],[(np.array([[(0.41295648139511504-0.17280776715854063j), (0.5430881450957462-0.7103940362502399j)], [(-0.5430881450957462-0.7103940362502399j), (0.41295648139511504+0.17280776715854063j)]], dtype=np.complex128), np.array([[(0.19013479709876246+0.4941656851052965j), (-0.6469746262664201+0.5487010730480224j)], [(0.6469746262664201+0.5487010730480224j), (0.19013479709876246-0.4941656851052965j)]], dtype=np.complex128)),(np.array([[(0.41295648139511504-0.17280776715854063j), (0.5430881450957462-0.7103940362502399j)], [(-0.5430881450957462-0.7103940362502399j), (0.41295648139511504+0.17280776715854063j)]], dtype=np.complex128), np.array([[(0.19013479709876246+0.4941656851052965j), (-0.6469746262664201+0.5487010730480224j)], [(0.6469746262664201+0.5487010730480224j), (0.19013479709876246-0.4941656851052965j)]], dtype=np.complex128))]],  0.49, 'Fraction of Weyl chamber reached with 2 gates: 0.600\nFraction of Weyl chamber reached with 2 gates and 3 gates(same single qubit): 1.000', ())


Why are the _inward files needed here? IIUC, Both TwoQubitGateTabulation.repr_inward and TwoQubitGateTabulation.json_inward are not needed here right now and should be removed.

The _inward files would need to be added to cirq-google when we remove the deprecated classes after the deprecation cycle is over so that we can continue to support deserialization of the old GateTabulation type into the newly introduced TwoQubitGateTabulation type.

See https://quantumai.google/cirq/dev/serialization#removing_a_serializable_value for more details.

I see, removed the files from the new directory. For some reason, the cirq-google/cirq_google/json_test_data contains identical .repr / .json and .repr_inward / .json_inward for this and many other classes, which is quite confusing. Should it be cleaned up?

cirq-core/cirq/protocols/json_serialization_test.py

…tests for GateTabulation, they might not be handling deprecation correctly; remove extra import

tanujkhattar · 2022-01-10T11:04:48Z

cirq-core/cirq/transformers/heuristic_decompositions/two_qubit_gate_tabulation.py

+    )
+
+
+# pylint: enable=missing-raises-doc


Now that we have the copyright in place, this line is not needed. Please delete.

tanujkhattar

LGTM % two final nits. Please fix the nits and we'll merge.

Thanks a lot for all the iterations! The final outcome looks great :)

tanujkhattar · 2022-01-10T11:37:59Z

@Ashalynd Please fix the lint checks

Ashalynd · 2022-01-10T15:16:57Z

@Ashalynd Please fix the lint checks

done

…ib#4461 (quantumlib#4602) - Adds `cirq. TwoQubitGateTabulation`, `cirq.TwoQubitGateTabulationResult`, and `cirq.two_qubit_gate_product_tabulation` public top level objects to Cirq. - TwoQubitGateTabulation is heuristic compilation technique for decomposing any 2q unitary to a given target 2q gate + single qubit rotations.

Ashalynd added 3 commits October 27, 2021 20:40

Moved GateTabulation and friends to cirq-core as requested in quantum…

8d7f5b6

…lib#4461

obligatory followup fix for typo

46a7ede

one more typo fix

8e8577b

Ashalynd requested review from cduck, vtomole, wcourtney and a team as code owners October 27, 2021 18:50

Ashalynd requested a review from tanujkhattar October 27, 2021 18:50

google-cla bot added the cla: yes Makes googlebot stop complaining. label Oct 27, 2021

CirqBot added the size: M 50< lines changed <250 label Oct 27, 2021

Merge branch 'master' into gatetab

bd8fc86

tanujkhattar requested changes Oct 27, 2021

View reviewed changes

cirq-google/cirq_google/__init__.py Outdated Show resolved Hide resolved

cirq-core/cirq/optimizers/two_qubit_gate_compilation.py Outdated Show resolved Hide resolved

MichaelBroughton assigned tanujkhattar Oct 29, 2021

Ashalynd added 2 commits November 10, 2021 19:04

Merge branch 'master' into gatetab

5215976

added back deprecated classes for the time being

decbfc6

Ashalynd added 10 commits November 17, 2021 23:07

working on errors

fa86a08

merged master

79953c2

deleted by mistake

fa0788e

made tests pass

209f4a5

mostly works

94823e2

Merge branch 'master' into gatetab

2344258

formatting

3323f20

removed now-unreachable clause from test

3d7b229

lint fix

bc5b5c4

Merge branch 'master' into gatetab

1950632

Ashalynd requested a review from tanujkhattar November 18, 2021 15:01

Ashalynd added the Ready for Re-Review For when reviewers take their time. label Nov 18, 2021

tanujkhattar requested changes Nov 19, 2021

View reviewed changes

Ashalynd added 3 commits January 5, 2022 20:39

updated tests for "old" gate_compilcation

479814a

removed unused import

3a8d411

removed another unused import

542614e

tanujkhattar requested changes Jan 5, 2022

View reviewed changes

tanujkhattar removed the Ready for Re-Review For when reviewers take their time. label Jan 5, 2022

Ashalynd added 5 commits January 5, 2022 23:33

one more round of changes

31ee180

formatting

38dce7e

removed unused import

b7ef60f

Merge branch 'master' into gatetab

9b60602

Merge branch 'master' into gatetab

fd6fa49

Ashalynd added the Ready for Re-Review For when reviewers take their time. label Jan 5, 2022

Ashalynd requested a review from tanujkhattar January 5, 2022 23:15

tanujkhattar reviewed Jan 6, 2022

View reviewed changes

cirq-core/cirq/protocols/json_serialization_test.py Outdated Show resolved Hide resolved

Ashalynd added 4 commits January 7, 2022 23:53

Merge branch 'master' into gatetab

54a5a41

restore changes in json_serialization_test.py; disable serialization …

65307cc

…tests for GateTabulation, they might not be handling deprecation correctly; remove extra import

reenabled serialization test for GateTabulation

581bfaa

Merge branch 'master' into gatetab

4b3da36

Ashalynd requested a review from tanujkhattar January 8, 2022 02:11

tanujkhattar reviewed Jan 10, 2022

View reviewed changes

tanujkhattar approved these changes Jan 10, 2022

View reviewed changes

Ashalynd added 2 commits January 10, 2022 12:24

addressed the reviewer's comments

a314689

Merge branch 'gatetab' of github.com:Ashalynd/Cirq into gatetab

fb73754

tanujkhattar removed the Ready for Re-Review For when reviewers take their time. label Jan 10, 2022

made linter happy

8232505

tanujkhattar merged commit 778d317 into quantumlib:master Jan 10, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move GateTabulation and friends to cirq-core as requested in #4461 #4602

Move GateTabulation and friends to cirq-core as requested in #4461 #4602

Ashalynd commented Oct 27, 2021 •

edited by tanujkhattar

Loading

tanujkhattar commented Nov 17, 2021

Ashalynd commented Nov 17, 2021 •

edited

Loading

tanujkhattar left a comment

tanujkhattar Jan 5, 2022

Ashalynd Jan 5, 2022

tanujkhattar Jan 10, 2022

Ashalynd Jan 10, 2022

tanujkhattar left a comment

tanujkhattar commented Jan 10, 2022

Ashalynd commented Jan 10, 2022

		@@ -0,0 +1 @@
		cirq.TwoQubitGateTabulation(np.array([[(1+0j), 0j, 0j, 0j], [0j, (6.123233995736766e-17+0j), -1j, 0j], [0j, -1j, (6.123233995736766e-17+0j), 0j], [0j, 0j, 0j, (0.8660254037844387-0.49999999999999994j)]], dtype=np.complex128), np.array([[0.7853981633974483, 0.7853981633974483, 0.1308996938995748], [0.5455546451806152, 0.4274250500660468, 1.1102230246251565e-16], [0.482263980185593, 0.15541244226118112, -0.0], [0.08994436657892213, 0.0688663429298958, -0.0], [0.5337019863142523, 0.31800967733278185, -0.2565859360625904], [0.48438278888761155, 0.39121774851710633, 0.28728266960007875]], dtype=np.float64), [[],[(np.array([[(-0.8113361037323902+0.388228151720065j), (0.38855300559639544-0.20009795309891054j)], [(-0.38855300559639544-0.20009795309891054j), (-0.8113361037323902-0.388228151720065j)]], dtype=np.complex128), np.array([[(-0.15500592487333173-0.5209557609876208j), (-0.6259723589563565+0.5592288119996914j)], [(0.6259723589563565+0.5592288119996914j), (-0.15500592487333173+0.5209557609876208j)]], dtype=np.complex128))],[(np.array([[(-0.7591335658137299-0.5262321684934362j), (0.37132129473675946+0.09442685090928131j)], [(-0.37132129473675946+0.09442685090928131j), (-0.7591335658137299+0.5262321684934362j)]], dtype=np.complex128), np.array([[(0.004722472920167423+0.9809944588837683j), (0.18002103957975657+0.07224953423714474j)], [(-0.18002103957975657+0.07224953423714474j), (0.004722472920167423-0.9809944588837683j)]], dtype=np.complex128))],[(np.array([[(-0.0897362442086916+0.02445105905520244j), (-0.8503296766825128-0.5179662084918382j)], [(0.8503296766825128-0.5179662084918382j), (-0.0897362442086916-0.02445105905520244j)]], dtype=np.complex128), np.array([[(0.7441127742234152+0.6642425418602642j), (0.009545063892332065+0.07061810374004159j)], [(-0.009545063892332065+0.07061810374004159j), (0.7441127742234152-0.6642425418602642j)]], dtype=np.complex128))],[(np.array([[(-0.8113361037323902+0.388228151720065j), (0.38855300559639544-0.20009795309891054j)], [(-0.38855300559639544-0.20009795309891054j), (-0.8113361037323902-0.388228151720065j)]], dtype=np.complex128), np.array([[(-0.15500592487333173-0.5209557609876208j), (-0.6259723589563565+0.5592288119996914j)], [(0.6259723589563565+0.5592288119996914j), (-0.15500592487333173+0.5209557609876208j)]], dtype=np.complex128)),(np.array([[(-0.8113361037323902+0.388228151720065j), (0.38855300559639544-0.20009795309891054j)], [(-0.38855300559639544-0.20009795309891054j), (-0.8113361037323902-0.388228151720065j)]], dtype=np.complex128), np.array([[(-0.15500592487333173-0.5209557609876208j), (-0.6259723589563565+0.5592288119996914j)], [(0.6259723589563565+0.5592288119996914j), (-0.15500592487333173+0.5209557609876208j)]], dtype=np.complex128))],[(np.array([[(0.41295648139511504-0.17280776715854063j), (0.5430881450957462-0.7103940362502399j)], [(-0.5430881450957462-0.7103940362502399j), (0.41295648139511504+0.17280776715854063j)]], dtype=np.complex128), np.array([[(0.19013479709876246+0.4941656851052965j), (-0.6469746262664201+0.5487010730480224j)], [(0.6469746262664201+0.5487010730480224j), (0.19013479709876246-0.4941656851052965j)]], dtype=np.complex128)),(np.array([[(0.41295648139511504-0.17280776715854063j), (0.5430881450957462-0.7103940362502399j)], [(-0.5430881450957462-0.7103940362502399j), (0.41295648139511504+0.17280776715854063j)]], dtype=np.complex128), np.array([[(0.19013479709876246+0.4941656851052965j), (-0.6469746262664201+0.5487010730480224j)], [(0.6469746262664201+0.5487010730480224j), (0.19013479709876246-0.4941656851052965j)]], dtype=np.complex128))]], 0.49, 'Fraction of Weyl chamber reached with 2 gates: 0.600\nFraction of Weyl chamber reached with 2 gates and 3 gates(same single qubit): 1.000', ())

Move GateTabulation and friends to cirq-core as requested in #4461 #4602

Move GateTabulation and friends to cirq-core as requested in #4461 #4602

Conversation

Ashalynd commented Oct 27, 2021 • edited by tanujkhattar Loading

tanujkhattar commented Nov 17, 2021

Ashalynd commented Nov 17, 2021 • edited Loading

tanujkhattar left a comment

Choose a reason for hiding this comment

tanujkhattar Jan 5, 2022

Choose a reason for hiding this comment

Ashalynd Jan 5, 2022

Choose a reason for hiding this comment

tanujkhattar Jan 10, 2022

Choose a reason for hiding this comment

Ashalynd Jan 10, 2022

Choose a reason for hiding this comment

tanujkhattar left a comment

Choose a reason for hiding this comment

tanujkhattar commented Jan 10, 2022

Ashalynd commented Jan 10, 2022

Ashalynd commented Oct 27, 2021 •

edited by tanujkhattar

Loading

Ashalynd commented Nov 17, 2021 •

edited

Loading