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

Speed up ConsolidateBlocks pass #8779

Closed
mtreinish opened this issue Sep 22, 2022 · 5 comments · Fixed by #10467
Closed

Speed up ConsolidateBlocks pass #8779

mtreinish opened this issue Sep 22, 2022 · 5 comments · Fixed by #10467
Assignees
Labels
mod: transpiler Issues and PRs related to Transpiler performance type: feature request New feature or request

Comments

@mtreinish
Copy link
Member

mtreinish commented Sep 22, 2022

What should we add?

When running a large transpile() (with 1k circuit qubits) with optimization level 3 a substantial amount of time, ~32%, of the compilation is spent in the ConsolidateBlocks pass. For something that's supposed to take a block of gates on 2 qubit and replace it with an equivalent UnitaryGate this is a larger portion of time than would normally be expected. Looking at the profile:

Screenshot_2022-09-22_07-14-59

Most of the time seems to be spent in the Operator object creation. We had similar performance issues around the use of Operator to compute unitaries in Optimize1qGatesDecomposition and we moved to computing the matrix directly (which is admittedly easier to do for 1q) and sped up the pass significantly (see #5909). We should look at doing a similar optimization in ConsolidateBlocks and fall back to using Operator if there are gates in the block we can't work directly with a matrix on.

@mtreinish mtreinish added type: feature request New feature or request performance mod: transpiler Issues and PRs related to Transpiler labels Sep 22, 2022
@mtreinish mtreinish added this to the 0.23.0 milestone Sep 22, 2022
@jakelishman
Copy link
Member

A slight alternative of course is to speed up the Operator constructor path. There's no real reason that we should be calling QuantumCircuit.to_instruction to handle circuits (which I'm pretty sure involves a deepcopy), and then unwrapping the instructions back down to its definition again.

That said, the extra overhead the Operator class has (in things like qargs construction, and so using einsum instead of straight matmul) is still probably going to be way too high.

@raynelfss
Copy link
Contributor

Me and danielleodigie are interested in looking into this issue. Can we be assigned?

@jakelishman
Copy link
Member

@danielleodigie: because of GitHub weirdness, it's tricky for us to formally assign you unless you comment on this issue.

@danielleodigie
Copy link
Contributor

oops I didn't know that, I'd like to be assigned too!

@jakelishman
Copy link
Member

No worries - it's just one of those weird little GitHub things. Maybe it's to avoid people spamming famous people by assigning them on random little repos? I dunno.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mod: transpiler Issues and PRs related to Transpiler performance type: feature request New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants