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

More performance improvements for Collect2qBlocks #6680

Merged
merged 19 commits into from
Aug 27, 2021

Conversation

IvanIsCoding
Copy link
Contributor

@IvanIsCoding IvanIsCoding commented Jul 4, 2021

Summary

Related to Qiskit/rustworkx#265

Uses the ported version of the single-sweep algorithm from #6534 in retworkx.

This PR brings a performance boost and removes the bottleneck from the graph traversal

Details and comments

Depends on retworkx 0.10 being released Retworkx 0.10 has been released, this PR is ready for review

@IvanIsCoding IvanIsCoding requested a review from a team as a code owner July 4, 2021 20:59
@IvanIsCoding
Copy link
Contributor Author

IvanIsCoding commented Jul 4, 2021

Results show roughly a 1.25x speed up compared to the main branch. Using the same benchmark from #6534:

import timeit
from qiskit import QuantumRegister, QuantumCircuit
from qiskit.circuit.library import QuantumVolume
from qiskit.converters import circuit_to_dag
from qiskit.transpiler.passes.optimization import Collect2qBlocks
from qiskit.compiler import transpile

N = 100
q = QuantumRegister(N, 'q')
qc = QuantumCircuit(q)
qc.append(
        QuantumVolume(N, N//2, seed=2, classical_permutation=False), q
)

qc = transpile(qc, optimization_level=0, basis_gates=['u1', 'u2', 'u3', 'cx'])
dag = circuit_to_dag(qc)
c2q = Collect2qBlocks()

print(
        timeit.timeit("c2q.run(dag)", globals=globals(), number=10)
)

Main: 0.89404s
#6680: 0.70438s

@ajavadia
Copy link
Member

ajavadia commented Jul 6, 2021

Are the logic improvements here reflected in the retworkx version?
#6534

@IvanIsCoding
Copy link
Contributor Author

Are the logic improvements here reflected in the retworkx version?
#6534

Yes, it should be identical. Which means this PR should be on hold until the issues #6686 are explored further

@kdk kdk added the on hold Can not fix yet label Jul 13, 2021
@IvanIsCoding IvanIsCoding changed the title [WIP] More performance improvements for Collect2qBlocks More performance improvements for Collect2qBlocks Aug 25, 2021
@mtreinish mtreinish added Changelog: None Do not include in changelog performance and removed on hold Can not fix yet labels Aug 27, 2021
mtreinish
mtreinish previously approved these changes Aug 27, 2021
Copy link
Member

@mtreinish mtreinish left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for doing this.

@mtreinish
Copy link
Member

I removed the automerge tag, since as @jakelishman reminded me this needs an upgrade release note documenting the increase in the minimum retworkx version used. I'll add this shortly and just merge it after

@jakelishman jakelishman added automerge Changelog: API Change Include in the "Changed" section of the changelog and removed Changelog: None Do not include in changelog labels Aug 27, 2021
@mergify mergify bot merged commit 32f5216 into Qiskit:main Aug 27, 2021
@IvanIsCoding IvanIsCoding deleted the collect_bicolor branch August 27, 2021 22:29
@kdk kdk added this to the 0.19 milestone Nov 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Changelog: API Change Include in the "Changed" section of the changelog performance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants