Skip to content

Reuse VF2 scoring views for all scoring (backport #11115)#11135

Merged
jakelishman merged 1 commit into
stable/0.45from
mergify/bp/stable/0.45/pr-11115
Oct 27, 2023
Merged

Reuse VF2 scoring views for all scoring (backport #11115)#11135
jakelishman merged 1 commit into
stable/0.45from
mergify/bp/stable/0.45/pr-11115

Conversation

@mergify
Copy link
Copy Markdown
Contributor

@mergify mergify Bot commented Oct 27, 2023

This is an automatic backport of pull request #11115 done by Mergify.


Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

  • @Mergifyio refresh will re-evaluate the rules
  • @Mergifyio rebase will rebase this PR on its base branch
  • @Mergifyio update will merge the base branch into this PR
  • @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

  • look at your merge queues
  • generate the Mergify configuration with the config editor.

Finally, you can contact us on https://mergify.com

* Reuse VF2 scoring views for all scoring

As part of the VF2Layout and VF2PostLayout passes when there are a large
number of matches found we're spending an inordinate amount of time in
scoring rebuilding the same views over and over again of the interaction
graph for each scoring call. For example, in one test cProfile showed
that with #11112 when running transpile() on a 65 Bernstein Vazirani
circuit with a secret of all 1s for FakeSherbrooke with
optimization_level=3 we were calling vf2_utils.score_layout() 161,761
times which took a culmulative time of 14.33 secs. Of that time though
we spent 5.865 secs building the edge list view.

These views are fixed for a given interaction graph which doesn't change
during the duration of the run() method on these passes. To remove this
inefficiency this commit moves the construction of the views to the
beginning of the passes and just reuses them by reference for each
scoring call, avoiding the reconstruction overhead.

* Add EdgeList Rust pyclass to avoid repeated conversion

This commit adds a new pyclass written in rust that wraps a rust
Vec. Previously the scoring function also used an dict->IndexMap
conversion, but the mapping structure wasn't necessary and added
additional overhead, so it was converted to a list/Vec to speed up the
execution even further. By using this new pyclass as the input to the
rust scoring function we avoid converting the edge list from a list to
an Vec on each call which will reduce the overhead even further.

(cherry picked from commit a67fe87)
@mergify mergify Bot requested a review from a team as a code owner October 27, 2023 19:35
@qiskit-bot
Copy link
Copy Markdown
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:

@github-actions github-actions Bot added performance Changelog: None Do not include in the GitHub Release changelog. Rust This PR or issue is related to Rust code in the repository labels Oct 27, 2023
@github-actions github-actions Bot added this to the 0.45.0 milestone Oct 27, 2023
@jakelishman jakelishman enabled auto-merge October 27, 2023 19:39
@coveralls
Copy link
Copy Markdown

Pull Request Test Coverage Report for Build 6671493818

  • 30 of 37 (81.08%) changed or added relevant lines in 4 files are covered.
  • 7 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.003%) to 86.876%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit/transpiler/passes/layout/vf2_utils.py 12 14 85.71%
crates/accelerate/src/vf2_layout.rs 14 19 73.68%
Files with Coverage Reduction New Missed Lines %
qiskit/pulse/library/waveform.py 3 93.75%
crates/qasm2/src/lex.rs 4 90.66%
Totals Coverage Status
Change from base Build 6670973451: 0.003%
Covered Lines: 73866
Relevant Lines: 85025

💛 - Coveralls

@jakelishman jakelishman added this pull request to the merge queue Oct 27, 2023
Merged via the queue into stable/0.45 with commit 6af66a1 Oct 27, 2023
@mergify mergify Bot deleted the mergify/bp/stable/0.45/pr-11115 branch October 27, 2023 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Changelog: None Do not include in the GitHub Release changelog. performance Rust This PR or issue is related to Rust code in the repository

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants