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.

Sign up for GitHub

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

Jump to bottom

Edit cutting explanation #657

Merged
merged 5 commits into from
Aug 20, 2024
Merged

Edit cutting explanation #657

merged 5 commits into from
Aug 20, 2024

Conversation

ibrahim-shehzad
Copy link
Collaborator

Correct the $\theta$ parameter in the explanatory material but replacing it with $-\theta/2$.

@coveralls
Copy link

coveralls commented Aug 19, 2024 •
edited
Loading

Pull Request Test Coverage Report for Build 10464180802

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%
Totals Coverage Status
Change from base Build 10218959980: 0.0%
Covered Lines: 2410
Relevant Lines: 2410
💛 - Coveralls

@ibrahim-shehzad ibrahim-shehzad self-assigned this Aug 19, 2024
@ibrahim-shehzad ibrahim-shehzad added documentation Improvements or additions to documentation stable backport potential Suitable to be backported to most recent stable branch by Mergify labels Aug 19, 2024
@garrison
Copy link
Member

Thanks for catching this and submitting a PR. This error is due to a convention mismatch: there is a prefactor in the exponential in the definition of the RZZGate, but this was not in the original gate cutting paper. The code correctly accounts for this conversion, but the documentation does not until we merge the current PR.

https://github.com/Qiskit-Extensions/circuit-knitting-toolbox/blob/a9bfc91b9855a78fd6b0dd9ffadcd03069808fc7/circuit_knitting/cutting/qpd/decompositions.py#L393-L395

garrison
garrison approved these changes Aug 20, 2024
View reviewed changes
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, thank you 💯

@ibrahim-shehzad ibrahim-shehzad merged commit 812341a into main Aug 20, 2024
11 checks passed
@ibrahim-shehzad ibrahim-shehzad deleted the fix-cutting-docs branch August 20, 2024 13:48
mergify bot pushed a commit that referenced this pull request Aug 20, 2024
@ibrahim-shehzad @mergify
Edit cutting explanation (#657)
3365d8e 
* edit cutting explanation

* edit cutting explanation

* adding missing parenthesis

* add remaining parenthesis

(cherry picked from commit 812341a)
@mergify mergify bot mentioned this pull request Aug 20, 2024
Merged
garrison added a commit that referenced this pull request Aug 20, 2024
@mergify @ibrahim-shehzad @garrison
Edit cutting explanation (#657) (#658)
ab5ea64 
* edit cutting explanation

* edit cutting explanation

* adding missing parenthesis

* add remaining parenthesis

(cherry picked from commit 812341a)

Co-authored-by: Ibrahim Shehzad <[email protected]>
Co-authored-by: Jim Garrison <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@garrison garrison garrison approved these changes

Assignees

@ibrahim-shehzad ibrahim-shehzad

Labels
documentation Improvements or additions to documentation stable backport potential Suitable to be backported to most recent stable branch by Mergify
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ibrahim-shehzad @coveralls @garrison