GoogleCZTargetGateset #5744

verult · 2022-07-13T00:19:12Z

Added GoogleCZTargetGateset to include postprocess transformers which are previously supported by optimize_for_sycamore, and to allow for future customizations of the CZ target gateset.
- At this time, SycamoreTargetGateset and SqrtIswapTargetGateset will stay as is because they are no longer necessary for current Google devices. While they are still useful for simulated devices, they don't have to be perfect.

tanujkhattar

As discussed offline: One minor comment on the GoogleCZTargetGateset implementation; and a request to break up the PRs into 2 smaller PRs. Thanks!

cirq-google/cirq_google/transformers/target_gatesets/google_cz_gateset.py

…is transformers

cirq-google/cirq_google/json_resolver_cache.py

cirq-google/cirq_google/transformers/target_gatesets/google_cz_gateset.py

tanujkhattar · 2022-07-14T21:27:47Z

cirq-google/cirq_google/transformers/target_gatesets/google_cz_gateset_test.py

+                cirq.GateFamily(cirq.ZPowGate, tags_to_ignore=[cirq_google.PhysicalZTag()]),
+                cirq.GateFamily(cirq.ZPowGate, tags_to_accept=[cirq_google.PhysicalZTag()]),


Isn't including both of these gate families is equivalent to including the type gate family for cirq.ZPowGate ?

That's true. My preference is to keep it the way it is, to mimic the GateFamily entries in GridDevice, but either way works.

I think including both in a test is confusing. It would be better to have different tests; by including one of each (maybe controlled by a flag)?

Also; do you think we should have a default value for additional_gates in the Google target gateset? What's the default for a google device? Do we accept both virtual and physical zs by default?

Synced offline. Will stay with an empty default. Tests are now split.

dstrain115

Looks fine to me modulo the docstring and Tanuj's comments

dstrain115 · 2022-07-15T21:55:50Z

cirq-google/cirq_google/transformers/target_gatesets/google_cz_gateset.py

+
+
+class GoogleCZTargetGateset(cirq.CZTargetGateset):
+    """CZTargetGateset implementation tailored to Google devices."""


I would prefer a more explanatory test. This is a fine one-line summary, but I would like to see a further description, such as why it is tailored to Google devices.

verult · 2022-07-15T23:41:22Z

Discovered an issue (#5783). Will investigate and address in a separate PR.

verult · 2022-07-16T01:15:36Z

Issue is fixed

dstrain115 · 2022-07-16T01:32:37Z

Looks like a flaky test due to floating precision, I re-ran tests to see if it fixes it.

dstrain115 · 2022-07-16T12:04:26Z

cirq-google/cirq_google/transformers/target_gatesets/google_cz_gateset_test.py

+        (
+            # PhysicalZ tag is erased
+            cirq.Circuit(
+                cirq.Z(_qa).with_tags(cirq_google.PhysicalZTag()) ** 0.5, cirq.CZ(_qa, _qb)


FYI: this needs to be (cirq.Z(_qa) ** 0.5).with_tags(cirq_google.PhysicalZTag()). Otherwise the exponentiation drops the tag.

I would also just tweak this test to get the weird floating point failure on macos to go away. Maybe just try 0.25 or 0.125 to see if it disappears. We can figure out a better way to test approximate equals after 1.0.

… try to bypass precision error

CirqBot · 2022-07-16T22:54:15Z

Automerge cancelled: A required status check is not present.

Missing statuses: ['Build docs', 'Build protos', 'Coverage check', 'Misc check', 'Notebook formatting', 'Pytest MacOS (3.7)', 'Pytest MacOS (3.8)', 'Pytest MacOS (3.9)', 'Pytest Ubuntu (3.7)', 'Pytest Ubuntu (3.8)', 'Pytest Ubuntu (3.9)', 'Pytest Windows (3.7)', 'Pytest Windows (3.8)', 'Pytest Windows (3.9)', 'Typescript lint check', 'Typescript tests', 'Typescript tests coverage']

CirqBot · 2022-07-17T01:34:06Z

Automerge cancelled: Pull Request is not mergeable.

@tanujkhattar

…target gatesets (#5765) * GridDevice target gateset generation logic now matches all required gates for a particular gateset, rather than just the 2q gate. * In target gatesets that support additional_gates, it's set to all gates in a device's gateset other than required gates in the target gateset, so that device gates are not decomposed during circuit transformation. First commit is from #5744 @tanujkhattar cc @dstrain115

* Introducing GoogleCZTargetGateset * Change constructor flag to control both eject_z and eject_phased_paulis transformers * Updated testing for eject_paulis * Fix constructor docstring * Split tests to individual gate families * Added more details in class docstring * Raising an op to a power removes tags. Exponentiating the gate instead * Exponentiate on gate instead of TaggedOperation; changed exp value to try to bypass precision error Co-authored-by: Cirq Bot <craiggidney+github+cirqbot@google.com>

@tanujkhattar

…target gatesets (quantumlib#5765) * GridDevice target gateset generation logic now matches all required gates for a particular gateset, rather than just the 2q gate. * In target gatesets that support additional_gates, it's set to all gates in a device's gateset other than required gates in the target gateset, so that device gates are not decomposed during circuit transformation. First commit is from quantumlib#5744 @tanujkhattar cc @dstrain115

verult requested a review from tanujkhattar July 13, 2022 00:19

verult requested review from wcourtney, a team, vtomole and cduck as code owners July 13, 2022 00:19

CirqBot added the size: L 250< lines changed <1000 label Jul 13, 2022

tanujkhattar requested changes Jul 14, 2022

View reviewed changes

cirq-google/cirq_google/transformers/target_gatesets/google_cz_gateset.py Show resolved Hide resolved

cirq-google/cirq_google/transformers/target_gatesets/google_cz_gateset.py Show resolved Hide resolved

Introducing GoogleCZTargetGateset

a7669b9

verult force-pushed the cg-device-refactor/google-cz-target-gateset branch from 7d3d2dd to a7669b9 Compare July 14, 2022 01:29

Change constructor flag to control both eject_z and eject_phased_paul…

a8a99b0

…is transformers

verult mentioned this pull request Jul 14, 2022

Use GoogleCZTargetGateset; add device gateset as additional gates in target gatesets #5765

Merged

Updated testing for eject_paulis

cb77485

tanujkhattar requested changes Jul 14, 2022

View reviewed changes

Fix constructor docstring

9ada35c

dstrain115 approved these changes Jul 15, 2022

View reviewed changes

verult added 2 commits July 15, 2022 23:36

Split tests to individual gate families

b3ac385

Added more details in class docstring

17b4b59

verult force-pushed the cg-device-refactor/google-cz-target-gateset branch from a762004 to 17b4b59 Compare July 15, 2022 23:39

verult requested a review from tanujkhattar July 16, 2022 00:26

Raising an op to a power removes tags. Exponentiating the gate instead

91b5969

dstrain115 reviewed Jul 16, 2022

View reviewed changes

Exponentiate on gate instead of TaggedOperation; changed exp value to…

62b7e44

… try to bypass precision error

verult added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jul 16, 2022

Merge branch 'master' into cg-device-refactor/google-cz-target-gateset

a3b6bf5

CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jul 16, 2022

Merge branch 'master' into cg-device-refactor/google-cz-target-gateset

d6c5288

CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Jul 16, 2022

MichaelBroughton added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jul 17, 2022

CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jul 17, 2022

tanujkhattar approved these changes Jul 17, 2022

View reviewed changes

tanujkhattar merged commit 7062846 into quantumlib:master Jul 17, 2022

CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Jul 17, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GoogleCZTargetGateset #5744

GoogleCZTargetGateset #5744

verult commented Jul 13, 2022 •

edited

tanujkhattar left a comment

tanujkhattar Jul 14, 2022

verult Jul 15, 2022

tanujkhattar Jul 15, 2022

verult Jul 15, 2022

dstrain115 left a comment

dstrain115 Jul 15, 2022

verult commented Jul 15, 2022

verult commented Jul 16, 2022

dstrain115 commented Jul 16, 2022

dstrain115 Jul 16, 2022

CirqBot commented Jul 16, 2022

CirqBot commented Jul 17, 2022

		cirq.GateFamily(cirq.ZPowGate, tags_to_ignore=[cirq_google.PhysicalZTag()]),
		cirq.GateFamily(cirq.ZPowGate, tags_to_accept=[cirq_google.PhysicalZTag()]),



		class GoogleCZTargetGateset(cirq.CZTargetGateset):
		"""CZTargetGateset implementation tailored to Google devices."""

GoogleCZTargetGateset #5744

GoogleCZTargetGateset #5744

Conversation

verult commented Jul 13, 2022 • edited

tanujkhattar left a comment

Choose a reason for hiding this comment

tanujkhattar Jul 14, 2022

Choose a reason for hiding this comment

verult Jul 15, 2022

Choose a reason for hiding this comment

tanujkhattar Jul 15, 2022

Choose a reason for hiding this comment

verult Jul 15, 2022

Choose a reason for hiding this comment

dstrain115 left a comment

Choose a reason for hiding this comment

dstrain115 Jul 15, 2022

Choose a reason for hiding this comment

verult commented Jul 15, 2022

verult commented Jul 16, 2022

dstrain115 commented Jul 16, 2022

dstrain115 Jul 16, 2022

Choose a reason for hiding this comment

CirqBot commented Jul 16, 2022

CirqBot commented Jul 17, 2022

verult commented Jul 13, 2022 •

edited