Add translation stage to IBM backend and update DD to use #483
Conversation
taalexander
force-pushed
the
taa-add-get-translation-stage
branch
from
f17fbcd to
3521936
Compare
Pull Request Test Coverage Report for Build 4139093156
💛 - Coveralls |
taalexander
force-pushed
the
taa-add-get-translation-stage
branch
from
3521936 to
4771006
Compare
| def get_translation_stage_plugin(self) -> str: | ||
| """Return the default translation stage plugin name for IBM backends.""" | ||
| return "ibm_backend" | ||
| return "ibm_dynamic_circuits" |
There was a problem hiding this comment.
I think doing this might cause problems for users who are using c_if but dynamic=False - the new plugin will convert the operations at optimisation levels 0 and 1, but then I think the job will fail went sent off to the next step in the pipeline?
I'm actually not sure what the solution is here, since dynamic=True and dynamic=False are handled by the same IBMBackend instance. We don't know if the user intends to set dynamic=True until Backend.run is called so get_translation_stage_plugin can't know it.
There was a problem hiding this comment.
Nobody can use c_if with dynamic=False AFAIK? The only other solution would be to set a different type of backend that is returned to the user from the provider?
There was a problem hiding this comment.
Can they not? I thought the old paths had some amount of support for it for like conditional resets? You'd know better than me - if c_if wouldn't have worked before, then I don't think there's any issue with this change.
There was a problem hiding this comment.
Yah, the old path doesn't support this (outside of simulators I suppose, in which case the simulators support the conversion regardless). The old path does support resets/MCM but these are unaffected.
taalexander
force-pushed
the
taa-add-get-translation-stage
branch
from
dc303bd to
8f4f41a
Compare
There was a problem hiding this comment.
Minor typos aside, this looks good to me.
I actually reviewed this offline with a
git diff thomas/dd-scf..thomas/taa-add-get-translation-stage
because the PR history has both branches, and GitHub doesn't offer great tools to separate the commits now that there's some date overlap between the two.
releasenotes/notes/add-translation-stage-plugin-to-backend-ed753e4323137d1c.yaml
Outdated
Show resolved
Hide resolved
Co-authored-by: Jake Lishman <[email protected]>
taalexander
force-pushed
the
taa-add-get-translation-stage
branch
from
8f4f41a to
9f3b880
Compare
taalexander
force-pushed
the
taa-add-get-translation-stage
branch
from
9f3b880 to
f8598ca
Compare
Summary
This PR builds on #480 and #420 to enable translation plugins for provider backends by default. This is a powerful feature that will solve usability issues with new-style control-flow.
As this is based on #420 for reviewing purposes look at - https://github.com/Qiskit/qiskit-ibm-provider/pull/483/files/4b446b04e25912f0de6a45a49e37532ed896a4bc..25f35228c0aad6ce3000fca7fc4aca2261c27a75
Details and comments
Currently the plugin is named
ibm_dynamic_circuitsbut applies transformations that are generally good for IBM backends. TheIBMBackendcurrently does not differentiate between dynamic/non-dynamic circuit backends but this might be desirable in the long run. Perhaps, in the near term we could renameibm_dynamic_circuitstoibmor something along these lines (or keep both and have them be identical).