-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update QuantumScriptSerializer.serialize_observables
to better handle new operator arithmetic
#670
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #670 +/- ##
==========================================
+ Coverage 97.09% 98.70% +1.60%
==========================================
Files 149 173 +24
Lines 19387 24399 +5012
==========================================
+ Hits 18824 24082 +5258
+ Misses 563 317 -246 ☔ View full report in Codecov by Sentry. |
[sc-59698] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work @mudit2812!
I'm not sure what's going on with the codecov warnings. I added tests specifically targeting the "uncovered" lines and I know that those lines are being covered locally. My guess is that the warnings are artifacts of the continuously updated coverage that happens due to the way that coverage data is uploaded to codecov. The checks window also says that 100% patch coverage has been achieved. |
I have encountered that in the past. I would say feel free to ignore it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, thanks @mudit2812 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, @mudit2812 !
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🪂
Context:
This PR is implementing some technical debt that was left behind after #649 was merged.
Description of the Change:
Hamiltonian
. WhileHamiltonian
now does have a pauli rep, it behaves lazily. I also assumed that there might be pipelines in lightning that take advantage ofHamiltonian.grouping_indices
. If this is not a major concern, I can update to also use the pauli rep withHamiltonian
.Prod
when the operands might have overlapping wires. In the case of Paulis, this is handled automatically by the pauli rep. However, in other cases, we end up in an infinite recursion as simplifying does not lead to any changes to the operands. If anyone has an idea about this, I would love to hear it. The current solutions I have in mind are to either raise an error, or fallback to treating the observable as aHermitian
(although the matrix might not actually be hermitian).cmake
since there is now a release with the bug fix that was causing trouble (See here for context).Benefits:
More optimized pipeline for serialization with new operator arithmetic.
Possible Drawbacks:
Related GitHub Issues: