Cirq icon indicating copy to clipboard operation
Cirq copied to clipboard

`Using processor: ` string is incorrect

Open eliottrosenberg opened this issue 2 years ago • 5 comments

Description of the issue

The Using processor: ... string that gets printed when you load a processor is not correct.

See this colab.

Cirq version 1.4.0.dev20240126200039

@senecameeks @wcourtney @NoureldinYosri

eliottrosenberg avatar Jan 26 '24 22:01 eliottrosenberg

Thanks for the report! It looks like that message comes from here. I personally don't like this side-effect at all since it's referring to a value that you intentionally ignore in the script. My vote would be to remove it, but I think that it would also go away if the function were used as it appears was intended. e.g., augmenting the colab:

project_id = "i-dont-have-experimental"
processor_id = "SYC02_4B_CANYON"

qcs_objects = get_qcs_objects_for_notebook(project_id, processor_id)

processor = qcs_objects.processor
...

I expect that the incantation provided in the colab comes from the quickstart, so I'd recommend

  1. Update the quickstart with the new form (or some variation) that doesn't result choosing the wrong processor in the utility.
  2. Remove the print statement entirely since there's no reason that the client needs to use the value and they can always print it if they like.

@eliottrosenberg - does that sound alright to you?

wcourtney avatar Jan 27 '24 01:01 wcourtney

That sounds good to me! I did not realize that qcs_objects already has device and sampler attributes. That's really convenient but wasn't explained in the quickstart.

eliottrosenberg avatar Jan 27 '24 06:01 eliottrosenberg

Actually, could we modify get_qcs_objects_for_notebook to take a run_name and device_config_name as optional inputs? That way its sampler attribute could already be the one that the user will use.

eliottrosenberg avatar Jan 27 '24 17:01 eliottrosenberg

I've always found the get_qcs_object_for_notebook function to be awkward, starting from the name, and I wonder if we can get away from it with a little bit of refactoring. For example, we already have a get_engine_sampler function, and it would be great if people can just do

sampler = cirq_google.get_engine_sampler(device, run_name, device_config)

The returned sampler has a processor attribute that returns the processor, so that one simple call should be all one needs to get started taking data.

maffoo avatar Jan 27 '24 19:01 maffoo

On second look, I think there are two different use cases that we're trying to combine.

  1. The quickstart tutorial is meant to familiarize the user with the resources and how they relate. In this case, we must begin with an engine object to list processors and can follow the resource tree as we introduce concepts. For this, we should probably just use the more straight-forward get_engine, which would also remove the message at the heart of this issue (https://github.com/quantumlib/Cirq/issues/6427).

  2. After the quickstart, the user is pointed to a template colab "as a base for your own explorations". IMO, is a good place to introduce a simplified setup when the user already knows what they want and just needs a simple configuration. Here, we should discuss the merits of updating get_qcs_objects_for_notebook to accept the run_name and device_config vs using the get_engine_* methods. It looks like this notebook could use a more thorough review and I'd rather track that work in a new issue.

wcourtney avatar Jan 29 '24 18:01 wcourtney