Skip to content

Enforces specifying run_name and device_config_name on Engine interfaces #6649

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
merged 7 commits into from
Jun 25, 2024
Merged

Enforces specifying run_name and device_config_name on Engine interfaces #6649

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
1f354ab
Remove default values from client interfaces (e.g. EngineProgram.run_…
senecameeks Jun 21, 2024
6b09fd9
fix
senecameeks Jun 21, 2024
d74587b
more test files changes
senecameeks Jun 21, 2024
2bf4e82
address nits
senecameeks Jun 24, 2024
a94c8e8
mv args
senecameeks Jun 25, 2024
0497c85
lint
senecameeks Jun 25, 2024
ee4ae61
Merge branch 'main' into u/smeeks/rm_defaults
senecameeks Jun 25, 2024
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
10 changes: 6 additions & 4 deletions cirq-google/cirq_google/engine/abstract_processor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,9 @@ async def run_async(
program_labels: Optional[Dict[str, str]] = None,
job_description: Optional[str] = None,
job_labels: Optional[Dict[str, str]] = None,
run_name: str = "",
device_config_name: str = "",
*,
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we need the * here and elsewhere? what is the purpose?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mainly for readability and convenience. This preserves parameter ordering and keeps the number of required changes low.

Copy link
Collaborator

Choose a reason for hiding this comment

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

On a second look, it is a bit strange to have optional positional arguments followed by mandatory kw arguments run_name, device_config_name. I tried to move the added *, just before the first optional arguments and the tests pass.

Can we do so and also move the run_name, device_name as the first arguments after *,?
The order of kw-only arguments does not matter, but for code readability it is nicer to have
mandatory arguments followed by optional.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good point! Moved.

run_name: str,
device_config_name: str,
) -> cirq.Result:
"""Runs the supplied Circuit on this processor.

Expand Down Expand Up @@ -125,8 +126,9 @@ async def run_sweep_async(
program_labels: Optional[Dict[str, str]] = None,
job_description: Optional[str] = None,
job_labels: Optional[Dict[str, str]] = None,
run_name: str = "",
device_config_name: str = "",
*,
run_name: str,
device_config_name: str,
) -> 'abstract_job.AbstractJob':
"""Runs the supplied Circuit on this processor.

Expand Down
5 changes: 3 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 @@ -123,8 +123,9 @@ async def run_sweep_async(
program_labels: Optional[Dict[str, str]] = None,
job_description: Optional[str] = None,
job_labels: Optional[Dict[str, str]] = None,
run_name: str = "",
device_config_name: str = "",
*,
run_name: str,
device_config_name: str,
) -> 'abstract_job.AbstractJob':
"""Runs the supplied Circuit on this processor.

Expand Down
10 changes: 8 additions & 2 deletions cirq-google/cirq_google/engine/engine_processor_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -848,7 +848,10 @@ def test_run_sweep_params_with_unary_rpcs(client):

processor = cg.EngineProcessor('a', 'p', EngineContext(enable_streaming=False))
job = processor.run_sweep(
program=_CIRCUIT, params=[cirq.ParamResolver({'a': 1}), cirq.ParamResolver({'a': 2})]
program=_CIRCUIT,
params=[cirq.ParamResolver({'a': 1}), cirq.ParamResolver({'a': 2})],
run_name="run",
device_config_name="config_alias",
)
results = job.results()
assert len(results) == 2
Expand Down Expand Up @@ -887,7 +890,10 @@ def test_run_sweep_params_with_stream_rpcs(client):

processor = cg.EngineProcessor('a', 'p', EngineContext(enable_streaming=True))
job = processor.run_sweep(
program=_CIRCUIT, params=[cirq.ParamResolver({'a': 1}), cirq.ParamResolver({'a': 2})]
program=_CIRCUIT,
params=[cirq.ParamResolver({'a': 1}), cirq.ParamResolver({'a': 2})],
run_name="run",
device_config_name="config_alias",
)
results = job.results()
assert len(results) == 2
Expand Down
8 changes: 4 additions & 4 deletions cirq-google/cirq_google/engine/engine_program.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,8 @@ async def run_sweep_async(
description: Optional[str] = None,
labels: Optional[Dict[str, str]] = None,
*,
run_name: str = "",
device_config_name: str = "",
run_name: str,
device_config_name: str,
) -> engine_job.EngineJob:
"""Runs the program on the QuantumEngine.

Expand Down Expand Up @@ -138,8 +138,8 @@ async def run_async(
description: Optional[str] = None,
labels: Optional[Dict[str, str]] = None,
*,
run_name: str = "",
device_config_name: str = "",
run_name: str,
device_config_name: str,
) -> cirq.Result:
"""Runs the supplied Circuit via Quantum Engine.

Expand Down
14 changes: 12 additions & 2 deletions cirq-google/cirq_google/engine/engine_program_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,12 @@ def test_run_sweeps_delegation(create_job_async):
program = cg.EngineProgram('my-proj', 'my-prog', EngineContext())
param_resolver = cirq.ParamResolver({})
job = program.run_sweep(
job_id='steve', repetitions=10, params=param_resolver, processor_id='mine'
job_id='steve',
repetitions=10,
params=param_resolver,
processor_id='mine',
run_name="run_name",
device_config_name="config",
)
assert job._job == quantum.QuantumJob()

Expand Down Expand Up @@ -134,7 +139,12 @@ def test_run_delegation(create_job_async, get_results_async):
program = cg.EngineProgram('a', 'b', EngineContext())
param_resolver = cirq.ParamResolver({})
results = program.run(
job_id='steve', repetitions=10, param_resolver=param_resolver, processor_id='mine'
job_id='steve',
repetitions=10,
param_resolver=param_resolver,
processor_id='mine',
run_name="run_name",
device_config_name="config",
)

assert results == cg.EngineResult(
Expand Down
17 changes: 14 additions & 3 deletions cirq-google/cirq_google/engine/engine_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,12 +533,21 @@ def test_run_multiple_times(client):

engine = cg.Engine(project_id='proj', proto_version=cg.engine.engine.ProtoVersion.V2)
program = engine.create_program(program=_CIRCUIT)
program.run(processor_id='processor0', param_resolver=cirq.ParamResolver({'a': 1}))
program.run(
processor_id='processor0',
param_resolver=cirq.ParamResolver({'a': 1}),
run_name="run",
device_config_name="config_alias",
)
run_context = v2.run_context_pb2.RunContext()
client().create_job_async.call_args[1]['run_context'].Unpack(run_context)
sweeps1 = run_context.parameter_sweeps
job2 = program.run_sweep(
processor_id='processor0', repetitions=2, params=cirq.Points('a', [3, 4])
processor_id='processor0',
repetitions=2,
params=cirq.Points('a', [3, 4]),
run_name="run",
device_config_name="config_alias",
)
client().create_job_async.call_args[1]['run_context'].Unpack(run_context)
sweeps2 = run_context.parameter_sweeps
Expand Down Expand Up @@ -629,7 +638,9 @@ def test_bad_sweep_proto():
engine = cg.Engine(project_id='project-id', proto_version=cg.ProtoVersion.UNDEFINED)
program = cg.EngineProgram('proj', 'prog', engine.context)
with pytest.raises(ValueError, match='invalid run context proto version'):
program.run_sweep(processor_id='processor0')
program.run_sweep(
processor_id='processor0', run_name="run", device_config_name="config_alias"
)


@mock.patch('cirq_google.engine.engine_client.EngineClient', autospec=True)
Expand Down
2 changes: 1 addition & 1 deletion cirq-google/cirq_google/engine/simulated_local_processor_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ def test_run():
proc = SimulatedLocalProcessor(processor_id='test_proc')
q = cirq.GridQubit(5, 4)
circuit = cirq.Circuit(cirq.X(q), cirq.measure(q, key='m'))
result = proc.run(circuit, repetitions=100)
result = proc.run(circuit, repetitions=100, run_name="run", device_config_name="config_alias")
assert np.all(result.measurements['m'] == 1)


Expand Down
14 changes: 8 additions & 6 deletions cirq-google/cirq_google/engine/virtual_engine_factory_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,22 @@ def _test_processor(processor: cg.engine.abstract_processor.AbstractProcessor):
Also tests the non-Sycamore qubits and gates fail."""
good_qubit = cirq.GridQubit(5, 4)
circuit = cirq.Circuit(cirq.X(good_qubit), cirq.measure(good_qubit))
results = processor.run(circuit, repetitions=100)
results = processor.run(circuit, repetitions=100, run_name="run", device_config_name="config")
assert np.all(results.measurements[str(good_qubit)] == 1)
with pytest.raises(RuntimeError, match='requested total repetitions'):
_ = processor.run(circuit, repetitions=100_000_000)
_ = processor.run(
circuit, repetitions=100_000_000, run_name="run", device_config_name="config"
)

bad_qubit = cirq.GridQubit(10, 10)
circuit = cirq.Circuit(cirq.X(bad_qubit), cirq.measure(bad_qubit))
with pytest.raises(ValueError, match='Qubit not on device'):
_ = processor.run(circuit, repetitions=100)
_ = processor.run(circuit, repetitions=100, run_name="run", device_config_name="config")
circuit = cirq.Circuit(
cirq.testing.DoesNotSupportSerializationGate()(good_qubit), cirq.measure(good_qubit)
)
with pytest.raises(ValueError, match='Cannot serialize op'):
_ = processor.run(circuit, repetitions=100)
_ = processor.run(circuit, repetitions=100, run_name="run", device_config_name="config")


def test_create_device_from_processor_id():
Expand Down Expand Up @@ -195,13 +197,13 @@ def test_create_default_noisy_quantum_virtual_machine():
bad_qubit = cirq.GridQubit(10, 10)
circuit = cirq.Circuit(cirq.X(bad_qubit), cirq.measure(bad_qubit))
with pytest.raises(ValueError, match='Qubit not on device'):
_ = processor.run(circuit, repetitions=100)
_ = processor.run(circuit, repetitions=100, run_name="run", device_config_name="config")
good_qubit = cirq.GridQubit(5, 4)
circuit = cirq.Circuit(
cirq.testing.DoesNotSupportSerializationGate()(good_qubit), cirq.measure(good_qubit)
)
with pytest.raises(ValueError, match='.* contains a gate which is not supported.'):
_ = processor.run(circuit, repetitions=100)
_ = processor.run(circuit, repetitions=100, run_name="run", device_config_name="config")
device_specification = processor.get_device_specification()
expected = factory.create_device_spec_from_processor_id(processor_id)
assert device_specification is not None
Expand Down
2 changes: 1 addition & 1 deletion cirq-google/cirq_google/workflow/processor_record_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ def test_engine_result():

circ = cirq.Circuit(cirq.measure(cirq.GridQubit(5, 4)))

res1 = proc.run(circ)
res1 = proc.run(circ, run_name="run", device_config_name="config_alias")
assert isinstance(res1, cg.EngineResult)
res2 = samp.run(circ)
assert isinstance(res2, cg.EngineResult)
Loading