-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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.
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
Add cg.ProcessorSampler
#5361
Add cg.ProcessorSampler
#5361
Changes from 5 commits
c3437bc
94e8c48
fde41eb
0680011
848ac63
ef0943a
442a099
cd9d846
8c5ea28
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,76 @@ | ||
# Copyright 2022 The Cirq Developers | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# https://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Optional: should EngineSampler use this class underneath? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think it (easily) can.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am starting to agree with maffoo. It seems like ProcessorSampler is basically the same as a QuantumEngineSampler with a processor set already. Seems like these two can/should be combined. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After off-line conversations, I am fully on-board with this change. I think we should deprecate QuantumEngineSampler and switch engine.get_sampler(processor_id) to use engine.get_processor(processor_id).get_sampler() (or, alternatively, make QuantumEngineSampler a thin wrapper around this ProcessorSampler functionality). Since this functionality currently supports multiple processor ids and sending an EngineProgram, it probably needs more thought and should be done in a follow-up PR. Short story, I approve this PR as is. |
||
|
||
from typing import List, Optional, Sequence, TYPE_CHECKING, Union, cast | ||
|
||
import cirq | ||
|
||
if TYPE_CHECKING: | ||
import cirq_google as cg | ||
|
||
|
||
class QuantumProcessorSampler(cirq.Sampler): | ||
"""A wrapper around AbstractProcessor to implement the cirq.Sampler interface.""" | ||
|
||
def __init__(self, *, processor: 'cg.engine.AbstractProcessor'): | ||
"""Inits QuantumProcessorSampler. | ||
|
||
Args: | ||
processor: AbstractProcessor instance to use. | ||
""" | ||
self._processor = processor | ||
|
||
def run_sweep( | ||
self, program: 'cirq.AbstractCircuit', params: cirq.Sweepable, repetitions: int = 1 | ||
) -> Sequence['cg.EngineResult']: | ||
job = self._processor.run_sweep(program=program, params=params, repetitions=repetitions) | ||
return job.results() | ||
|
||
def run_batch( | ||
self, | ||
programs: Sequence[cirq.AbstractCircuit], | ||
params_list: Optional[List[cirq.Sweepable]] = None, | ||
repetitions: Union[int, List[int]] = 1, | ||
) -> Sequence[Sequence['cg.EngineResult']]: | ||
"""Runs the supplied circuits. | ||
|
||
In order to gain a speedup from using this method instead of other run | ||
methods, the following conditions must be satisfied: | ||
1. All circuits must measure the same set of qubits. | ||
2. The number of circuit repetitions must be the same for all | ||
circuits. That is, the `repetitions` argument must be an integer, | ||
or else a list with identical values. | ||
""" | ||
if isinstance(repetitions, List) and len(programs) != len(repetitions): | ||
raise ValueError( | ||
'len(programs) and len(repetitions) must match. ' | ||
f'Got {len(programs)} and {len(repetitions)}.' | ||
) | ||
if isinstance(repetitions, int) or len(set(repetitions)) == 1: | ||
# All repetitions are the same so batching can be done efficiently | ||
if isinstance(repetitions, List): | ||
repetitions = repetitions[0] | ||
job = self._processor.run_batch( | ||
programs=programs, params_list=params_list, repetitions=repetitions | ||
) | ||
return job.batched_results() | ||
# Varying number of repetitions so no speedup | ||
return cast( | ||
Sequence[Sequence['cg.EngineResult']], | ||
super().run_batch(programs, params_list, repetitions), | ||
) | ||
|
||
@property | ||
def processor(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: add return type. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
return self._processor |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
# Copyright 2022 The Cirq Developers | ||
# | ||
# Licensed under the Apache License, Version 2.0 (the "License"); | ||
# you may not use this file except in compliance with the License. | ||
# You may obtain a copy of the License at | ||
# | ||
# https://www.apache.org/licenses/LICENSE-2.0 | ||
# | ||
# Unless required by applicable law or agreed to in writing, software | ||
# distributed under the License is distributed on an "AS IS" BASIS, | ||
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
# See the License for the specific language governing permissions and | ||
# limitations under the License. | ||
|
||
from unittest import mock | ||
|
||
import pytest | ||
|
||
import cirq | ||
import cirq_google as cg | ||
|
||
|
||
@pytest.mark.parametrize('circuit', [cirq.Circuit(), cirq.FrozenCircuit()]) | ||
def test_run_circuit(circuit): | ||
processor = mock.Mock() | ||
sampler = cg.QuantumProcessorSampler(processor=processor) | ||
params = [cirq.ParamResolver({'a': 1})] | ||
sampler.run_sweep(circuit, params, 5) | ||
processor.run_sweep.assert_called_with(params=params, program=circuit, repetitions=5) | ||
|
||
|
||
def test_run_batch(): | ||
processor = mock.Mock() | ||
sampler = cg.QuantumProcessorSampler(processor=processor) | ||
a = cirq.LineQubit(0) | ||
circuit1 = cirq.Circuit(cirq.X(a)) | ||
circuit2 = cirq.Circuit(cirq.Y(a)) | ||
params1 = [cirq.ParamResolver({'t': 1})] | ||
params2 = [cirq.ParamResolver({'t': 2})] | ||
circuits = [circuit1, circuit2] | ||
params_list = [params1, params2] | ||
sampler.run_batch(circuits, params_list, 5) | ||
processor.run_batch.assert_called_with( | ||
params_list=params_list, programs=circuits, repetitions=5 | ||
) | ||
|
||
|
||
def test_run_batch_identical_repetitions(): | ||
processor = mock.Mock() | ||
sampler = cg.QuantumProcessorSampler(processor=processor) | ||
a = cirq.LineQubit(0) | ||
circuit1 = cirq.Circuit(cirq.X(a)) | ||
circuit2 = cirq.Circuit(cirq.Y(a)) | ||
params1 = [cirq.ParamResolver({'t': 1})] | ||
params2 = [cirq.ParamResolver({'t': 2})] | ||
circuits = [circuit1, circuit2] | ||
params_list = [params1, params2] | ||
sampler.run_batch(circuits, params_list, [5, 5]) | ||
processor.run_batch.assert_called_with( | ||
params_list=params_list, programs=circuits, repetitions=5 | ||
) | ||
|
||
|
||
def test_run_batch_bad_number_of_repetitions(): | ||
processor = mock.Mock() | ||
sampler = cg.QuantumProcessorSampler(processor=processor) | ||
a = cirq.LineQubit(0) | ||
circuit1 = cirq.Circuit(cirq.X(a)) | ||
circuit2 = cirq.Circuit(cirq.Y(a)) | ||
params1 = [cirq.ParamResolver({'t': 1})] | ||
params2 = [cirq.ParamResolver({'t': 2})] | ||
circuits = [circuit1, circuit2] | ||
params_list = [params1, params2] | ||
with pytest.raises(ValueError, match='2 and 3'): | ||
sampler.run_batch(circuits, params_list, [5, 5, 5]) | ||
|
||
|
||
def test_run_batch_differing_repetitions(): | ||
processor = mock.Mock() | ||
job = mock.Mock() | ||
job.results.return_value = [] | ||
processor.run_sweep.return_value = job | ||
sampler = cg.QuantumProcessorSampler(processor=processor) | ||
a = cirq.LineQubit(0) | ||
circuit1 = cirq.Circuit(cirq.X(a)) | ||
circuit2 = cirq.Circuit(cirq.Y(a)) | ||
params1 = [cirq.ParamResolver({'t': 1})] | ||
params2 = [cirq.ParamResolver({'t': 2})] | ||
circuits = [circuit1, circuit2] | ||
params_list = [params1, params2] | ||
repetitions = [1, 2] | ||
sampler.run_batch(circuits, params_list, repetitions) | ||
processor.run_sweep.assert_called_with(params=params2, program=circuit2, repetitions=2) | ||
processor.run_batch.assert_not_called() | ||
|
||
|
||
def test_processor_sampler_processor_property(): | ||
processor = mock.Mock() | ||
sampler = cg.QuantumProcessorSampler(processor=processor) | ||
assert sampler.processor is processor | ||
|
||
|
||
def test_with_local_processor(): | ||
sampler = cg.QuantumProcessorSampler( | ||
processor=cg.engine.SimulatedLocalProcessor(processor_id='my-fancy-processor') | ||
) | ||
r = sampler.run(cirq.Circuit(cirq.measure(cirq.LineQubit(0), key='z'))) | ||
assert isinstance(r, cg.EngineResult) | ||
assert r.job_id == 'projects/fake_project/processors/my-fancy-processor/job/2' | ||
assert r.measurements['z'] == [[0]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wcourtney Will changing this cause any backwards incompatibility problems? This affects the QCS engine in addition to the simulated version.
I think it should be fine, but want to double-check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a backwards incompatible change for anyone using
processor.get_sampler()
and trying to use multiple processor_ids. However, this workflow doesn't make any sense and I don't think many people are usingprocessor.get_sampler()
.Most people using
cg.get_engine().get_sampler()
orcg.get_engine_sampler()
will be unaffectedThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the PR description