Delete SelectionRegisters and replace uses of Registers with Tuple[Register, ...]#6278
Conversation
|
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
tanujkhattar
added
BREAKING CHANGE
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #6278 +/- ##
=======================================
Coverage 97.88% 97.88%
=======================================
Files 1106 1106
Lines 95710 95699 -11
=======================================
- Hits 93683 93674 -9
+ Misses 2027 2025 -2
☔ View full report in Codecov by Sentry. |
mpharrigan
left a comment
mpharrigan
left a comment
There was a problem hiding this comment.
Looks like mostly rote changes, which lgtm
What do you think about documenting how to use ravel and unravel for multiple selection registers? Presumably if you didn't know about it originally others may have the same problem! Where would be the most discoverable place to put this? Maybe a paragraph in the SelectionRegister docstring
| return f'cirq_ft.Register(name="{self.name}", shape={self.shape})' | ||
|
|
||
|
|
||
| def total_bits(registers: Iterable[Register]) -> int: |
There was a problem hiding this comment.
I know none of these had docstrings, but it might be a good time to add a quick one-line docstring to each
There was a problem hiding this comment.
added one line docstrings
| regs = [cirq_ft.SelectionRegister('x', n, N), cirq_ft.SelectionRegister('y', m, M)] | ||
| for x in range(regs[0].iteration_length): | ||
| for y in range(regs[1].iteration_length): | ||
| assert np.ravel_multi_index((x, y), (N, M)) == x * M + y |
| target_regs = {reg.name: quregs[reg.name] for reg in self.target_registers} | ||
| extra_regs = {reg.name: quregs[reg.name] for reg in self.extra_registers} |
|
@mpharrigan Addressed your comments. Added a detailed docstring to |
3 participants
This PR brings the
cirq_ft.Registerscloser toqualtran.Signature. It's part of a larger effort to consolidatecirq-ftandqualtrandata types.