-
Notifications
You must be signed in to change notification settings - Fork 21
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
[Feature] Pyqtorch - First Order Adjoint Differentiation #155
Conversation
Are we moving adjoint diff from pyqtorch to qadence? |
@nmheim, cant reply to your comment directly. we need our own qadence adjoint because we need to handle custom backward passes for nested circuits, and scaleblocks |
just wondering how the gate-level params are enforced, is that done automatically when choosing the |
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.
First part of the review. Overall, it looks great to me but there is one very important point missing here: documentation. I would at least add a small example in the getting started tutorial + a script in the examples/
folder.
@nmheim here https://github.com/pasqal-io/qadence/blob/ds/adjoint/qadence/extensions.py#L97 |
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.
Thanks, @dominikandreasseitz. I am pre-approving this but I would add proper documentation before merging.
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.
Very nice @dominikandreasseitz. It is much clearer for me after our discussion. Very minor comment for the sake of discussion. Otherwise LGTM.
Fixes #97
This MR adds a "adjoint" differentiation mode to qadence which is a implementation of Algorithm 1 of https://arxiv.org/pdf/2009.02823.pdf.
Limitations:
pyqtorch
backendAdditional changes:
Future Todos: