Return processor_id and project_id in get_qcs_objects_for_notebooks #5759
Conversation
dstrain115
requested review from
wcourtney,
a team,
vtomole,
cduck and
verult
as code owners
| @@ -17,11 +17,13 @@ | |||
|
|
|||
|
|
|||
| def test_get_device_sampler(): | |||
| result = get_qcs_objects_for_notebook() | |||
| assert result.device is cg.Sycamore | |||
| result = get_qcs_objects_for_notebook('not_a_valid_project_name') | |||
There was a problem hiding this comment.
Can we add tests using the new fields, or would that break some QCS behavior?
| @@ -17,11 +17,13 @@ | |||
|
|
|||
|
|
|||
| def test_get_device_sampler(): | |||
| result = get_qcs_objects_for_notebook() | |||
| assert result.device is cg.Sycamore | |||
| result = get_qcs_objects_for_notebook('not_a_valid_project_name') | |||
There was a problem hiding this comment.
I think it'd be good for the tests to continue to exercise the no-arguments call variant. Perhaps we can do both? (We already exercise passing both arguments on line 28 below.)
|
Ok. This is really tricky to test, since it involves authentication. If you test it locally and you have application default credentials, you are signed in as well, which further complicates things. Let me see what I can do. |
|
Tweaked the tests a bit. PTAL |
| @@ -63,8 +63,7 @@ def get_qcs_objects_for_notebook( | |||
| # Check for Google Application Default Credentials and run | |||
| # interactive login if the notebook is executed in Colab. In | |||
| # case the notebook is executed in Jupyter notebook or other | |||
| # IPython runtimes, no interactive login is provided, it is | |||
| # assumed that the `GOOGLE_APPLICATION_CREDENTIALS` env var is | |||
|
|
|||
There was a problem hiding this comment.
Unintended deletion?
| @@ -96,17 +100,18 @@ def get_qcs_objects_for_notebook( | |||
| print(f"Available processors: {[p.processor_id for p in processors]}") | |||
| print(f"Using processor: {processor.processor_id}") | |||
| if not project_id: | |||
| project_id = processor.project_id | |||
| project_id = getattr(processor, 'project_id', getattr(processor, '_project_name', None)) | |||
There was a problem hiding this comment.
Should this instead use is_virtual to determine the default project_id?
There was a problem hiding this comment.
Added this to AbstractLocalProcessor instead.
|
Tweaked things a bit so that it only returns virtual or prod, not the PhasedFSim simulator. Also upped the tests so that it covers all the lines and fixes the pylint stuff. |
dstrain115
added
the
automerge
CirqBot
added
the
front_of_queue_automerge
CirqBot
removed
automerge
…uantumlib#5759) This adds two fields in the QCSObjectForNotebook This will allow us some flexibility in google notebooks so as not to specify the processor_id. This will also be a step in enabling tests for these notebooks.
This adds two fields in the QCSObjectForNotebook
This will allow us some flexibility in google notebooks so as not to specify the processor_id. This will also be a step in enabling tests for these notebooks.