Skip to content

Use CIRCUIT_SERIALIZER as default serializer for quantum engine #4983

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
CirqBot merged 6 commits into from
Feb 15, 2022
Merged

Use CIRCUIT_SERIALIZER as default serializer for quantum engine #4983

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
3dfb960
Use CIRCUIT_SERIALIZER as default serializer for quantum engine
maffoo Feb 11, 2022
dec70f3
Merge branch 'master' into u/maffoo/circuit-serializer
maffoo Feb 14, 2022
cb7e261
Merge branch 'master' into u/maffoo/circuit-serializer
maffoo Feb 14, 2022
6f5f2d0
Remove `serializer` arg on Engine
maffoo Feb 15, 2022
138ccce
Merge branch 'master' into u/maffoo/circuit-serializer
maffoo Feb 15, 2022
48c61a2
Merge branch 'master' into u/maffoo/circuit-serializer
maffoo Feb 15, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions cirq-google/cirq_google/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@

from cirq_google.serialization import (
arg_from_proto,
CIRCUIT_SERIALIZER,
CircuitSerializer,
CircuitOpDeserializer,
DeserializingArg,
Expand Down
2 changes: 1 addition & 1 deletion cirq-google/cirq_google/engine/abstract_engine.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ def get_processor(self, processor_id: str) -> abstract_processor.AbstractProcess

@abc.abstractmethod
def get_sampler(
self, processor_id: Union[str, List[str]], gate_set: Serializer
self, processor_id: Union[str, List[str]], gate_set: Optional[Serializer] = None
) -> cirq.Sampler:
"""Returns a sampler backed by the engine.

Expand Down
36 changes: 20 additions & 16 deletions cirq-google/cirq_google/engine/engine.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,18 +35,19 @@
import cirq
from cirq._compat import deprecated
from cirq_google.api import v2
from cirq_google.engine import engine_client, abstract_engine, abstract_program
from cirq_google.engine.client import quantum
from cirq_google.engine.result_type import ResultType
from cirq_google.serialization import SerializableGateSet, Serializer
from cirq_google.serialization.arg_func_langs import arg_to_proto
from cirq_google.engine import (
abstract_engine,
abstract_program,
engine_client,
engine_program,
engine_job,
engine_processor,
engine_program,
engine_sampler,
)
from cirq_google.engine.client import quantum
from cirq_google.engine.result_type import ResultType
from cirq_google.serialization import CIRCUIT_SERIALIZER, SerializableGateSet, Serializer
from cirq_google.serialization.arg_func_langs import arg_to_proto

if TYPE_CHECKING:
import cirq_google
Expand Down Expand Up @@ -85,6 +86,7 @@ def __init__(
verbose: Optional[bool] = None,
client: 'Optional[engine_client.EngineClient]' = None,
timeout: Optional[int] = None,
serializer: Serializer = CIRCUIT_SERIALIZER,
) -> None:
"""Context and client for using Quantum Engine.

Expand All @@ -99,6 +101,7 @@ def __init__(
created.
timeout: Timeout for polling for results, in seconds. Default is
to never timeout.
serializer: Used to serialize circuits when running jobs.

Raises:
ValueError: If either `service_args` and `verbose` were supplied
Expand All @@ -110,6 +113,7 @@ def __init__(
self.proto_version = proto_version or ProtoVersion.V2
if self.proto_version == ProtoVersion.V1:
raise ValueError('ProtoVersion V1 no longer supported')
self.serializer = serializer

if not client:
client = engine_client.EngineClient(service_args=service_args, verbose=verbose)
Expand Down Expand Up @@ -149,6 +153,7 @@ def __init__(
verbose: Optional[bool] = None,
timeout: Optional[int] = None,
context: Optional[EngineContext] = None,
serializer: Serializer = CIRCUIT_SERIALIZER,
Copy link
Copy Markdown
Collaborator

@verult verult Feb 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the future we'd like to remove SerializableGateset, making CIRCUIT_SERIALIZER the only serializer AFAIK. What if we don't provide an option to configure the serializer and always use CIRCUIT_SERIALIZER?

Copy link
Copy Markdown
Contributor Author

@maffoo maffoo Feb 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could certainly do that, though having it configurable gives us a way to test out CircuitSerializer2 when that comes out...

(I kid, but only slightly)

Copy link
Copy Markdown
Collaborator

@verult verult Feb 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would lean towards keeping the code simpler now and adding this back when the use case comes up.

Another reason to keep the parameter is if we want to inject a mock to test that Engine calls the serializer with the expected circuit, but it doesn't look like we do that today. And I'm not sure if we should - if the logic for prepping the serializer call isn't complicated then such a test is probably overkill.

Copy link
Copy Markdown
Contributor Author

@maffoo maffoo Feb 15, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the option to customize this on Engine. I've kept it on EngineContext since that gets passed around to various places where needed and I'd prefer to access the serializer in one way on the context instead of hard-coding CIRCUIT_SERIALIZER everywhere it is used.

) -> None:
"""Supports creating and running programs against the Quantum Engine.

Expand All @@ -167,6 +172,8 @@ def __init__(
to never timeout.
context: Engine configuration and context to use. For most users
this should never be specified.
serializer: Default serializer to use, which will take effect if
context is not specified.

Raises:
ValueError: If context is provided and one of proto_version, service_args, or verbose.
Expand All @@ -178,6 +185,7 @@ def __init__(
if not context:
context = EngineContext(
proto_version=proto_version,
serializer=serializer,
service_args=service_args,
verbose=verbose,
timeout=timeout,
Expand All @@ -195,7 +203,7 @@ def run(
param_resolver: cirq.ParamResolver = cirq.ParamResolver({}),
repetitions: int = 1,
processor_ids: Sequence[str] = ('xmonsim',),
gate_set: Optional[Serializer] = None,
gate_set: Serializer = None,
Copy link
Copy Markdown
Collaborator

@verult verult Feb 11, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's not too much hassle, could you add deprecation notices to these fields? In my discussions I haven't heard arguments against removing this field in the future.

Copy link
Copy Markdown
Contributor Author

@maffoo maffoo Feb 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could do that here, of in a separate PR; which do you prefer? I think there are probably still some usages of this parameter in tests and example notebooks that we'd want to change, so I might prefer making a separate PR to track all that down.

Copy link
Copy Markdown
Collaborator

@verult verult Feb 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Separate PR sounds good!

program_description: Optional[str] = None,
program_labels: Optional[Dict[str, str]] = None,
job_description: Optional[str] = None,
Expand Down Expand Up @@ -233,8 +241,6 @@ def run(
Raises:
ValueError: If no gate set is provided.
"""
if not gate_set:
raise ValueError('No gate set provided')
return list(
self.run_sweep(
program=program,
Expand Down Expand Up @@ -301,8 +307,6 @@ def run_sweep(
Raises:
ValueError: If no gate set is provided.
"""
if not gate_set:
raise ValueError('No gate set provided')
engine_program = self.create_program(
program, program_id, gate_set, program_description, program_labels
)
Expand Down Expand Up @@ -502,7 +506,7 @@ def create_program(
ValueError: If no gate set is provided.
"""
if not gate_set:
raise ValueError('No gate set provided')
gate_set = self.context.serializer

if not program_id:
program_id = _make_random_id('prog-')
Expand Down Expand Up @@ -548,7 +552,7 @@ def create_batch_program(
ValueError: If no gate set is provided.
"""
if not gate_set:
raise ValueError('Gate set must be specified.')
gate_set = self.context.serializer
if not program_id:
program_id = _make_random_id('prog-')

Expand Down Expand Up @@ -601,7 +605,7 @@ def create_calibration_program(
ValueError: If not gate set is given.
"""
if not gate_set:
raise ValueError('Gate set must be specified.')
gate_set = self.context.serializer
if not program_id:
program_id = _make_random_id('calibration-')

Expand Down Expand Up @@ -784,7 +788,7 @@ def get_processor(self, processor_id: str) -> engine_processor.EngineProcessor:

@deprecated(deadline="v1.0", fix="Use get_sampler instead.")
def sampler(
self, processor_id: Union[str, List[str]], gate_set: Serializer
self, processor_id: Union[str, List[str]], gate_set: Optional[Serializer] = None
) -> engine_sampler.QuantumEngineSampler:
"""Returns a sampler backed by the engine.

Expand All @@ -802,7 +806,7 @@ def sampler(
return self.get_sampler(processor_id, gate_set)

def get_sampler(
self, processor_id: Union[str, List[str]], gate_set: Serializer
self, processor_id: Union[str, List[str]], gate_set: Optional[Serializer] = None
) -> engine_sampler.QuantumEngineSampler:
"""Returns a sampler backed by the engine.

Expand Down
4 changes: 2 additions & 2 deletions cirq-google/cirq_google/engine/engine_processor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
calibration_layer,
engine_sampler,
)
from cirq_google.serialization import circuit_serializer, serializable_gate_set, serializer
from cirq_google.serialization import serializable_gate_set, serializer
from cirq_google.serialization import gate_sets as gs

if TYPE_CHECKING:
Expand Down Expand Up @@ -113,7 +113,7 @@ def get_sampler(
return engine_sampler.QuantumEngineSampler(
engine=self.engine(),
processor_id=self.processor_id,
gate_set=gate_set or circuit_serializer.CIRCUIT_SERIALIZER,
gate_set=gate_set,
)

def run_batch(
Expand Down
2 changes: 1 addition & 1 deletion cirq-google/cirq_google/engine/engine_sampler.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def __init__(
*,
engine: 'cirq_google.Engine',
processor_id: Union[str, List[str]],
gate_set: 'cirq_google.serialization.Serializer',
gate_set: Optional['cirq_google.serialization.Serializer'] = None,
):
"""Inits QuantumEngineSampler.

Expand Down
Loading