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

Remove transpilations inside subexperiment generation #556

Merged
merged 27 commits into from
Apr 24, 2024

Conversation

caleb-johnson
Copy link
Collaborator

@caleb-johnson caleb-johnson commented Apr 22, 2024

Fixes #529
Fixes #530

The copies used in the transpilation passes in subexperiment generation start becoming a bottleneck for larger experiments. This PR removes the transpilation in favor of modifying the QuantumCircuit objects directly and in-place. This is more in-line with the rest of our workflow and scales much better in that context.
subexp_gen

  • Release note

@coveralls
Copy link

coveralls commented Apr 22, 2024

Pull Request Test Coverage Report for Build 8820682434

Details

  • 46 of 46 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.05%) to 95.484%

Totals Coverage Status
Change from base Build 8819948894: 0.05%
Covered Lines: 3489
Relevant Lines: 3654

💛 - Coveralls

@caleb-johnson caleb-johnson changed the title No coverage for inplace Remove transpilations inside subexperiment generation Apr 22, 2024
@caleb-johnson
Copy link
Collaborator Author

caleb-johnson commented Apr 22, 2024

@garrison @ibrahim-shehzad I do think we should move away from DAGCircuit in our workflow until we decide to move entirely to it. This chart shows a pretty dramatic time cost as experiments scale up if we continue using transpiler passes to clean up resets.

@caleb-johnson
Copy link
Collaborator Author

Something funny going on with the reno linter in CI for this PR hmmm

@caleb-johnson caleb-johnson requested a review from garrison April 22, 2024 22:10
@caleb-johnson
Copy link
Collaborator Author

caleb-johnson commented Apr 22, 2024

Something funny going on with the reno linter in CI for this PR hmmm

Docs build is broken on main branch now as well. Submitted #559 to address

@caleb-johnson
Copy link
Collaborator Author

caleb-johnson commented Apr 23, 2024

The chart was made using Tutorial 1 at 100 qubits on the All-Z observable. So 2 partitions, 1 observable.

Copy link
Collaborator

@ibrahim-shehzad ibrahim-shehzad left a comment

Choose a reason for hiding this comment

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

The speedup is really impressive! Thanks for putting this all together, it looks great to me.

garrison
garrison previously approved these changes Apr 24, 2024
Copy link
Member

@garrison garrison left a comment

Choose a reason for hiding this comment

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

Looks great, thanks!

garrison added a commit that referenced this pull request Apr 24, 2024
This is due to python 3.8 no longer being available on the macos-latest CI runner.

This also restores the ubuntu minimum version to 3.8 so that we test on 3.8 somewhere.
#515 (comment)

We might see intermittent CI failures again until #556 is merged (expected to be very soon).
@caleb-johnson caleb-johnson merged commit f3fd581 into main Apr 24, 2024
11 checks passed
@caleb-johnson caleb-johnson deleted the no-transpiles branch April 24, 2024 18:21
garrison added a commit that referenced this pull request Apr 25, 2024
garrison added a commit that referenced this pull request Apr 25, 2024
garrison added a commit that referenced this pull request Apr 25, 2024
garrison added a commit that referenced this pull request May 16, 2024
* Bump macos-latest python version

This is due to python 3.8 no longer being available on the macos-latest CI runner.

This also restores the ubuntu minimum version to 3.8 so that we test on 3.8 somewhere.
#515 (comment)

We might see intermittent CI failures again until #556 is merged (expected to be very soon).

* Update test_latest_versions.yml

use python 3.12 on macos, because then we will skip cplex
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Time a transpilation routine for a utility-scale set of subexperiments Profile subexperiment generation
4 participants