-
Notifications
You must be signed in to change notification settings - Fork 604
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
Evolution operator #3375
Evolution operator #3375
Conversation
Codecov Report
@@ Coverage Diff @@
## master #3375 +/- ##
=======================================
Coverage 99.86% 99.86%
=======================================
Files 309 309
Lines 27542 27597 +55
=======================================
+ Hits 27506 27561 +55
Misses 36 36
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it 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.
Nice work so far! Made a few comments, let me know once they are addressed and I can give it another look 👍🏼
Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca>
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.
Great work @lillian542, just left some suggestions and comments.
Co-authored-by: Utkarsh <utkarshazad98@gmail.com>
* get parameter shift working with evolution operator * Fix bug in Exp.hash * Update tests * Coeff to string in hash * Update test Co-authored-by: Lillian Frederiksen <lillian542@gmail.com>
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.
Great work! Left some comments for you to address.
Just one suggestion, since this operation doesn't have a top level constructor (eg. for Sum
class we have a top level qml.op_sum
), I think it might be worth while to just make it available top level. This would require adding an import in the PennyLane init file with from ops import Evolution
and would unlock the qml.Evolution()
UI
You should confirm with Tom or Josh to make sure this is actually what we want. Otherwise I think it looks great!
Co-authored-by: Jay Soni <jbsoni@uwaterloo.ca>
9b96a7e
to
ce1ed34
Compare
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.
Leaving some suggestions. I might not be able to take a look again due to limited internet availability but please feel free to get this merged once you have gone through them and carefully checked the rendering for the docs!
Co-authored-by: Utkarsh <utkarshazad98@gmail.com>
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.
Awesome work!
Context:
Exp
class does not havegenerator
method orparameter_frequency
property defined, even though these could be defined for some instances ofExp
.Exp
where the base operator includes parameters (e.g.0.5*qml.PauliX(0)
, or any instance ofqml.Hamiltonian
), the operator hasnum_params > 1
, and standard functions for differentiation via parameter shift, such asqml.generator(op)
, don't work.Description of the Change:
generator
andparameter_frequency
toExp
class in cases where it is defined (when the coefficient has no real component and the base operator is Hermitian)Evolution
operator defined to be of the form exp(ixG) with a single, trainable parameter x. This allows differentiation with regards to the parameter x, even in instances where the base operator G contains additional parameters.