Reimplement qubit unitary #6986
Conversation
Co-authored-by: Christina Lee <christina@xanadu.ai>
|
@JerryChen97, are these CI tests supposed to fail? |
Sorry, forgot to fix them on e2e branch. I'll fix soon |
| if sp.sparse.issparse(state): | ||
| raise TypeError("State should not be sparse in default qubit pipeline") |
There was a problem hiding this comment.
How would we get a sparse state in the first place?
There was a problem hiding this comment.
It won't happen for the whole pipeline and any common users.
This is to block such misuse in our own dev. One previous test fail was caused by such reason and the error was not clear because it only emerges in deeper callstack. Having such blocker would help prevent such scenario.
pennylane/ops/qubit/matrix_ops.py
Outdated
| if sp.sparse.issparse(U): | ||
| return U.toarray() |
There was a problem hiding this comment.
I'm kinda going back and forth about whether or not we should raise MatrixUndefinedError here if U is sparse. I am leaning toward only providing a dense matrix U is dense and only provide a sparse matrix if U is sparse.
Then it's up to whatever consumes the operator to doing any dense-sparse or sparse to dense conversions.
Mind giving that a try and seeing what happens?
There was a problem hiding this comment.
Trying here: 7030e31
There was a problem hiding this comment.
Where the failures just unit tests?
There was a problem hiding this comment.
@albi3ro vastly most are Controlled.sparse_matrix(wire_order=..) not implemented.
There was a problem hiding this comment.
which is still interesting though. Why before I've never seen them triggered?
There was a problem hiding this comment.
Why would changing QubitUnitary have changed Controlled tests?
There was a problem hiding this comment.
@albi3ro it seems that the deferred measurement converts some Conditionals into Controlled, and then the Controlled used Controlled(QubitUnitary(...))
There was a problem hiding this comment.
@albi3ro it seems that the deferred measurement converts some Conditionals into Controlled, and then the Controlled used Controlled(QubitUnitary(...))
This can be observed by putting break point between results1 and results2 inside tests/devices/qubit/test_simulate.py::TestMidMeasurements::test_simple_dynamic_circuit, at commit 24a8f1b
|
External fail is exactly the same as this morning. Just ignore it until we merge this PR to e2e branch and update there with master |
**Context:** In our on-going sparse matrix epic, we used to postponed the full implementation of controlled ops family due to others of higher priorities. However, in a related PR https://github.com/PennyLaneAI/pennylane/actions/runs/13464002433/job/37625640042?pr=6986 it seems inevitable to have the `wire_order` branch of `Controlled.sparse_matrix` method implemented. For the sake of traceability, and to prevent some the mentioned PR from being furthermore spaghettized with high entanglement, we implement the `wire_order` branch here with unit tests, and watch and fix whatever new impact introduced as an independent PR instead of hotfix overthere. **Description of the Change:** Direct call `expand_matrix` just as what `matrix()` did above `sparse_matrix`, right before format-convert. Convert the error-raise test into a consistency-check with `matrix(wire_order=...)` **Benefits:** Fully fledged sparse `Controlled` Unblock #6986 **Possible Drawbacks:** Small chance to still have incompatibility in the future, which is also why we would like to have this separated such that easy to revert. **Related GitHub Issues:** [sc-84949] --------- Co-authored-by: Andrija Paurevic <46359773+andrijapau@users.noreply.github.com>
Context:
Our current sparse repr for
QubitUntiaryis quite confusing: itsmatrix()return a sparse matrix by default, if this obj was instantiated by a CSR, which is inconsistent from what we expected for other operators, thatmatrix()should return a dense andsparse_matrix()gives out the sparse, well separated from each other.Therefore, a slight re-work is needed to make things clearer. Basically, we will overload the
has_matrixandhas_sparsefrom parents and have a more friendly and straightforward usage.Description of the Change:
Benefits:
Possible Drawbacks:
Related GitHub Issues: