Skip to content

Conversation

@augustehirth
Copy link
Collaborator

Some phrasing changes to the new classical control section. Ran a formatter.

@augustehirth augustehirth requested review from a team, cduck and vtomole as code owners June 14, 2022 11:45
@CirqBot CirqBot added the size: M 50< lines changed <250 label Jun 14, 2022
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

"print(circuit)"
"print(circuit)\n",
"\n",
"# Simulate the message and teleport circuits for bloch vectors\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Capitalize Bloch outside variable names.

Also, why are we using Bloch vectors here? I guess bloch_vector_from_state_vector is convenient for comparing individual qubits out of the entire state, but it still feels kind of roundabout.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

That's exactly why. It's a slight trade-off of adding more code so that there is more direct printed content indicating that the two qubits are in the same state. The final_state_vectors by themselves are not directly comparable and just showing counts from results could obfuscate the point because different states could produce the same results. Is there a better way of seeing the state of a single qubit?

Copy link
Collaborator

Choose a reason for hiding this comment

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

You're probably right about it being the clearest comparison - I couldn't find a better alternative in a quick search. Let's keep the code as-is but clarify why we're using Bloch vectors in the comment.

Copy link
Collaborator

@95-martin-orion 95-martin-orion left a comment

Choose a reason for hiding this comment

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

Ready to merge once the clarifying comment on Bloch vector usage is added.

@augustehirth augustehirth added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jun 15, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jun 15, 2022
@CirqBot
Copy link
Collaborator

CirqBot commented Jun 15, 2022

Automerge cancelled: A status check is failing.

@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 Jun 15, 2022
@MichaelBroughton MichaelBroughton added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jun 15, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jun 15, 2022
@CirqBot
Copy link
Collaborator

CirqBot commented Jun 15, 2022

Automerge cancelled: A required status check is not present.

Missing statuses: ['Misc check', 'Pytest Ubuntu (3.7)', 'Pytest Ubuntu (3.8)', 'Pytest Ubuntu (3.9)', 'Pytest Windows (3.7)', 'Pytest Windows (3.8)', 'Pytest Windows (3.9)', 'Typescript lint check', 'Typescript tests', 'Typescript tests coverage']

@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 Jun 15, 2022
@augustehirth augustehirth added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jun 16, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jun 16, 2022
@CirqBot CirqBot merged commit 2547933 into quantumlib:master Jun 16, 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 Jun 16, 2022
@augustehirth augustehirth deleted the classical_control_phrasing branch June 16, 2022 09:47
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
Some phrasing changes to the new classical control section. Ran a formatter.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size: M 50< lines changed <250

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants