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

Print multi-qubit circuit with asymmetric depolarizing noise correctly #5931

Merged
merged 8 commits into from
Dec 19, 2022
Merged

Print multi-qubit circuit with asymmetric depolarizing noise correctly #5931

merged 8 commits into from
Dec 19, 2022

Conversation

paaige
Copy link
Contributor

@paaige paaige commented Oct 26, 2022 •
edited
Loading

Closes #5927. Fixes based on discussion in the issue.

@paaige paaige requested review from a team, vtomole and cduck as code owners October 26, 2022 20:12
@google-cla
Copy link

google-cla bot commented Oct 26, 2022

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@CirqBot CirqBot added the size: S 10< lines changed <50 label Oct 26, 2022
vtomole
vtomole previously requested changes Oct 27, 2022
View reviewed changes
Copy link
Collaborator

@vtomole vtomole left a comment

Choose a reason for hiding this comment

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

Congrats on your first pull request to Cirq @paaige!

You'll need to change the return type of the method you modified to make mypy happy followed by signing the CLA in order for us to merge this.

@tanujkhattar
Copy link
Collaborator

@paaige This is great. Please also update the _approx_eq_ method to fix another (unrelated) bug in the same class; as highlighted in the second part of my comment here: #5927 (comment)

tanujkhattar
tanujkhattar requested changes Dec 19, 2022
View reviewed changes
Copy link
Collaborator

@tanujkhattar tanujkhattar left a comment

Choose a reason for hiding this comment

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

LGTM % nit.

cirq-core/cirq/ops/common_channels.py Outdated Show resolved Hide resolved
@tanujkhattar tanujkhattar added the ANNOUNCE CHANGE For pull requests that are not breaking, but still important to mention in release notes. label Dec 19, 2022
@tanujkhattar tanujkhattar added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Dec 19, 2022
@CirqBot
Copy link
Collaborator

CirqBot commented Dec 19, 2022

Automerge cancelled: No approved review.

@CirqBot CirqBot removed the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Dec 19, 2022
tanujkhattar
tanujkhattar approved these changes Dec 19, 2022
View reviewed changes
Copy link
Collaborator

@tanujkhattar tanujkhattar left a comment

Choose a reason for hiding this comment

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

I made the minor changes so we can merge the PR. Thanks for the contribution!

@tanujkhattar tanujkhattar dismissed vtomole’s stale review December 19, 2022 19:56

I've made the changes, so the review is stale now.

@tanujkhattar tanujkhattar added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Dec 19, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Dec 19, 2022
@CirqBot CirqBot merged commit 7892143 into quantumlib:master Dec 19, 2022
@CirqBot CirqBot removed automerge Tells CirqBot to sync and merge this PR. (If it's running.) front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. labels Dec 19, 2022
harry-phasecraft pushed a commit to PhaseCraft/Cirq that referenced this pull request Oct 31, 2024
@paaige
Print multi-qubit circuit with asymmetric depolarizing noise correctly (
333a8e7 
quantumlib#5931)

Closes quantumlib#5927. Fixes based on discussion in the issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tanujkhattar tanujkhattar tanujkhattar approved these changes

@vtomole vtomole vtomole left review comments

@cduck cduck Awaiting requested review from cduck

@95-martin-orion 95-martin-orion Awaiting requested review from 95-martin-orion 95-martin-orion was automatically assigned from quantumlib/cirq-maintainers

Assignees
No one assigned
Labels
ANNOUNCE CHANGE For pull requests that are not breaking, but still important to mention in release notes. size: S 10< lines changed <50
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unable to print multi-qubit circuit with asymmetric depolarizing noise
4 participants
@paaige @tanujkhattar @CirqBot @vtomole