SerializableGateSet deprecation #5573

verult · 2022-06-22T17:49:41Z

Deprecate global SerializableGateSets
Deprecate GOOGLE_GATESETS. This is not exposed in cirq_google/init.py; it is only used in engine_program.py for selecting the deserializer using the Program.language.gate_set proto field. Removing other gatesets from consideration here is premature since global gatesets have not been removed. This code should move to CircuitSerializer completely before 1.0.
Deprecate the SerializableGateSet class. A new class wrapper is introduced to bypass the behavior that deprecation warnings thrown while loading global gatesets at the module level fail unit tests.

dstrain115 · 2022-06-22T19:46:35Z

cirq-google/cirq_google/__init__.py

@@ -164,7 +164,10 @@
 _compat.deprecate_attributes(
    __name__,
    {
-        'Bristlecone': ('v0.15', 'Bristlecone will no longer be supported.'),
-        'Foxtail': ('v0.15', 'Foxtail will no longer be supported.'),
+        'XMON': ('v0.16', 'SerializableGateSet will no longer be supported.'),


Should we point them to CircuitSerializer? Or say that gatesets are no longer required for Engine, or something?

dstrain115 · 2022-06-22T20:00:04Z

cirq-google/cirq_google/engine/engine_validator_test.py

        engine_validator.validate_gate_set(
-            [circuit] * 10, [{}] * 10, 1000, cg.FSIM_GATESET, max_size=100000
+            [circuit] * 5, [{}] * 5, 1000, cg.FSIM_GATESET, max_size=100000


Is it possible to change this reference to CircuitSerializer? Same with below.

We can, but to help get deprecations out the door, I deferred caller changes to after the 0.15 release wherever I can. We'll have to cross this bridge again when we remove SerializableGateSet.

Will keep a note of this for SerializableGateSet removal.

dstrain115 · 2022-06-22T20:01:36Z

cirq-google/cirq_google/optimizers/convert_to_sqrt_iswap_test.py

-    cig.SQRT_ISWAP_GATESET.serialize(converted_circuit)
-    cig.SQRT_ISWAP_GATESET.serialize(converted_circuit_iswap)
-    cig.SQRT_ISWAP_GATESET.serialize(converted_circuit_iswap_inv)
+    with cirq.testing.assert_deprecated('SerializableGateSet', deadline='v0.16', count=None):


We should consider validating this with a compilation target gate set instead?

Same as the other case - I'll keep a note of this for SerializableGateSet removal as well.

dstrain115 · 2022-06-22T21:44:26Z

cirq-google/cirq_google/serialization/serializable_gate_set.py

@@ -24,10 +24,11 @@
 _GateOrFrozenCircuitTypes = Union[Type[cirq.Gate], Type[cirq.FrozenCircuit]]


-class SerializableGateSet(serializer.Serializer):
-    """A class for serializing and deserializing programs and operations.
+class _SerializableGateSet(serializer.Serializer):


Should we deprecate the functions in this class? If we deprecate the functions in the class, then any use will generate a warning, right? That way, instantiation won't generate a warning, but if it is used to serialize a class, it will generate a warning.

Hmmm yeah potentially. One concern is that there could be more call sites to the methods than the constructor since some code interacts with SerializableGateSet via the Serializer interface, leading to more tests that need to be updated. More work, but certainly less hacky.

I tried deprecating the serialize() method and only got test failures in serializable_gate_set_test.py, so perhaps it's OK. On the other hand, I'll run into the same problem for deprecating serializers as well, and I can't tell for sure whether test files for testing callers of class methods will fail.

Given that, my preference is to do the simplest thing and keep it the way it, since a lot of the code here will be removed very soon.

I think it would be preferable to at least deprecate the serialize() method. That way people get a message when they use the most common call site at least.

Chatted offline - the most common call site for users is via global gatesets, which have a separate deprecation warning in place to tell users about SerializableGateSet deprecation.

Deprecating the class directly will cause global gatesets such as SYC_GATESET to throw a deprecation warning during module loading. A shell class (containing class documentation) is created to avoid this.

…factor/well-known-gatesets-deprecation

verult · 2022-06-23T02:54:32Z

In the latest push: expanded on the fix message for deprecated global gatesets, and also added a sentence about the gateset no long being necessary in engine.

dstrain115

Author commented that the _SerializableGateset path was already protected by a deprecation message via the deprecate_attributes() call.

…recation

@dstrain115

* Deprecate global SerializableGateSets * Deprecate `GOOGLE_GATESETS`. This is not exposed in cirq_google/__init__.py; it is only used in `engine_program.py` for selecting the deserializer using the Program.language.gate_set proto field. Removing other gatesets from consideration here is premature since global gatesets have not been removed. This code should move to CircuitSerializer completely before 1.0. * Deprecate the `SerializableGateSet` class. A new class wrapper is introduced to bypass the behavior that deprecation warnings thrown while loading global gatesets at the module level fail unit tests. @dstrain115 @MichaelBroughton

verult requested review from MichaelBroughton and dstrain115 June 22, 2022 17:49

verult requested review from wcourtney, a team, vtomole and cduck as code owners June 22, 2022 17:49

CirqBot added the size: XL lines changed >1000 label Jun 22, 2022

dstrain115 reviewed Jun 22, 2022

View reviewed changes

verult added 2 commits June 23, 2022 02:47

Deprecate global SerializableGateSets

8071b1e

Deprecate SerializableGateSet.

0bb9a06

Deprecating the class directly will cause global gatesets such as SYC_GATESET to throw a deprecation warning during module loading. A shell class (containing class documentation) is created to avoid this.

verult force-pushed the cg-device-refactor/well-known-gatesets-deprecation branch from b79bb1e to 0bb9a06 Compare June 23, 2022 02:49

Merge branch 'master' of github.com:quantumlib/Cirq into cg-device-re…

82ad9da

…factor/well-known-gatesets-deprecation

Fix syntax error

986aed9

MichaelBroughton requested a review from dstrain115 June 23, 2022 05:49

dstrain115 approved these changes Jun 23, 2022

View reviewed changes

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

Merge branch 'master' into cg-device-refactor/well-known-gatesets-dep…

342c06d

…recation

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

CirqBot merged commit abfc619 into quantumlib:master Jun 23, 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 Jun 23, 2022

verult mentioned this pull request Jul 7, 2022

cirq-google Device Refactor #5050

Closed

40 tasks

verult mentioned this pull request Jul 22, 2022

Deprecating classes used by global singletons is challenging #5809

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

SerializableGateSet deprecation #5573

SerializableGateSet deprecation #5573

Uh oh!

verult commented Jun 22, 2022

Uh oh!

dstrain115 Jun 22, 2022

Uh oh!

dstrain115 Jun 22, 2022

Uh oh!

verult Jun 22, 2022 •

edited

Loading

Uh oh!

dstrain115 Jun 22, 2022

Uh oh!

verult Jun 22, 2022

Uh oh!

dstrain115 Jun 22, 2022

Uh oh!

verult Jun 22, 2022

Uh oh!

verult Jun 23, 2022

Uh oh!

dstrain115 Jun 23, 2022

Uh oh!

verult Jun 23, 2022

Uh oh!

verult commented Jun 23, 2022

Uh oh!

dstrain115 left a comment

Uh oh!

Uh oh!

SerializableGateSet deprecation #5573

SerializableGateSet deprecation #5573

Uh oh!

Conversation

verult commented Jun 22, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

verult Jun 22, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

verult commented Jun 23, 2022

Uh oh!

dstrain115 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

verult Jun 22, 2022 •

edited

Loading