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

Incorrect (or at least inconsistent) T cost for a TwoBitCSwap gate. #1463

Open
wjhuggins opened this issue Oct 10, 2024 · 2 comments
Open

Incorrect (or at least inconsistent) T cost for a TwoBitCSwap gate. #1463

wjhuggins opened this issue Oct 10, 2024 · 2 comments

Comments

@wjhuggins
Copy link
Collaborator

To summarize a discussion I had with @tanujkhattar, @mpharrigan, and @NoureldinYosri:

We currently count the T and Clifford costs for a TwoBitCSwap (Fredkin) gate using the construction from https://arxiv.org/pdf/1308.4134. However, we can implement this gate using two CNOT gates and a Toffoli gate, which has a four T-gate construction using ancilla qubits (https://arxiv.org/pdf/1212.5069).

I think we should default to the ancilla-assisted construction for the Fredkin gate.

The simplest solution might be to no longer treat a Fredkin gate as a leaf node at all and instead decompose it into a Toffoli gate and two CNOT gates. If not, then we should at least be consistent in how we treat the resource counts for Fredkin and Toffoli gates.

@wjhuggins
Copy link
Collaborator Author

This is connected to #1121, which is related to the observation that we can do compute / uncompute pairs for Toffoli and Fredkin gates more cheaply than a naive implementation.

@mpharrigan
Copy link
Collaborator

As a first step, I'd recommend writing a simple circuit as a bespoke method like

def to_clifford_t_circuit(self) -> 'cirq.FrozenCircuit':

and updating the references section to highlight the disparity.

As a leaf bloq, we can change the T cost of a cswap on the fly. See the ts_per_cswap argument to GateCounts.total_t_count and GateCounts.to_legacy_t_complexity. Those default to 7. It can be changed to 4. Although maybe the "legacy" method should keep using 7 since it's been that way for a while

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants