Add CX cancellation pass

[Misty-W]

Fixes #17.

Adds a custom Qiskit transpiler pass to cancel redundant CX gates, according to commutation rules summarized here: https://www.aspdac.com/aspdac2019/archive/pdf/2D-2.pdf, swapping commuting gates, and using Qiskit functions to check whether the gates are inverses of each other.

As noted in the file itself, the added file is a derivative work from related transpiler passes in Qiskit. The improvement is in using the commutation information obtained to switch the gates, check inverses, and perform cancellations where applicable.

[natestemen]

Cool to see what a transformation pass looks like on Qiskit's DAG! Thanks for letting me take a look. Some questions mostly :)

[natestemen]

Just checking my understanding. You want to loop over every node, and compare it with every node that has come prior, right? If it doesn't matter if you're looking forward or backward then this could also be simplified to loop over the nodes directly, instead of indices.

for i, node1 in enumerate(topo_sorted_nodes):
    for node2 in topo_sorted_nodes[i+1:]:

Question: Suppose we have a circuit, which at some point has gates $\cdots ABC\cdots$ where $A$ does not commute with $B$ and $B$ does not commute with $C$. Is $B$ checked for inverse/commutation against every other gate in the circuit? If so this is likely very expensive!

---

Another small thing: idx1 == 0 is not used since the range computed on the line below doesn't yield anything (i.e. list(range(-1, -1, -1)) == []).

[Misty-W]

Yes, you have the right idea, and I agree looping over the nodes directly is better. I also agree that this is not the most efficient approach in the long term, but it is the same approach as our baseline, i.e. what the Qiskit commutation checker uses. The improvement is that we use the commutation information obtained to switch the gates, check inverses, and perform cancellations where applicable.

[jordandsullivan]

Cool, let's create a new issue for optimizing this and we can tackle it in the future.

[natestemen]

Apologies for the "outdated" comments. Looks like I started reviewing and a file got moved mid-way through.

[Misty-W]

thanks for the review @natestemen! no worries about the outdate comments, that was on me for moving things around.

[natestemen]

It's not clear to me that the commutation rules are being applied in a way that's useful here. E.g. I don't think the following situation (which is the simplest I can imagine) is being found/optimized.

Suppose you have a circuit defined as $C = \mathtt{CNOT}_{1,2} \cdot U \cdot \mathtt{CNOT}_{1,2}$ where $U\in\mathsf{U}(4)$ commutes with $\mathtt{CNOT}$. In order to compile this circuit to $C = U$, you need to

1. check if $U$ commutes with one of the $\mathtt{CNOT}$ gates
2. commute the gates (i.e. $C = U \cdot \mathtt{CNOT}_{1,2} \cdot \mathtt{CNOT}_{1,2}$)
3. perform a round of inverse cancellations ($C = U$)

As my understanding goes (and I might be missing some magic that is performed on behalf of the qiskit transpiler pipeline), we are only checking if gates next to each other are inverses. There is a commutation check, but gates are never actually swapped to be checked against their new neighbors.

[natestemen]

I'm confused why this block is needed. I would expect _check_inverse(node1, node2) == _check_inverse(node2, node1) which would make this second block redundant. What am I missing here?

[Misty-W]

I wasn't sure at first, but the first inverse check is redundant- another holdover from the Qiskit commutation checker. I removed it and tested again, and it still works.

[natestemen]

I know this has been merged now, but my comment regarding commutation not being used is still open (at least to me). As far as I understand, the code only implements nearest neighbor inverse checks, which is different from the issue and PR description goals.

[jordandsullivan]

I know this has been merged now, but my comment regarding commutation not being used is still open (at least to me). As far as I understand, the code only implements nearest neighbor inverse checks, which is different from the issue and PR description goals.

Are you able to produce a code snippet that demonstrates what the code is failing to do @nathanshammah? Do you have something different in mind than what's in this test: https://github.com/unitaryfund/ucc/blob/main/ucc/transpiler_passes/tests/test_cx_cancellation.py

[natestemen]

Yup! Here's a commit b7ef8e9 with a new test that shows that the $R_z$-control rule¹ is not being applied.

BTW, reminder that I'm nate, not Nathan 😛.

Footnotes

1. Defined as the first example on page 10 of https://www.aspdac.com/aspdac2019/archive/pdf/2D-2.pdf. ↩

[jordandsullivan]

Agh my brain is broken this week @natestemen I am so sorry XD
I owe you a birthday card with your name written on it correctly! Thank you very much for creating that example. :)

@Misty-W does this help clear up what may be happening in your open PR #36 ?

[Misty-W]

@jordandsullivan and @natestemen, I believe I understand the issue and will try to fix it in PR #36. I need to be careful with the indexing, but if I get that right, it should work.