Improve generic select documentation #105
Conversation
cirq_qubitization/generic_select.py
Outdated
| @@ -6,12 +7,23 @@ | |||
|
|
|||
|
|
|||
| class GenericSelect(unary_iteration.UnaryIterationGate): | |||
| r"""Gate that implements SELECT for a Hamiltonian expressed as an LCU. | |||
| r"""A generic controlled SELECT operation for applying Pauli strings. | |||
There was a problem hiding this comment.
"for applying Pauli strings" seems like an abrupt / incomplete ending.
Maybe "Operation to selectively apply Pauli's to a target register conditioned on integer stored in selection register." ?
I'm open to suggestions here.
There was a problem hiding this comment.
How about:
A generic controlled SELECT gate for selecting and applying operators from an array of PauliStrings
I think we can assume some familiarity with the notion of "SELECT is array indexing" at least in the first introductory line, so I don't think we need "conditioned on an integer" or anything like that.
There was a problem hiding this comment.
or "list" if not array
There was a problem hiding this comment.
7b1054f do you think this is ok?
There was a problem hiding this comment.
Looks good, except that it should fit in a single line. Maybe:
class GenericSelect(unary_iteration.UnaryIterationGate):
r"""Generic SELECT gate for selecting and applying operators from an array of `PauliString`s.
"""| return GenericSelect(selection_bitsize, target_bitsize, select_unitaries=dense_ham) | ||
| target_bitsize = 4 | ||
| us = ['XIXI', 'YIYI', 'ZZZZ', 'ZXYZ'] | ||
| us = [cirq.DensePauliString(u) for u in us] |
There was a problem hiding this comment.
Reusing same variable would lead to a mypy error?
| us = [cirq.DensePauliString(u) for u in us] | |
| us_dps = [cirq.DensePauliString(u) for u in us] |
There was a problem hiding this comment.
xref: quantumlib/Cirq#3832
- we can configure mypy to be fine with this
- it will probably be the default anyways
- I think it's way better to re-use existing name than having hungarian variable names and the previous version still hanging around that you could accidentally use instead.
There was a problem hiding this comment.
Sounds good, maybe open an issue to make sure we update the mypy config to add the flag so it doesn't complain. Right now, the config is basically copied from Cirq.
like #103