Skip to content
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

[Return-types #4] Parameter shift grad transform #2886

Merged
merged 160 commits into from
Sep 16, 2022

Conversation

antalszava
Copy link
Contributor

@antalszava antalszava commented Aug 2, 2022

Context:
This is a continuation of the new return types push that was started in #2814 #2815 #2860.

Description of the Change:
Updates the parameter-shift gradient transform to handle the new way of returning measurement results.

Benefits:
The parameter-shift gradient transform outputs sequences.

Note: most of this PR includes porting over the tests/gradients/test_parameter_shift.py file to be compatible with the new return types structure. Most of the tests are copied from that file and adjusted.

Possible Drawbacks:
N/A

Related GitHub Issues:
N/A

TODOs:

  • QNode integration (along with updated examples in the docstring);
  • Shot vector integration;
  • Parameter broadcasting integration.

Copy link
Contributor

@AlbertMitjans AlbertMitjans left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All my comments/questions have been addressed, thus I will approve this PR.

However I believe this PR should need a second approval before merging.

Copy link
Contributor

@eddddddy eddddddy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @antalszava 🔥 🔥 🔥

My major comments have all been addressed. I just have 2 more minor requests to clean up the code a bit. Also briefly looked over the tests and they all seem good too 🙂 .

pennylane/gradients/parameter_shift.py Outdated Show resolved Hide resolved
pennylane/gradients/parameter_shift.py Outdated Show resolved Hide resolved
pennylane/gradients/parameter_shift.py Outdated Show resolved Hide resolved
Copy link
Contributor

@rmoyard rmoyard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @antalszava 💯 I've left some about tests that need to be commented. It would also be great to remove the changes for the execution pipeline. Still an open question about creating new gradients function to keep cleaner or do we consider them user facing and then those changes are all good?

tests/returntypes/test_parameter_shift_new.py Outdated Show resolved Hide resolved
tests/returntypes/test_parameter_shift_new.py Outdated Show resolved Hide resolved
tests/returntypes/test_parameter_shift_new.py Outdated Show resolved Hide resolved
tests/returntypes/test_parameter_shift_new.py Outdated Show resolved Hide resolved
tests/returntypes/test_parameter_shift_new.py Outdated Show resolved Hide resolved
pennylane/_qubit_device.py Show resolved Hide resolved
pennylane/interfaces/execution.py Outdated Show resolved Hide resolved
pennylane/gradients/parameter_shift.py Outdated Show resolved Hide resolved
pennylane/gradients/parameter_shift.py Outdated Show resolved Hide resolved
pennylane/gradients/parameter_shift.py Outdated Show resolved Hide resolved
Copy link
Contributor

@rmoyard rmoyard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @antalszava, I suggest to make the var function private otherwise it looks very good! 💯

pennylane/gradients/parameter_shift.py Outdated Show resolved Hide resolved
pennylane/gradients/parameter_shift.py Outdated Show resolved Hide resolved
tests/returntypes/test_parameter_shift_new.py Show resolved Hide resolved
@antalszava antalszava changed the title [Return-types #4] Parameter shift grad transform - new return types rework [Return-types #4] Parameter shift grad transform Sep 16, 2022
@antalszava antalszava merged commit a91e84f into master Sep 16, 2022
@antalszava antalszava deleted the grad_transforms_new_return branch September 16, 2022 13:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants