Deprecate QuantumEngineSampler #5432
Conversation
- Deprecates this class in favor of ProcessorSampler - Moves get_engine_sampler into engine with the other utility functions - Change engine.get_sampler to only support a single processor and have it point to the processor's sampler. Fixes: quantumlib#5371
dstrain115
requested review from
wcourtney,
a team,
vtomole,
cduck and
verult
as code owners
|
@mrwojtek FYI: QuantumEngineSampler is being deprecated in favor of ProcessorSampler. Let me know if you see any problems with the changes to the calibration workflow code. |
| processor_id = getattr(sampler.processor, 'processor_id', None) | ||
| engine = sampler.processor.engine() if processor_id is not None else None |
There was a problem hiding this comment.
Seems odd to check for the presence of a processor_id attribute to determine whether to call .engine(). Do we need a check at all, given that AbstractProcessor.engine() method is defined as returning Optional[AbstractEngine]?
There was a problem hiding this comment.
Yeah, there were some typing shenanigans that made this necessary. I refactored so that the function below takes a processor instead of an engine, which removes the need for this.
| @@ -801,21 +800,29 @@ def sampler( | |||
| @util.deprecated_gate_set_parameter | |||
| def get_sampler( | |||
| self, processor_id: Union[str, List[str]], gate_set: Optional[Serializer] = None | |||
| ) -> engine_sampler.QuantumEngineSampler: | |||
| ) -> cirq.Sampler: | |||
There was a problem hiding this comment.
Here, too, might be better to type as returning ProcessorSampler.
| @@ -782,7 +781,7 @@ def get_processor(self, processor_id: str) -> engine_processor.EngineProcessor: | |||
| @util.deprecated_gate_set_parameter | |||
| def sampler( | |||
| self, processor_id: Union[str, List[str]], gate_set: Optional[Serializer] = None | |||
| ) -> engine_sampler.QuantumEngineSampler: | |||
| ) -> cirq.Sampler: | |||
There was a problem hiding this comment.
Should this return ProcessorSampler instead?
There was a problem hiding this comment.
Yeah, it's probably better for this to be more specific.
| @@ -877,3 +884,30 @@ def get_engine_calibration( | |||
| May return None if no calibration metrics exist for the device. | |||
| """ | |||
| return get_engine(project_id).get_processor(processor_id).get_current_calibration() | |||
|
|
|||
|
|
|||
| def get_engine_sampler(processor_id: str, project_id: Optional[str] = None) -> cirq.Sampler: | |||
There was a problem hiding this comment.
Here, too, consider typing the return as ProcessorSampler.
| A `QuantumEngineSampler` instance. | ||
|
|
||
| Raises: | ||
| ValueError: If the supplied gate set is not a supported gate set name. |
There was a problem hiding this comment.
nit: remove this as there's no gate_set_name parameter.
| Args: | ||
| processor_id: Engine processor ID (from Cloud console or | ||
| ``Engine.list_processors``). | ||
| gate_set_name: One of ['sqrt_iswap', 'sycamore']. |
| personal project IDs in shared code. | ||
|
|
||
| Returns: | ||
| A `QuantumEngineSampler` instance. |
There was a problem hiding this comment.
nit: I'd suggest removing this, since it doesn't add anything beyond the return type annotation. If you want to leave it, can just say it returns a sampler.
| @@ -877,3 +884,26 @@ def get_engine_calibration( | |||
| May return None if no calibration metrics exist for the device. | |||
| """ | |||
| return get_engine(project_id).get_processor(processor_id).get_current_calibration() | |||
|
|
|||
|
|
|||
| def get_engine_sampler( | |||
There was a problem hiding this comment.
Why was this moved? Just for better organizational sense?
|
nice; when I was doing #5361 I was worried the calibration dispatching would be a mess; but you've done it very nicely |
* Deprecate QuantumEngineSampler - Deprecates this class in favor of ProcessorSampler - Moves get_engine_sampler into engine with the other utility functions - Change engine.get_sampler to only support a single processor and have it point to the processor's sampler. - Fix calibration workflow to use ProcessorSampler Fixes: quantumlib#5371
it point to the processor's sampler.
Fixes: #5371