Skip to content

Conversation

@dabacon
Copy link
Collaborator

@dabacon dabacon commented Apr 5, 2022

No description provided.

@dabacon dabacon requested review from a team, cduck and vtomole as code owners April 5, 2022 00:11
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@dabacon dabacon requested a review from mpharrigan April 5, 2022 00:11
@CirqBot CirqBot added the size: M 50< lines changed <250 label Apr 5, 2022
{
"cell_type": "markdown",
"metadata": {
"id": "9c07f9b01c71"
Copy link
Collaborator

@mpharrigan mpharrigan Apr 5, 2022

Choose a reason for hiding this comment

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

of the class cirq.Qid class.

too much class

>unitary evolution on three qudits, a qubit, a qutrit, and another qutrit,

so six things? 3 + 1 + 1+ 1. Maybe a colon is supposed to be here?


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.

yeah, think also that we decided that cirq.Class could be used for first instance and then not repeated. For notebooks this doesn't seem to matter as it doesn't deep link.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@dabacon
Copy link
Collaborator Author

dabacon commented Apr 5, 2022

Removed outputs from notebook.

@mpharrigan
Copy link
Collaborator

I just took a look at the beginning of this before realizing how extensive the changes were. I'm OOO until monday, so unassigning myself for review

@mpharrigan mpharrigan removed their assignment Apr 5, 2022
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 with a few nits.

},
"source": [
"Most of the time in quantum computation, we work with qubits, which are 2-level quantum systems. A qu-*d*-it is a generalization of a qubit to a d-level or d-dimension system.\n",
"Most of the time in quantum computation, we work with qubits, which are 2-level quantum systems. But it is possible to also define quantum computation with higher dimensional systems. A qu-*d*-it is a generalization of a qubit to a d-level or d-dimension system. For example, the state of a single qubit is a superposition of two basis states, $|\\psi\\rangle=\\alpha|0\\rangle+\\beta|1\\rangle$, whereas the state of a qudit for a three dimensional system is a superposition of three basis states $|\\psi\\rangle=\\alpha|0\\rangle+\\beta|1\\rangle+\\gamma|2\\rangle$.\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 don't think this is a complete sentence as written. Perhaps change to "However, it is possible..."

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

"In Cirq, qudits work exactly like qubits except they have a `dimension` attribute different than 2, and they can only be used with gates specific to that dimension. In cirq, both qubits and qudits are subclasses of the class `cirq.Qid`. \n",
"\n",
"Both qubits and qudits are represented by a `Qid` object.\n",
"To apply a gate to some qudits, the dimensions of the qudits must match the dimensions it works on. For example, if a gate represents a unitary evolution on three qudits, a qubit, a qutrit, and another qutrit, the gate's \"qid shape\" is `(2, 3, 3)` and its `on` method will accept exactly 3 `Qid`s with dimension 2, 3, and 3, respectively.\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add a colon or something here (or parentheses) to show that the qubit, qutrit and qutrit are the 3 qudits.

For example, if a gate represents a unitary evolution on three qudits**:** a qubit, a qutrit, and another qutrit, then the gate's ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

rewrote as three sentences to make clearer.

" \n",
" This gate acts on three-level systems. In the computational basis of\n",
" this system it enacts the transformation U|x> = |x + 1 mod 3>, or\n",
" in other words U|0> = |1>, U|1> = |2>, and U|2> = |0>.\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Optional. Consider using right angle bracket "〉" rather than ">".

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

" \"\"\"\n",
" \n",
" def _qid_shape_(self):\n",
" # By implementing this method this gate implements the\n",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Optional: Can this be a docstring with triple quotes rather than line comments? Same below.

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

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

CirqBot commented May 10, 2022

Automerge cancelled: A required status check is not present.

Missing statuses: ['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 May 10, 2022
@dabacon dabacon merged commit 13de219 into quantumlib:master May 10, 2022
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.

4 participants