Deprecate use of BackendProperties (BackendV1) in transpilation pipeline
#13719
Conversation
|
One or more of the following people are relevant to this code:
|
…size safeness of removal.
There was a problem hiding this comment.
Overall, this looks about as clean as we'll manage, thanks Elena. I think none of my comments are actionable for this PR.
I wish that we'd handled this move a little cleaner overall - if the Target was supposed to supersede BackendProperties, I wish we'd had generate_preset_pass_manager and transpile set that option to None the second a Target was passed (or at least overwrite them with the data re-derived from the Target so we knew it was identical), which we could have introduced at any point, especially once all the transpiler passes were migrated, but oh well, too late for that now.
As it stands, we're relying on each individual pass to make sure it prioritises the Target, and of the ones touched by this PR, either there's (minor) bugs in their implementations of that, or we're interpreting "priority" different in some places.
There was a problem hiding this comment.
In DenseLayout, there's a path where a disagreement between the Target and BackendProperties can cause the backend_props argument to be used in place of the Target (if Target.qargs is None). That might be a bug - perhaps in _build_error_matrix we should have
if target is not None:
if target.qargs is None:
return zeros
# ... build errors
elif backend_prop and coupling_map:
# ... build errors
return errorsinstead of what we have right now, which is the target.qargs is not None being in the top-line if.
If a bug, it can be a follow-up.
There was a problem hiding this comment.
Hmmm this does look like a bug, but the bug would only affect 1.4 because the elif backend_props branch is anyways going away in 2.0. Given that 1.4 is supposed to not include fixes, and we are probably too late for 1.3.3, should we just... let the "legacy" behavior be even if it's inconsistent with the prioritization of the target?
There was a problem hiding this comment.
We have the same if target.is not None and target.qargs is not None maybe-problem in vf2_utils.build_average_error_map. Also affects VF2PostLayout.
There was a problem hiding this comment.
In _build_gate_errors in this file, we use both Target and BackendProperties if they're both given. The other similar _build_* functions use only Target if given. Don't know if it's a bug or intentional.
That's probably not a huge deal for the deprecation - the alternate path would just be to update the Target if it was intended behaviour anyway.
There was a problem hiding this comment.
This is surely a bug, but the rust path currently skips these gate errors so it will not affect the code when the plugin is set to "default" (yes, a cleanup of this python code is pending).
releasenotes/notes/deprecate-backend-v1-props-09481526448e6bfd.yaml
Outdated
Show resolved
Hide resolved
releasenotes/notes/deprecate-backend-v1-props-09481526448e6bfd.yaml
Outdated
Show resolved
Hide resolved
Co-authored-by: Jake Lishman <jake@binhbar.com>
…om/ElePT/qiskit-terra into deprecate-backend-props-transpiler
Summary
This PR deprecates the following uses of
BackendPropertiesto be removed in 2.0:backend_propinput argument inDenseLayoutpropertiesinput argument inVF2Layoutpropertiesinput argument inVF2PostLayoutbackend_propsinput argument inUnitarySynthesisbackend_propertiesinPassManagerConfigbackend_propertiesingenerate_routing_passmanagerbackend_propertiesingenerate_translation_passmanagerbackend_propertiesinTarget.from_configurationDetails and comments
This PR unblocks #13706 and further 2.0 removals.