Closed appleby closed 5 years ago
I will look into this later tonight, but being new to quilc it might take me a bit. If someone wants to unbreak the build on my behalf before then, please be my guest :).
I’ll look at this tomorrow morning. It’s very likely a problem with the new comments not being read by whatever the quilc test is doing, and so the two matrices it’s trying to compare are only equivalent up to a qubit permutation it doesn’t know about. Similarly intended tests in the CL-QUIL test suite work OK, so I don’t expect something is fundamentally broken, and users can probably rest easy.
We’ll see, though!
I don't know a qubit from a cucumber so take this with a grain of salt, but I noticed that prior to this change, a CURRENT_REWIRING pragma appears at the end of code, just before the HALT:
$ ./quilc.before < app/tests/faithfulness-test-files/CCNOTs.quil | tail -3
RZ(-2.5480657262161737) 4
PRAGMA CURRENT_REWIRING "#(2 4 0 3 5 1 6 7)"
HALT
Whereas after, the rewiring is attached to the HALT as comment.
$ ./quilc < app/tests/faithfulness-test-files/CCNOTs.quil | tail -3
RX(-pi/2) 4
RZ(-2.5480657262161737) 4
HALT # Exiting rewiring: #(2 4 0 3 5 1 6 7)
When quilc is invoked with the "--protoquil" flag, then the HALT instruction is stripped. In the former case the rewiring pragma remains and the single CURRENT_REWIRING turns into a sequence of 3 pragmas with an EXPECTED_REWIRING in the middle:
$ ./quilc.before -P < app/tests/faithfulness-test-files/CCNOTs.quil | tail -4
RZ(-2.5480657262161737) 4
PRAGMA CURRENT_REWIRING "#(2 4 0 3 5 1 6 7)"
PRAGMA EXPECTED_REWIRING "#(2 4 0 3 5 1 6 7)"
PRAGMA CURRENT_REWIRING "#(2 4 0 3 5 1 6 7)"
But since the rewiring is now "attached" to HALT, it gets dropped.
$ ./quilc -P < app/tests/faithfulness-test-files/CCNOTs.quil | tail -4
RX(pi/2) 4
RZ(1.760797612472632) 4
RX(-pi/2) 4
RZ(-2.5480657262161737) 4
I noticed that in quil::parsed-program-to-logical-matrix
it does some twiddling of resultant matrix based on the rewiring pragmas/comments, but I have no idea what any of this means, so apologies if this is expected / irrelevant.
My next idea was to try replacing the HALT with a no-op stealing the rewiring comment and see if that fixes it, but I won't have time to try before @ecpeterson looks into it this morning, so posting this speculation now in case it's relevant.
The faithfulness tests that were recently enabled in #189 are failing an assertion in
cl-quil::scale-out-matrix-phases
. The test failure appears to be a result of changes made in #159. At least, if I rebase and drop that commit, the test passes again. Note that #159 was merged just before #189, so @ecpeterson wouldn't have known.Probably related to @karalekas comment here: https://github.com/rigetti/quilc/pull/197#issuecomment-483434459
Here is the backtrace: