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

Introduce Dynamic Definition function to the wire cutting modules. #285

Merged
merged 36 commits into from Aug 18, 2023

Conversation

hitomitak
Copy link
Contributor

@hitomitak hitomitak commented Jun 23, 2023

Ported Dynamic Definition code to the wire cutting function and added its test. This function has the following features.

  • Create a bin from each simulation results (dd_bin)
  • Reconstruct the full probability distribute from bin (reconstruct_dd_full_distribute)

The original code used a file to perform the merge process of each result but this PR changes to do it on memory.
The following code is the example of their use.

subcircuit_instance_probabilities = evaluate_subcircuits(cuts)
dd_bins = create_dd_bin(
    subcircuit_instance_probabilities,  cuts, mem_limits=3 , recursion_depth=10 
)
prob = reconstruct_dd_full_distribution(circuit, cuts, dd_bins)
metrics, _ = verify(circuit, prob)

@coveralls
Copy link

coveralls commented Jun 23, 2023

Pull Request Test Coverage Report for Build 5482095635

  • 194 of 194 (100.0%) changed or added relevant lines in 3 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+1.0%) to 91.0%

Totals Coverage Status
Change from base Build 5433896893: 1.0%
Covered Lines: 2629
Relevant Lines: 2889

💛 - Coveralls

@caleb-johnson
Copy link
Collaborator

This looks great, thanks! Looks like the linter is failing.

You should be able to

pip install black
black .

from the root of the repo. You can also run tox -elint locally (I think) to see if there are any other linter errors.

Copy link
Collaborator

@caleb-johnson caleb-johnson left a comment

Choose a reason for hiding this comment

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

Really nice work, and thanks again for porting this over. We have had this as a to-do for a while now :)

Most of my comments are just pedantic docstring cleanups and commented print statement cleanups.

If you could take a look at the linter (currently failing the black --check) and also the two lines missing from coverage in the new code. If you can cover those two lines with a couple simple tests, we like to do that. If it's a pain to cover those lines for some reason, we can ignore them for coverage explicitly.

circuit_knitting/cutting/cutqc/wire_cutting.py Outdated Show resolved Hide resolved
circuit_knitting/cutting/cutqc/wire_cutting.py Outdated Show resolved Hide resolved
@garrison garrison added enhancement New feature or request cutqc Related to the wire cutting code based on the CutQC paper, arXiv:2012.02333 labels Jun 26, 2023
hitomitak and others added 22 commits June 26, 2023 10:29
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
Co-authored-by: Caleb Johnson <calebj1524@outlook.com>
@hitomitak
Copy link
Contributor Author

Thank you for reviewing. I should have combined your suggestions with batch into one commit. Sorry for the dirty commit log😰

I added the test to achieve 100% coverage and executed lint programs.

@caleb-johnson
Copy link
Collaborator

Thank you for reviewing. I should have combined your suggestions with batch into one commit. Sorry for the dirty commit log😰

I added the test to achieve 100% coverage and executed lint programs.

Hah! No worries at all! Thanks for the quick turnaround

@caleb-johnson
Copy link
Collaborator

I'm going to give @garrison a chance to look at this, and I'm going to make another pass shortly, but it's looking great

@garrison
Copy link
Member

Please add the following line to the body of one of your commit messages, in order to acknowledge the original author of this code. (You could even create an empty commit, using git commit --allow-empty.)

Co-authored-by: weiT1993 <tangwei1027@gmail.com>

Co-authored-by: weiT1993 <tangwei1027@gmail.com>
@hitomitak
Copy link
Contributor Author

I added the co-author and fix lint.

@garrison
Copy link
Member

Actually, for our purposes, I think it would be better to include the full text of the license in the file.

This PR deserves a release note, too.

@garrison
Copy link
Member

I updated reconstruct_dd_full_distribute -> reconstruct_dd_full_distribution in 9c9164f, so it would be parallel to the existing name reconstruct_full_distribution.

@garrison garrison merged commit 4b3cc99 into Qiskit-Extensions:main Aug 18, 2023
10 checks passed
@garrison
Copy link
Member

Thank you again for this!! It will be in the v0.3 release, which we expect to tag shortly. 🎉

@caleb-johnson
Copy link
Collaborator

Thanks @hitomitak !

@hitomitak
Copy link
Contributor Author

Sorry I was on summer vacation and could not respond. Thanks for the merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cutqc Related to the wire cutting code based on the CutQC paper, arXiv:2012.02333 enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants