-
Notifications
You must be signed in to change notification settings - Fork 615
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
DeprecateQNode.gradient_fn
#6244
Conversation
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.
Just a couple of questions :)
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.
Cool, just one comment from me (except for the tests)
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #6244 +/- ##
=======================================
Coverage 99.59% 99.59%
=======================================
Files 444 444
Lines 42307 42307
=======================================
Hits 42137 42137
Misses 170 170 ☔ View full report in Codecov by Sentry. |
Co-authored-by: Pietropaolo Frisoni <pietropaolo.frisoni@xanadu.ai>
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 :)
**Context:** The existence of `QNode.gradient_fn` ties us in to a bit more of an object oriented framework with a lot of in-place mutation of the qnode. By freeing ourselves from this property, we can have a bit more of a functional structure with less coupling and side effects. It will also free us up to start making other logical changes and improvements. `QNode.gradient_fn` is also not really defined, so it's hard to tell what it should actually be and reflect. Things have changed enough recently with more dynamic gradient validation, that it no longer really carries the same information it did when it was added. There isn't really a good analog yet of `QNode.gradient_fn`, since it's kinda a "processed diff method". We do have stories for next quarter to start adding helper transforms for things like this, but it won't be immediate. **Description of the Change:** **Benefits:** **Possible Drawbacks:** **Related GitHub Issues:** [sc-71844] --------- Co-authored-by: Pietropaolo Frisoni <pietropaolo.frisoni@xanadu.ai>
**Context:** The existence of `QNode.gradient_fn` ties us in to a bit more of an object oriented framework with a lot of in-place mutation of the qnode. By freeing ourselves from this property, we can have a bit more of a functional structure with less coupling and side effects. It will also free us up to start making other logical changes and improvements. `QNode.gradient_fn` is also not really defined, so it's hard to tell what it should actually be and reflect. Things have changed enough recently with more dynamic gradient validation, that it no longer really carries the same information it did when it was added. There isn't really a good analog yet of `QNode.gradient_fn`, since it's kinda a "processed diff method". We do have stories for next quarter to start adding helper transforms for things like this, but it won't be immediate. **Description of the Change:** **Benefits:** **Possible Drawbacks:** **Related GitHub Issues:** [sc-71844] --------- Co-authored-by: Pietropaolo Frisoni <pietropaolo.frisoni@xanadu.ai>
Context:
The existence of
QNode.gradient_fn
ties us in to a bit more of an object oriented framework with a lot of in-place mutation of the qnode. By freeing ourselves from this property, we can have a bit more of a functional structure with less coupling and side effects. It will also free us up to start making other logical changes and improvements.QNode.gradient_fn
is also not really defined, so it's hard to tell what it should actually be and reflect. Things have changed enough recently with more dynamic gradient validation, that it no longer really carries the same information it did when it was added.There isn't really a good analog yet of
QNode.gradient_fn
, since it's kinda a "processed diff method". We do have stories for next quarter to start adding helper transforms for things like this, but it won't be immediate.Description of the Change:
Benefits:
Possible Drawbacks:
Related GitHub Issues:
[sc-71844]