Skip to content

Conversation

@95-martin-orion
Copy link
Collaborator

Documents CircuitOperation usage on the "Circuits" page.

@95-martin-orion 95-martin-orion requested review from a team, cduck and vtomole as code owners May 25, 2022 16:31
@95-martin-orion 95-martin-orion requested a review from viathor May 25, 2022 16:31
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@95-martin-orion
Copy link
Collaborator Author

CC @augustehirth

@CirqBot CirqBot added the size: M 50< lines changed <250 label May 25, 2022
"id": "cDAVDT7bAwDO"
},
"source": [
"Especially useful is dropping the last moment (which are often just measurements): `circuit[:-1]`, or reversing a circuit: `circuit[::-1]`.\n"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does CircuitOperation add a link to the reference page or we need to write cirq.CircuitOperation ? Please change, here and elsewhere, if it's the latter.


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done for the first appearance of CircuitOperation - I think this is our strategy in general, similar to Wikipedia-style linking.

"id": "cDAVDT7bAwDO"
},
"source": [
"Especially useful is dropping the last moment (which are often just measurements): `circuit[:-1]`, or reversing a circuit: `circuit[::-1]`.\n"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should also add a section to explain circuit_op.repeat_until and circuit_op.mapped_circuit.


Reply via ReviewNB

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

repeat_until requires measurement and classical control, neither of which are discussed in this high-level doc. Happy to add mapped_circuit, though.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

"id": "cDAVDT7bAwDO"
},
"source": [
"Especially useful is dropping the last moment (which are often just measurements): `circuit[:-1]`, or reversing a circuit: `circuit[::-1]`.\n"
Copy link
Collaborator

Choose a reason for hiding this comment

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

qubits = [cirq.GridQubit(x, y) for x in range(3) for y in range(3)] can be qubits=cirq.GridQubit.square(3)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

"subcircuit_op = cirq.CircuitOperation(cirq.FrozenCircuit(CZ(q0, q1)))\n",
"repeated_subcircuit_op = subcircuit_op.repeat(2)\n",
"moved_subcircuit_op = subcircuit_op.with_qubit_mapping({q0: q2})\n",
"circuit = cirq.Circuit(\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

It might be useful to have some line comments to further explain this example.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done.

Copy link
Collaborator

@augustehirth augustehirth left a comment

Choose a reason for hiding this comment

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

Pending others' comments, looks great to me.

@95-martin-orion
Copy link
Collaborator Author

Pinging @dstrain115 or @tanujkhattar for final approval.

Copy link
Collaborator

@dstrain115 dstrain115 left a comment

Choose a reason for hiding this comment

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

LGTM

@95-martin-orion 95-martin-orion added the automerge Tells CirqBot to sync and merge this PR. (If it's running.) label Jun 2, 2022
@CirqBot CirqBot added the front_of_queue_automerge CirqBot uses this label to indicate (and remember) what's being merged next. label Jun 2, 2022
@CirqBot CirqBot merged commit dc19b85 into quantumlib:master Jun 2, 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 2, 2022
rht pushed a commit to rht/Cirq that referenced this pull request May 1, 2023
Documents `CircuitOperation` usage on the "Circuits" page.
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