Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ResetAfterMeasureSimplification transpiler pass #8330

Merged
merged 7 commits into from
Sep 2, 2022

Conversation

mtreinish
Copy link
Member

Summary

This commit adds a new transpiler pass to simplify resets after a
measurement. This pass when run will replace any reset after a
measurement with a conditional X gate. This is because the reset
operation on IBM backends is implemented by performing a conditional x
gate after a reset. So doing this simplification will improve the
fidelity of the circuit because we're removing a duplicate measurement
which was implicit in the reset. This pass is based on the marz library:
https://github.com/Qiskit-Partners/marz which did the same thing but at
the QuantumCircuit level.

One note is that this pass is basically specific to IBM backends so it's
not added to the preset pass managers. Ideally we'd be able to have the
IBM backends run this as part of the init stage or something to do the
logical transformation early in the compilation. But right now there is
no mechanism to do this (see #8329), so for now having the pass and
letting users specify it in the pass manager directly is the best
option. After #8329 is implemented we can look at adding this pass to
that hook interface in the ibm provider's backends directly so that they
can leverage this optimization whenever they're the compilation target.

Details and comments

This commit adds a new transpiler pass to simplify resets after a
measurement. This pass when run will replace any reset after a
measurement with a conditional X gate. This is because the reset
operation on IBM backends is implemented by performing a conditional x
gate after a reset. So doing this simplification will improve the
fidelity of the circuit because we're removing a duplicate measurement
which was implicit in the reset. This pass is based on the marz library:
https://github.com/Qiskit-Partners/marz which did the same thing but at
the QuantumCircuit level.

One note is that this pass is basically specific to IBM backends so it's
not added to the preset pass managers. Ideally we'd be able to have the
IBM backends run this as part of the init stage or something to do the
logical transformation early in the compilation. But right now there is
no mechanism to do this (see Qiskit#8329), so for now having the pass and
letting users specify it in the pass manager directly is the best
option. After Qiskit#8329 is implemented we can look at adding this pass to
that hook interface in the ibm provider's backends directly so that they
can leverage this optimization whenever they're the compilation target.
@mtreinish mtreinish requested a review from nonhermitian July 12, 2022 13:45
@mtreinish mtreinish requested a review from a team as a code owner July 12, 2022 13:45
@qiskit-bot
Copy link
Collaborator

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:

  • @Qiskit/terra-core

@coveralls
Copy link

coveralls commented Jul 12, 2022

Pull Request Test Coverage Report for Build 2975962918

  • 24 of 24 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.009%) to 84.201%

Totals Coverage Status
Change from base Build 2975949685: 0.009%
Covered Lines: 57040
Relevant Lines: 67743

💛 - Coveralls

Copy link
Contributor

@taalexander taalexander left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not all IBM backends implement reset with a conditional gate. Furthermore, as only IBM dynamic circuit backends are capable of applying the conditional operations and exploiting for performance I would recommend adding this pass to the IBM provider as in Qiskit/qiskit-ibm-provider#367

@nonhermitian
Copy link
Contributor

nonhermitian commented Jul 12, 2022

Not all IBM backends implement reset with a conditional gate

I do not think that that actually matters here provided that conditional x gates are more efficient then whatever you use for reset. But it is an interesting point that I was unaware of.

As it turns out, @mtreinish and myself were talking in parallel about transpiler passes in the IBM provider.

nonhermitian
nonhermitian previously approved these changes Jul 13, 2022
Copy link
Contributor

@nonhermitian nonhermitian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is good from my end, but of course I am biased. The substitute_node_with_dag is nice, and I wish I thought of that when I was hacking around with the DAG.

I would say though that this is not 100% IBM specific, but the applicability to other platforms depends on the details; If your measurement does project into the computational basis states and your conditional-x gate (provided you can do them) is lower in error than whatever reset mechanism your using then you should win. That being said, I would agree that this is a likely candidate for the IBM provider / the notion of a staged passmanager

Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it can still be reasonable to include the pass in Terra, even if it was originally designed for certain IBM-specific backends. If nothing else, it's a nice example of a simple optimisation pass, but I do also think it's going to be a reasonable optimisation that quite a few backends will want to include in their backend-specific "codegen" transpiler stages. I'd have a problem with it if we added it to the default pass managers (which this PR doesn't do), because then we'd be making unfair assumptions about backends.

jakelishman
jakelishman previously approved these changes Aug 15, 2022
Copy link
Member

@jakelishman jakelishman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks fine to me. Did we decide to move forwards with including it in Terra as an example (especially now there's more extensibility in the preset pass managers), or did we decide it's too IBM specific?

(My vote, as above, is that it's fine to include as a possible plugin, just not in the preset pass managers.)

@nonhermitian
Copy link
Contributor

I vote for adding it.

@mtreinish mtreinish requested a review from jakelishman August 31, 2022 14:39
@mtreinish mtreinish added the Changelog: New Feature Include in the "Added" section of the changelog label Aug 31, 2022
@mergify mergify bot merged commit ab52d8b into Qiskit:main Sep 2, 2022
@mtreinish mtreinish deleted the reset-after-measure-simplification branch September 28, 2022 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: New Feature Include in the "Added" section of the changelog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants