-
Notifications
You must be signed in to change notification settings - Fork 616
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
Upgrade qml.lie_closure
to handle dense matrices
#6811
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6811 +/- ##
=======================================
Coverage 99.59% 99.60%
=======================================
Files 476 477 +1
Lines 45193 45262 +69
=======================================
+ Hits 45012 45081 +69
Misses 181 181 ☔ View full report in Codecov by Sentry. |
…o dla_lie_closure_dense
…o dla_lie_closure_dense
…o dla_lie_closure_dense
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! Left a few comments about doc readability and arguments/UI
pennylane/pauli/dla/util.py
Outdated
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.
I think we usually avoid having a "utils" submodule if we can name it something more specific or group functions by their usage.
Since we only have one function in here for now (trace_inner_product
), does it still make sense to add the util.py
file?
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.
I'm expecting there to be more "utility functions", see e.g. here
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.
Surely not all of them, but some might make sense to keep public because they are used in different places and might be handy for users
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 and thorough job, @Qottmann 🎉
This basically looks good to go for me, just with the change dense -> matrix
pending on the earlier files (by order of git diff :D)
I am wondering whether the trace inner product should be a qml.math
feature?
Co-authored-by: David Wierichs <[email protected]>
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.
Right now the expected fails caused by the removal of the control_wires
arg of MultiControlledX
have been cleaned. But there are still multiple fails which require the following changes according to the context here.
Co-authored-by: Yushao Chen (Jerry) <[email protected]>
…o dla_lie_closure_dense
Upgrading from labs functionality
labs.dla.lie_closure_dense
is integrated into the logic ofqml.lie_closure
Also adding
qml.pauli.trace_inner_product
as a utility function for Pauli matrices and operators.Integrate lie closure dense to qml lie closure [sc-81965]