🐛 Fix ancillary handling in ZX checker #512
Conversation
Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
burgholzer
added
c++
Disables ZX Checker when non-garbage ancillae are defined.
Signed-off-by: Tom Peham <[email protected]>
Welcome to Codecov 🎉Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests. ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment Thanks for integrating Codecov - We've got you covered ☂️ |
…ebugging-ancilla-qubits
Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
Signed-off-by: burgholzer <[email protected]>
There was a problem hiding this comment.
@pehamTom many thanks for the changes here. I pushed a couple of commits with cleanups of unrelated changes and a small fix for an oversight in the new shortcut of the ZX Checker run method.
I only have two real questions left, which you will find inline.
Other than that, it would be nice if you could add a changelog entry for this fix.
| if (qc1->getNqubitsWithoutAncillae() == 0) { | ||
| miter = zx::ZXDiagram(); | ||
| } else { | ||
| const auto numQubits1 = static_cast<zx::Qubit>(qc1->getNqubits()); | ||
| for (zx::Qubit i = 0; i < static_cast<zx::Qubit>(qc1->getNancillae()); | ||
| ++i) { | ||
| const auto anc = numQubits1 - i - 1; | ||
| miter.makeAncilla( | ||
| anc, static_cast<zx::Qubit>(p1.at(static_cast<qc::Qubit>(anc)))); | ||
| } | ||
| miter.invert(); | ||
| } | ||
|
|
||
| if (qc2->getNqubitsWithoutAncillae() == 0) { | ||
| return; | ||
| } | ||
| const auto numQubits2 = static_cast<zx::Qubit>(qc2->getNqubits()); | ||
| for (zx::Qubit i = 0; i < static_cast<zx::Qubit>(qc2->getNancillae()); ++i) { | ||
| const auto anc = numQubits2 - i - 1; | ||
| dPrime.makeAncilla( | ||
| anc, static_cast<zx::Qubit>(p2.at(static_cast<qc::Qubit>(anc)))); | ||
| } | ||
| miter.invert(); | ||
| miter.concat(dPrime); | ||
| } |
There was a problem hiding this comment.
I don't fully understand the reasoning behind this change.
Could you please clarify why adding ancillary qubits to the circuits isn't an option when they lack data qubits?
There was a problem hiding this comment.
It became clear in our discussion, non-garbage ancillaries cannot be supported in the current version of the ZX checker. Since all garbage ancillaries are traced out, a lack of data qubits is equivalent to no operation at all.
There was a problem hiding this comment.
A, fair enough. That make sense.
Would you mind adding some of that as an in-code comment? Otherwise, the next person looking at the code might wonder the same thing as I just did.
| @@ -191,6 +191,7 @@ TEST_F(ZXTest, Ancilla) { | |||
| qc1.i(0); | |||
| qc2.cx(1, 0); | |||
| qc2.setLogicalQubitAncillary(1); | |||
| qc2.setLogicalQubitGarbage(1); | |||
There was a problem hiding this comment.
Could you clarify, why this is needed now, but was not needed previously?
Before this PR, the test worked without this change.
Would it be too hard to continue supporting that?
There was a problem hiding this comment.
There was a misunderstanding on my part on the role of ancillaries in mqt core. As discussed, the current state of the ZX checker cannot support non-garbage ancillaries. That this worked before was simply because every ancillary was treated as if it were garbage.
There was a problem hiding this comment.
Yeah. Makes sense...
Do you think it would be worth it to add a dedicated option for the ZX Checker to simply treat all ancillaries as garbage?
I could imagine that many people would probably be using that option.
Description
Fixes #508
The regression tests and the corresponding CI failures demonstrate the underlying issue in the ZX checkers:
equivalent, while all other checkers agree that the two circuits are not equivalent.Note the first two tests fail for separate reasons, which means there are multiple things going wrong here.
This PR adjusts the conditions under which the ZX checker declares it can handle the underlying circuits, making it more conservative than before.
Additionally, the out of range error is fixed.
Checklist: