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.

Sign up for GitHub

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 ancilla for BiQubitsMixer sided too. #6313

Merged
merged 2 commits into from
Oct 10, 2023
Merged

Make ancilla for BiQubitsMixer sided too. #6313

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b98b1d4
Make ancilla for BiQubitsMixer sided too.
fdmalone Oct 9, 2023
b222feb
Add note regarding need for adjoint for ancilla qubits in BiQubitsMixer.
fdmalone Oct 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 12 additions & 4 deletions cirq-ft/cirq_ft/algos/arithmetic_gates.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from typing import Iterable, Optional, Sequence, Tuple, Union, List, Iterator
from numpy.typing import NDArray
from typing import Iterable, Iterator, List, Optional, Sequence, Tuple, Union

from cirq._compat import cached_property
import attr
import cirq
from cirq._compat import cached_property
from numpy.typing import NDArray

from cirq_ft import infra
from cirq_ft.algos import and_gate

Expand Down Expand Up @@ -148,7 +149,14 @@ class BiQubitsMixer(infra.GateWithRegisters):

@cached_property
def signature(self) -> infra.Signature:
return infra.Signature.build(x=2, y=2, ancilla=3)
one_side = infra.Side.RIGHT if not self.adjoint else infra.Side.LEFT
return infra.Signature(
[
infra.Register('x', 2),
infra.Register('y', 2),
infra.Register('ancilla', 3, side=one_side),
Copy link
Collaborator

Choose a reason for hiding this comment

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

lets rename it target. this will result in some changes to the decomposition and docstring

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's also improve the docstring to more clearly specify what each of the 3 target bits contain. It's not clear to me right now by just reading the docstring.

Copy link
Collaborator

Choose a reason for hiding this comment

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

the short answer is .. enough information to do the adjoint. coming to think of it, this is why I kept the register name from the paper as ancilla. because they have no use otherthan as information holders for that will be used by the adjoint.

the actually useful information is stored inplace.

so maybe skip renaming it for now and I will come back for it later.

Copy link
Collaborator

Choose a reason for hiding this comment

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

hmm, we've renamed such registers to junk in Qualtran.

@fdmalone As part of this PR, I think we should at least capture this discussion in the docstring and let the reader know that the ancilla is used to reduce the T-counts and assumes that it should be cleaned up later by the adjoint.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. I'll try distill something later today. We should open an issue to track either here or in qualtran.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

]
)

def __repr__(self) -> str:
return f'cirq_ft.algos.BiQubitsMixer({self.adjoint})'
Expand Down