Skip to content

Support the new quantum engine processor_selector in engine_client#6254

Merged
verult merged 18 commits intoquantumlib:masterfrom
jurruti:quantum_engine
Aug 28, 2023
Merged

Support the new quantum engine processor_selector in engine_client#6254
verult merged 18 commits intoquantumlib:masterfrom
jurruti:quantum_engine

Conversation

@jurruti
Copy link
Copy Markdown
Contributor

@jurruti jurruti commented Aug 22, 2023

The update is no longer a breaking change. Note that I force pushed (because the refactor was quite heavy), so most comments from the first round are outdated.

@jurruti jurruti requested review from a team, cduck, verult, vtomole and wcourtney as code owners August 22, 2023 17:33
@CirqBot CirqBot added the size: L 250< lines changed <1000 label Aug 22, 2023
Comment thread cirq-google/cirq_google/engine/engine_client.py
Comment thread cirq-google/cirq_google/engine/engine_client.py Outdated
Comment thread cirq-google/cirq_google/engine/engine_client.py Outdated
Comment thread cirq-google/cirq_google/engine/engine_client.py
Comment thread cirq-google/cirq_google/engine/engine_client.py Outdated
Comment thread cirq-google/cirq_google/engine/engine_client.py Outdated
Comment thread cirq-google/cirq_google/engine/engine_client.py Outdated
Comment thread cirq-google/cirq_google/engine/engine_client_test.py
Comment thread cirq-google/cirq_google/engine/engine_client.py Outdated
Copy link
Copy Markdown
Collaborator

@verult verult left a comment

Choose a reason for hiding this comment

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

First pass, haven't looked at tests yet

Comment thread cirq-google/cirq_google/engine/engine_client.py Outdated
Comment thread cirq-google/cirq_google/engine/engine_client.py Outdated
Comment thread cirq-google/cirq_google/engine/engine_client.py Outdated
Comment thread cirq-google/cirq_google/engine/engine_client.py
Comment thread cirq-google/cirq_google/engine/engine_client.py Outdated
Comment thread cirq-google/cirq_google/engine/engine_client.py
Comment thread cirq-google/cirq_google/engine/engine_client.py Outdated
Comment thread cirq-google/cirq_google/engine/engine_client.py Outdated
@codecov
Copy link
Copy Markdown

codecov bot commented Aug 23, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (ed26d2f) 97.60% compared to head (4baf027) 97.60%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #6254   +/-   ##
=======================================
  Coverage   97.60%   97.60%           
=======================================
  Files        1116     1116           
  Lines       95848    95911   +63     
=======================================
+ Hits        93556    93618   +62     
- Misses       2292     2293    +1     
Files Changed Coverage Δ
...rq_google/cloud/quantum_v1alpha1/types/__init__.py 100.00% <ø> (ø)
...q-core/cirq/sim/density_matrix_simulation_state.py 99.02% <100.00%> (+0.10%) ⬆️
cirq-core/cirq/sim/simulation_state_test.py 98.51% <100.00%> (ø)
cirq-google/cirq_google/cloud/quantum/__init__.py 100.00% <100.00%> (ø)
...gle/cirq_google/cloud/quantum_v1alpha1/__init__.py 100.00% <100.00%> (ø)
...irq_google/cloud/quantum_v1alpha1/types/quantum.py 100.00% <100.00%> (ø)
cirq-google/cirq_google/engine/engine_client.py 100.00% <100.00%> (ø)
...rq-google/cirq_google/engine/engine_client_test.py 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Copy Markdown
Collaborator

@verult verult left a comment

Choose a reason for hiding this comment

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

LGTM minus comments

Comment thread cirq-google/cirq_google/engine/engine_client.py
Comment thread cirq-google/cirq_google/engine/engine_client.py Outdated
Comment thread cirq-google/cirq_google/engine/engine_client.py
Comment thread cirq-google/cirq_google/engine/engine_client.py Outdated
Comment thread cirq-google/cirq_google/engine/engine_client_test.py Outdated
Comment thread cirq-google/cirq_google/engine/engine_client_test.py Outdated
Copy link
Copy Markdown
Collaborator

@wcourtney wcourtney left a comment

Choose a reason for hiding this comment

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

LGTM % a couple of nits. Thanks!

Comment thread cirq-google/cirq_google/engine/engine_client.py Outdated
)


def _fix_create_job_processor_id_args_and_kwargs(args, kwargs):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: omit "args_and_kwargs" from the name since they're already in the parameters list.

Also - since this is intended only to be used by the "rewrite" clause of the @deprecated_parameter, consider using that nomenclature and describing the action of the method, eg. rewrite_processor_ids_to_processor_id

@verult verult merged commit 8bd2161 into quantumlib:master Aug 28, 2023
@jurruti
Copy link
Copy Markdown
Contributor Author

jurruti commented Aug 28, 2023

In the last few commits I added the rewrite function to the deprecated parameter decorator. Now we only get requests for the quantum engine where processor_id is set instead of processor_ids in the processor selector . Therefore we can now delete processor_ids from the Quantum Engine API and refactor without affecting the cirq-google behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: L 250< lines changed <1000

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants