Move serialize_program and serialize_run_context into EngineContext #5034
Conversation
maffoo
requested review from
wcourtney,
a team,
vtomole,
cduck and
verult
as code owners
| program = serializer.serialize(program) | ||
| return _pack_any(program) | ||
|
|
||
| def serialize_run_context(self, sweeps: List[cirq.Sweep], repetitions: int) -> any_pb2.Any: |
There was a problem hiding this comment.
Since these are now public, can you add unit tests to cirq-google/cirq_google/engine/engine_test.py?
| @@ -126,6 +126,28 @@ def copy(self) -> 'EngineContext': | |||
| def _value_equality_values_(self): | |||
| return self.proto_version, self.client | |||
|
|
|||
| def serialize_program( | |||
There was a problem hiding this comment.
I find the name "program" confusing here. Since it's moving to public visibility, what do you think of making it something like serialize_circuit and let the weird naming in create_program be its own problem?
There was a problem hiding this comment.
The term "program" is fairly well-established, for example it is used in all the cirq.Sampler methods, so I'm inclined to leave this, but I could be persuaded.
| program = serializer.serialize(program) | ||
| return _pack_any(program) | ||
|
|
||
| def serialize_run_context(self, sweeps: List[cirq.Sweep], repetitions: int) -> any_pb2.Any: |
There was a problem hiding this comment.
Should this also accept a serializer for consistency with the serialize_program method above?
There was a problem hiding this comment.
The serializer is only used for circuits, so not needed for serializing the run context.
Also move pack_any into a utility module and use it in more places.
|
I reworked this a bit when thinking about testing:
|
| @@ -134,3 +134,37 @@ def test_sweep_with_flattened_sweep(): | |||
| param_sweep1 = cirq.Linspace('t', start=0, stop=1, length=20) | |||
| (_, param_sweep2) = cirq.flatten_with_sweep(circuit, param_sweep1) | |||
| assert v2.sweep_to_proto(param_sweep2) is not None | |||
|
|
|||
|
|
|||
| def test_run_context_to_proto(): | |||
There was a problem hiding this comment.
Can you also add a test to demonstrate that specifying the out parameter is appended to? Same for the batch case.
| from google.protobuf.message import Message | ||
|
|
||
|
|
||
| def pack_any(message: Message) -> any_pb2.Any: |
There was a problem hiding this comment.
Shocking that there's not a standard utility for this, but it doesn't look like there is...
It's convenient to be able to use these functions outside of having an
EngineProgramobject or callingEngine.create_program, for example when testing direct use of the streaming API with the combinedCreateQuantumProgramAndJobRequestcall.@wcourtney, @verult