Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Cirq-FT registers multi-dimensional #6200

Merged

Conversation

tanujkhattar
Copy link
Collaborator

@tanujkhattar tanujkhattar commented Jul 16, 2023

This brings the Cirq-FT registers one step closer to Qualtran registers.

Some notable changes in this PR:

  1. Register.bitsize is now Register.shape and a tuple instead of integer
  2. Added a method total_bits to Register class.
  3. Registers.bitsize is now Registers.total_bits()
  4. SelectionRegisters.build interface is changed to match Registers.build and all call sites are updated to use explicit constructor if iteration_length is non trivial (i.e. not 2 ** self.shape[0])
  5. Demonstrated the use of multi-dimensional registers in SwapWithZero class where the target register is now 2D.

cc @mpharrigan

Fixes #6053

@tanujkhattar tanujkhattar requested review from a team, vtomole and cduck as code owners July 16, 2023 23:57
@CirqBot CirqBot added the size: M 50< lines changed <250 label Jul 16, 2023
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@tanujkhattar tanujkhattar added the area/cirq-ft Issues related to the Cirq-FT sub-package label Jul 17, 2023
@mpharrigan
Copy link
Collaborator

Registers.total_bits() sounds better to me

) -> cirq.OP_TREE:
control, ancilla, target = quregs['control'], quregs['ancilla'], quregs['target']
if len(self.cv) == 2:
yield self._decompose_single_and(
self.cv[0], self.cv[1], control[0], control[1], *target
)
else:
yield self._decompose_via_tree(control, self.cv, ancilla, *target)
yield self._decompose_via_tree(control.tolist(), self.cv, ancilla.tolist(), *target)
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need tolist()? Do we need to update the type signatures for _decompose_via_tree if we're using list-specific things

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We need this mainly because Sequence[cirq.Qid] is not compatible with NDArray[cirq.Qid].

I'll update the signature of _decompose_via_tree to accept NDArray[cirq.Qid] instead.

Copy link
Collaborator

Choose a reason for hiding this comment

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

that's kindof annoying

cirq-ft/cirq_ft/infra/gate_with_registers.py Outdated Show resolved Hide resolved
cirq-ft/cirq_ft/infra/gate_with_registers.py Outdated Show resolved Hide resolved
cirq-ft/cirq_ft/infra/gate_with_registers.py Outdated Show resolved Hide resolved

@classmethod
def build(cls, **registers: int) -> 'Registers':
return cls(Register(name=k, bitsize=v) for k, v in registers.items())
def build(cls, **registers: Union[int, Tuple[int, ...]]) -> 'Registers':
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is going to be really screwwy when combined with SelectionRegisters.build

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hmm, I'll change SelectionRegisters.build to follow the Register.build interface and use 2 ** self.shape[0] as the default iteration length. All call sites using non-default iteration length will be updated to use the full constructor

@tanujkhattar
Copy link
Collaborator Author

@mpharrigan I've addressed all your comments and updated the PR.

@tanujkhattar tanujkhattar enabled auto-merge (squash) July 17, 2023 19:56
@tanujkhattar tanujkhattar merged commit 83ede36 into quantumlib:master Jul 17, 2023
33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/cirq-ft Issues related to the Cirq-FT sub-package size: M 50< lines changed <250
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for Qubit registers to Cirq
3 participants