Add Target methods to query the target for instruction properties #9158
Conversation
This commit adds 6 new helper methods to the Target class. These methods are used to query the target by either name or class to extract the InstructionProperties (or attributes of them) for a given instruction. Previously doing this by name was possible using the mapping protocol, but handling the combination of ideal gates with ``None`` at different levels in the internal dictionaries was cumbersome and error prone. These methods give a simple entry point to do the query which abstracts away the internal structure of the target. For the methods which use a class based lookup this wasn't easily accomplishable before, the closest available method would just return a boolean about whether the class was supported on the target or not. Implements Qiskit#8892
mtreinish
added
Changelog: New Feature
|
Thank you for opening a new pull request. Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient. While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone. One or more of the the following people are requested to review this:
|
Pull Request Test Coverage Report for Build 3490179528
💛 - Coveralls |
There was a problem hiding this comment.
Thanks for implementing this.
I'm still not sure how to handle the example in #8892. Lookup by class doesn't quite work here, because both operations are of the same class. We want to be able to get the properties for RZZGate(pi/6) and the properties for RZZGate(pi/4) separately. It has to be a lookup by class + parameter + qubit I think. If parameter is None it can give all matching classes. Similarly I think we should let qubit be None and return all matching instructions on any qubit.
It almost feels like we want to look up by the same thing that makes up circuit.data, but allow for wildcard flexibility too in the lookup.
| ) | ||
| return out_dict | ||
|
|
||
| def get_instruction_errors_by_class(self, operation_class, qargs, parameters=None): |
There was a problem hiding this comment.
personally I would be ok not having the special methods to get error and duration.. Since error and duration can be easily accessed from properties, I don't think they need dedicated lookups (and they are just two properties, there could be more). This would reduce the extra added methods from 6 to 2.
There was a problem hiding this comment.
I think the purpose of this PR is to provide wrapper of these attributes. Note that InstructionProperties is not exposed to developers who don't update gate properties by themselves. So hiding its implementation with these methods makes sense to me.
In transpiler passes requiring these information, the pattern of getattr(inst_props, "error", 0.0) or 0.0 is often used, so it seems to me like developers assume the case the dict value is not InstructionProperties (personally, I think they should use .error instead).
Anyways, there are several cases you cannot get error
- instruction is not defined (1)
- qargs is not defined for the instruction (2)
- dict value is missing for (instruction, qargs): No instruction set provided (3)
- dict value is provided but not
InstructionPropertiesinstance: Edge case (4) - dict value is provided but error is None value: Calibration is provided without benchmark, e.g. during gate bring-up (5)
This method returns None in all above cases. So this method reduces the complexity to access gate fidelity.
In accordance with convention, when None is returned, transpiler passes treat the instruction as ideal gate (i.e. zero-error) in the fidelity metric. I'm not sure if this is correct behavior in the case of (5). In this case, we could calculate coherence limit with device T1 and gate .duration, because we should know gate duration after calibration. This method can implement such logic since Target knows qubit T1.
| @@ -590,6 +590,245 @@ def operation_names_for_qargs(self, qargs): | |||
| raise KeyError(f"{qargs} not in target.") | |||
| return res | |||
|
|
|||
| def get_instruction_properties_by_class(self, operation_class, qargs, parameters=None): | |||
There was a problem hiding this comment.
this is nitpicky but the other methods in this class don't start with get_* so for consistency I would drop get_* here too
| ) | ||
| return out_dict | ||
|
|
||
| def get_instruction_errors_by_class(self, operation_class, qargs, parameters=None): |
There was a problem hiding this comment.
I think the purpose of this PR is to provide wrapper of these attributes. Note that InstructionProperties is not exposed to developers who don't update gate properties by themselves. So hiding its implementation with these methods makes sense to me.
In transpiler passes requiring these information, the pattern of getattr(inst_props, "error", 0.0) or 0.0 is often used, so it seems to me like developers assume the case the dict value is not InstructionProperties (personally, I think they should use .error instead).
Anyways, there are several cases you cannot get error
- instruction is not defined (1)
- qargs is not defined for the instruction (2)
- dict value is missing for (instruction, qargs): No instruction set provided (3)
- dict value is provided but not
InstructionPropertiesinstance: Edge case (4) - dict value is provided but error is None value: Calibration is provided without benchmark, e.g. during gate bring-up (5)
This method returns None in all above cases. So this method reduces the complexity to access gate fidelity.
In accordance with convention, when None is returned, transpiler passes treat the instruction as ideal gate (i.e. zero-error) in the fidelity metric. I'm not sure if this is correct behavior in the case of (5). In this case, we could calculate coherence limit with device T1 and gate .duration, because we should know gate duration after calibration. This method can implement such logic since Target knows qubit T1.
| qubit 0 which will accept any parameter (excluding fixed angle variants). | ||
|
|
||
| Returns: | ||
| dict: A dictionary mapping all the operation names to error rates for all matching |
There was a problem hiding this comment.
I'm not sure how this method will be used in practice. Even if we get multiple errors for different gate implementations, transpiler passes don't choose most efficient pulse gate , i.e. .add_calibration, so fidelity metric of optimized sequence doesn't match. I think we can drop these methods if we don't have practical use case. User can still get the same function by creating the reverse mapping of Target._gate_name_map.
Summary
This commit adds 6 new helper methods to the Target class. These methods are used to query the target by either name or class to extract the InstructionProperties (or attributes of them) for a given instruction. Previously doing this by name was possible using the mapping protocol, but handling the combination of ideal gates with
Noneat different levels in the internal dictionaries was cumbersome and error prone. These methods give a simple entry point to do the query which abstracts away the internal structure of the target.For the methods which use a class based lookup this wasn't easily accomplishable before, the closest available method would just return a boolean about whether the class was supported on the target or not.
Details and comments
Implements #8892