Add new processor selector parameters in cirq-google/engine run methods #6267
Conversation
jurruti
requested review from
wcourtney,
vtomole,
cduck,
verult and
a team
as code owners
jurruti
changed the title
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #6267 +/- ##
=======================================
Coverage 97.87% 97.87%
=======================================
Files 1106 1106
Lines 95589 95624 +35
=======================================
+ Hits 93561 93596 +35
Misses 2028 2028
☔ View full report in Codecov by Sentry. |
| @@ -237,6 +263,8 @@ 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 = "", | |||
There was a problem hiding this comment.
Do we want to update calibration methods?
There was a problem hiding this comment.
We don't (at least not in this PR). Note that these arguments are for run_sweep, not run_calibration (github view is a bit confusing).
| @@ -64,6 +64,8 @@ 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 = "", | |||
There was a problem hiding this comment.
It seems more consistent with the other params to use Optional[str]; why the difference?
There was a problem hiding this comment.
I followed the decision we made in EngineClient to keep it as a String field instead of Optional[String] because there is no semantic difference between "" and None.
However, I do think it would be good to make them all Optional to have more consistency. I can do it in a different PR if that is ok.
| @@ -64,6 +64,8 @@ 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 = "", | |||
There was a problem hiding this comment.
Can't add a comment below, but should these be passed to self.run_sweep_async on line 99?
There was a problem hiding this comment.
Whoops, thank you for the catch.
This pull request (PR) adds two new parameters to the run methods of
EngineProgram,Engine, andEngineProcessor:run_nameanddevice_configuration_name. These parameters allow users to select a specific configuration for the processor when running the circuit. They are also added to theinitfunction ofProcessorSampler(and theget_samplermethods withinEngineandEngineProcessor).Additionally, the run methods of
EngineProgramandEnginenow include a parameter calledprocessor_id. This parameter can be used to specify a specific processor for the run, and will substitute the deprecated parameterprocessor_ids, because allowing multiple candidate processors was not useful in practice.