CliffordBackend with packed bits
#1239
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1239 +/- ##
=======================================
Coverage 99.81% 99.81%
=======================================
Files 72 72
Lines 10490 10517 +27
=======================================
+ Hits 10471 10498 +27
Misses 19 19
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
renatomello
added
the
quantum_info module
Co-authored-by: Andrea Pasquale <andreapasquale97@gmail.com>
|
@alecandido I was following your proposed order in #1231 (comment) for this PR. However, now that in main the qibojit test dependency points to the branch |
|
In this case, the solution would be to rebase your PR (qiboteam/qibojit#173) on top of Qibojit's And yes, it's not great, but now PRs are rather coupled. So, to avoid broken tests, we should first merge qiboteam/qibojit#170 |
| r, x, z = _get_rxz(state, nqubits) | ||
| x = _packbits(x, axis=1) | ||
| z = _packbits(z, axis=1) | ||
| return np.hstack((x, z, r[:, None])) |
There was a problem hiding this comment.
Why are you packing x and z separately?
There was a problem hiding this comment.
I guess the motivation is the _rowsum function above, in which you need to treat separately x, z, and r, but just to get a confirmation.
There was a problem hiding this comment.
Yes precisely, because a single row of the matrix contains the encoding for both x, z and r, thus you cannot pack the complete row in one go but pack the different sections of the row separately.
| return state | ||
|
|
||
|
|
||
| def _clifford_post_execution_reshape(state, nqubits): | ||
| def _clifford_post_execution_reshape(state, nqubits: int): |
There was a problem hiding this comment.
This is asymmetric with respect to the one above, i.e. there is no pack flag (and it is always unpacking).
What's the reason?
There was a problem hiding this comment.
I remember that the pre- function was called both before execution, but also in correspondence of collapsing measurements, where packing is not needed. But I will doublecheck because after I updated the measurement I might have changed this mechanism.
There was a problem hiding this comment.
I double checked and indeed the pack arg can be dropped, thanks for spotting this
|
Of course, whenever you can, merge/rebase on |
|
I believe there is a problem with the |
I saw you updated it, thanks. Simply, no one touched it after the merge. |
|
There is a test, I fixed it by setting a test target that depends on the |
Is the seed not fixed explicitly anywhere? Are we relying on some default? (it could be much more complicate than that, but this is the starting point to investigate) |
|
Yes the seed is set qibo/tests/test_models_circuit_features.py Line 321 in 036e882 but for some reason pytorch yields different results with macos using the same seed. I believe the problem exists in main as well, as I see the test failing in #1310 as well. Weirdly, this came out all of a sudden... (EDIT): indeed I just saw the issue #1313 |
|
@scarrazza just provided #1317 as a (temporary) solution for the test problem. I'd suggest merging/rebasing on it (or just wait that it will be merged in |
This PR introduces a further optimization for the
CliffordBackend. Namely the bits of the symplectic matrix are now packed asuint8arrays by thenp.packbitsfunction.Checklist: