Add grouping support for arithmetic ops #5070
Conversation
|
[sc-53504] |
There was a problem hiding this comment.
Thanks @mudit2812
Great to see progress on grouping with arithmetic ops!
I gave it a first pass and there are honestly a lot of things that I dont quite understand yet. Could you help me by providing some context to the changes where I ask questions and generally add a description to the PR what the goal is as well as developer comments. Since there are no added tests it is hard to understand the context of the PR.
Re caching and Sum not having a coeffs attribute, I think this is something we wanted to add for feature parity anyway. If this attribute existed, would this make this PR any easier?
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #5070 +/- ##
==========================================
- Coverage 99.68% 99.67% -0.02%
==========================================
Files 392 392
Lines 35844 35565 -279
==========================================
- Hits 35732 35448 -284
- Misses 112 117 +5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Great job @mudit2812 🥳
Left some comments on the pauli checks. Looks overall good though 👍
Co-authored-by: Korbinian Kottmann <43949391+Qottmann@users.noreply.github.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5070 +/- ##
==========================================
- Coverage 99.68% 99.67% -0.02%
==========================================
Files 394 394
Lines 36160 35875 -285
==========================================
- Hits 36047 35759 -288
- Misses 113 116 +3 ☔ View full report in Codecov by Sentry. |
Co-authored-by: Matthew Silverman <matthews@xanadu.ai>
Context:
Adding support for
ProdandSProdto be used with grouping.Description of the Change:
Tensor.__matmul__to stop mutating in-place and return a new op instead.Tensor. Consequently,Tensor.copy()andTensor.map_wires()also had to be updated.Tensor__init__to fix incorrect validation after the previous changes. The warning about overlapping wires is now raised inTensor.queue()instead of at the beginning of construction, so that raising the warning pre-maturely can be prevented.qml.pauli.group_observablesnow returnsProds if the observable list contains new arithmetic ops.pauli_to_binaryto rely on the Pauli rep. All operations that can be grouped should now always have a valid Pauli rep, so we don't need to worry about other cases, aspauli_to_binaryis only a utility function for grouping.are_pauli_words_qwcto correctly handle ops with Pauli reps. This change however means that any new arithmetic ops that havepauli_rep=Nonewill still not work with this function. If we eventually start doing more simplification eagerly, this can be addressed.split_non_commutingnow createsProds instead ofTensors for creating observables for measurements that use wires. Also made a change so that if a measurement without any wires is made, the observable that is created usestape.wires.Benefits:
Possible Drawbacks:
Related GitHub Issues: