Optimize ConsolidateBlocks pass

[raynelfss]

Summary

Tries to address #8779
The following commits attempt to speedup the ConsolidateBlocks pass by calculating the unitary of each block inside of the pass rather than passing it as a QuantumCircuit and converting it to an Operator, which is redundant.

Details and comments

Known issues:

• When a circuit is generated from a qasm file, some non-traditional gates will not have a defined to_matrix method and the algorithm will throw an exception. Fixed in 653e23c.
• On Windows devices, the transpiler will throw the following error with certain circuits:

LinAlgError: eig algorithm (geev) did not converge (only eigenvalues with order >= 4 have converged)

Performance

The following is an example with a 1024 qubit random circuit of depth 128.

• Old ConsolidateBlocks:
  
• New ConsolidateBlocks:
  

[raynelfss]

• Old ConsolidateBlocks snakeviz:
• New ConsolidateBlocks snakeviz:
  

[qiskit-bot]

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]

Pull Request Test Coverage Report for Build 5616076303

• 40 of 46 (86.96%) changed or added relevant lines in 3 files are covered.
• 7 unchanged lines in 4 files lost coverage.
• Overall coverage remained the same at 85.954%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/transpiler/passes/optimization/consolidate_blocks.py	10	12	83.33%
qiskit/transpiler/passes/utils/block_to_matrix.py	29	33	87.88%

Files with Coverage Reduction	New Missed Lines	%
crates/qasm2/src/expr.rs	1	93.76%
qiskit/circuit/tools/pi_check.py	1	91.23%
qiskit/transpiler/passes/synthesis/unitary_synthesis.py	1	90.33%
crates/qasm2/src/lex.rs	4	91.39%

Totals
Change from base Build 5615993162:	0.0%
Covered Lines:	72903
Relevant Lines:	84816

---

💛 - Coveralls

[ewinston]

The _block_to_matrix method seems to be taking over some of the responsibilities which Operator would normally take. If such an improvement is possible then it seems better to place it with Operator so other parts of the code could benefit from it. If it is the case, as suggested, that there is some trade off between matmul and einsum as a function of matrix size it would be nice to understand where that trade-off lies (e.g. n=2, 3,...) and apply the switch there.

Actually I believe @chriseclectic orginally was using matmul in Operator but switched to einsum for speed but perhaps that was just based on larger matrices?

[jakelishman]

Erick: this PR is deliberately meant to take over some of what Operator does just like #5909 does, because of the overheads from Operator itself. There's potentially the case for some slightly faster paths to be added to Operator for the case of small numbers of qubits, but Operator is rarely performance critical, and is optimised more towards generality. There's overhead in managing things like the OpShape, parsing the qargs into a format, and then of course there's the einsum parts.

einsum lets Operator avoid constructing the implicit full-tensor-product space matrices when composing a smaller operator onto a larger one, and it allows general argument re-ordering without the explicit swaps that the PR in its current form adds in (though of course it's possible to elide the 2 matmul swap ops and do a direct permutation, or the manual 4-tensor transposition in Numpy). For the cases considered by this PR, there's no need for any of Operators general handling, and you can see in the results the huge improvement from avoiding that overhead.

[ewinston]

Can you show the profile result in table format sorted by cumtime and tottime for the top ~20 rows?

[raynelfss]

Can you show the profile result in table format sorted by cumtime and tottime for the top ~20 rows?

@ewinston
This is the profile result of the old version:

• By tottime:
  
• By cumtime:
  

This is for the optimized version:

• By tottime:
  
• By cumtime:
  

[ewinston]

@raynelfss It appears like the profiling includes other passes besides the one under test which makes the output a little hard to interpret. Don't worry about it for now though since we're just taking the step of moving the block_to_matrix to transpiler/passes/utils.

[ewinston]

An exception should be raised if the dimension of matrix != 4.

[raynelfss]

For this, it would be better to just throw an exception whenever len(block_index_map) != 2 . Since earlier in the code we know that block_index_map comes from the following operation:

https://github.com/Qiskit/qiskit-terra/blob/c87d00c90af0635561c97a8ae54cd7eeccf78fce/qiskit/transpiler/passes/optimization/consolidate_blocks.py#L106

Which takes block_qargs (The set of all qubits in the block) as an argument and returns a mapped dictionary of the same length. Before we call _block_to_matrix we will check that len(block_qargs) <= 2 to call it, otherwise, the old method will be used. As shown here:

https://github.com/Qiskit/qiskit-terra/blob/c87d00c90af0635561c97a8ae54cd7eeccf78fce/qiskit/transpiler/passes/optimization/consolidate_blocks.py#L112-L123

So it would be better to just add an exception at the beginning of block_to_matrix that says if the length of block_index_length exceeds 2 then throw an error. Which would cover for cases in which the matrix has a dimension that exceeds 2x2.

[ewinston]

The "or less" should be removed.