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 DAGCircuit output option to OneQubitEulerDecomposer #9188

Merged
merged 3 commits into from
Dec 2, 2022

Conversation

mtreinish
Copy link
Member

@mtreinish mtreinish commented Nov 23, 2022

Summary

This commit adds a new option to the euler one qubit decomposer class,
use_dag, which when set to True will return a DAGCircuit object for the
decomposed matrix instead of a QuantumCircuit object. This is useful for
transpiler passes that call the decomposer so that they don't have to
convert from a circuit to a dag. The passes that use the decomposer are
also updated to use this new option.

This was originally attempted before in #5926 but was abandoned because
at the time there was no real performance improvement from doing this.
However, since then this class has changed quite a bit, and after #9185
the overhead of conversion between QuantumCircuit and DAGCircuit is a
more critical component of the runtime performance of the 1 qubit
optimization pass. By operating natively in DAGCircuit from the
decomposer we're able to remove this overhead and directly substitute
the output of the decomposer in the pass.

Details and comments

This PR is based on top of #9185 and is on hold until #9185 merges. After #9185 merges we will need to rebase this PR
and remove the on hold label.

@mtreinish mtreinish added on hold Can not fix yet performance Changelog: New Feature Include in the "Added" section of the changelog mod: quantum info Related to the Quantum Info module (States & Operators) labels Nov 23, 2022
@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:

@mtreinish mtreinish added this to the 0.23.0 milestone Nov 23, 2022
@coveralls
Copy link

coveralls commented Nov 23, 2022

Pull Request Test Coverage Report for Build 3605031077

  • 115 of 115 (100.0%) changed or added relevant lines in 3 files are covered.
  • 3 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.04%) to 84.596%

Files with Coverage Reduction New Missed Lines %
qiskit/circuit/quantumcircuitdata.py 1 85.85%
src/sabre_swap/layer.rs 2 98.95%
Totals Coverage Status
Change from base Build 3604374696: 0.04%
Covered Lines: 63151
Relevant Lines: 74650

💛 - Coveralls

This commit adds a new option to the euler one qubit decomposer class,
use_dag, which when set to True will return a DAGCircuit object for the
decomposed matrix instead of a QuantumCircuit object. This is useful for
transpiler passes that call the decomposer so that they don't have to
convert from a circuit to a dag. The passes that use the decomposer are
also updated to use this new option.

This was originally attempted before in Qiskit#5926 but was abandoned because
at the time there was no real performance improvement from doing this.
However, since then this class has changed quite a bit, and after Qiskit#9185
the overhead of conversion between QuantumCircuit and DAGCircuit is a
more critical component of the runtime performance of the 1 qubit
optimization pass. By operating natively in DAGCircuit from the
decomposer we're able to remove this overhead and directly substitute
the output of the decomposer in the pass.
@mtreinish mtreinish force-pushed the dagcircuit-euler-decompose-one branch from e66a2ae to f8029e8 Compare December 2, 2022 19:10
@mtreinish mtreinish removed the on hold Can not fix yet label Dec 2, 2022
jakelishman
jakelishman previously approved these changes Dec 2, 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.

I'm ok to merge this as-is, if you think the interior-mutability thing in the pfun etc isn't worth changing.

This commit updates the internal psx circuit generator function to pass
a dataclass with a field for the gates and phase instead of using 2
lists. This provides a cleaner interface for using a mutable reference
between the different functions used to add gates to the circuit.
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.

Yeah, this looks much cleaner, thanks.

@mergify mergify bot merged commit cace20b into Qiskit:main Dec 2, 2022
@mtreinish mtreinish deleted the dagcircuit-euler-decompose-one branch December 2, 2022 22:32
Cryoris pushed a commit to Cryoris/qiskit-terra that referenced this pull request Jan 12, 2023
* Add DAGCircuit output option to OneQubitEulerDecomposer

This commit adds a new option to the euler one qubit decomposer class,
use_dag, which when set to True will return a DAGCircuit object for the
decomposed matrix instead of a QuantumCircuit object. This is useful for
transpiler passes that call the decomposer so that they don't have to
convert from a circuit to a dag. The passes that use the decomposer are
also updated to use this new option.

This was originally attempted before in Qiskit#5926 but was abandoned because
at the time there was no real performance improvement from doing this.
However, since then this class has changed quite a bit, and after Qiskit#9185
the overhead of conversion between QuantumCircuit and DAGCircuit is a
more critical component of the runtime performance of the 1 qubit
optimization pass. By operating natively in DAGCircuit from the
decomposer we're able to remove this overhead and directly substitute
the output of the decomposer in the pass.

* Use a dataclass for internal gate list and phase tracking

This commit updates the internal psx circuit generator function to pass
a dataclass with a field for the gates and phase instead of using 2
lists. This provides a cleaner interface for using a mutable reference
between the different functions used to add gates to the circuit.

* Directly combine op node lists in optimize_1q_commutation
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 mod: quantum info Related to the Quantum Info module (States & Operators) performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants