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

Adds utility functions for the general parameter-shift rule #1932

Merged
merged 92 commits into from
Dec 2, 2021

Conversation

josh146
Copy link
Member

@josh146 josh146 commented Nov 24, 2021

Context: In #1788, a utility function to generate parameter-shift rules, given an operators spectrum, was added. As per the recent gradient ADR, the decision was made to use this pipeline as the default method for the parameter-shift rule in PennyLane, and deprecate the Operator.grad_recipe attribute.

This PR introduces several changes, and new utility function, in preparation for this move.

Description of the Change:

  • The function get_shift_rule was renamed to _get_shift_rule, and wrapped by a new user-facing wrapper function generate_shift_rule, to emphasize that the shift rule is computed, rather than retrieved.

  • As the grad_recipe is due to be removed from the parameter-shift logic (kept only as a fallback option), generate_shift_rule has been modified to simply return an array of shifts and coefficients.

  • generate_shift_rule automatically removes negative frequencies, so that it can be used with qml.fourier.spectrum (which returns both positive and negative frequencies).

  • _process_gradient_recipe has been added, which processes the shifts and coefficients to remove redundant shifts. In particular:

    • All small coefficients and shifts (up to a certain tolerance) are set to 0

    • Terms where the coefficients are 0 are removed

    • The terms are sorted as per abs(shift), to ensure that any unshifted terms are first.

    • Terms with the same shift value are combined, with corresponding coefficients summed.

  • A utility function, eigvals_to_frequencies is provided, allowing users to easily convert generator eigenvalues to the corresponding frequency spectrum.

  • A utility function, frequencies_to_period, is provided, allowing users to determine the period of f(t) = <0|U(t)† O U(t)|0> given the frequencies of G (where U(t) = exp(iGt)).

  • generate_shift_rule can now be used to return iterated shift rules that return higher order derivatives of an operator (e.g., ∂^2 f/∂t^2

  • generate_multi_shift_rule can now be used to return iterated shift rules that return higher order derivatives of multiple operators (e.g., ∂^(n+m) f / ∂x^n ∂y^m). This can be used to generate shift rules for the off-diagonal elements of Hessians.

  • CommutingEvolution has been updated to take into account these changes.

Benefits:

  • It is now possible to query for shift rules of multiple operations, to arbitrary order.

  • It is easier to understand the output of generate_shift_rules.

Possible Drawbacks: n/a

Related GitHub Issues: n/a

ral9000 and others added 30 commits October 21, 2021 23:17


def generate_multi_shift_rule(frequencies, shifts=None, orders=None):
r"""Computes the parameter shift rule with respect to two parametrized unitaries,
Copy link
Contributor

Choose a reason for hiding this comment

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

I think I need a bit more explanation here, are those gates like U(theta1) U(theta2)?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, exactly!

Copy link
Contributor

@mariaschuld mariaschuld left a comment

Choose a reason for hiding this comment

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

I am not sure I understood and checked all details, but what would help me a lot is if the docstrings would translate jargon to actual data structures (i.e. an array of shifts for parameter shift rules). I think they don't need to go deep, but at the moment they use a lot of terms that have no meaning to an outsider.

The small functions look otherwise very useful!

A quick general question: since we push this into the release, are we sure the names and functionality will stay like this, or is there an uncertainty?

Other than that, are we not testing the dispatch/math methods? (I forgot)

So not a strong "request changes", happy to accept asap!

Copy link
Contributor

@dwierichs dwierichs left a comment

Choose a reason for hiding this comment

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

Very nice set of utility functions! This looks super cool to use already and I'm exited to start using them.
Regarding the frequencies_to_period utility, we should either adapt the docstring or modify it to use the greatest common divisor (GCD) instead of f_min.

For the docstring of the multi shift rule I might have misunderstood something, but if not, I made a couple of suggestions there as well :)

doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
doc/releases/changelog-dev.md Outdated Show resolved Hide resolved
pennylane/gradients/general_shift_rules.py Outdated Show resolved Hide resolved
pennylane/gradients/general_shift_rules.py Outdated Show resolved Hide resolved
pennylane/gradients/general_shift_rules.py Outdated Show resolved Hide resolved
pennylane/gradients/general_shift_rules.py Outdated Show resolved Hide resolved
tests/gradients/test_general_shift_rules.py Show resolved Hide resolved
tests/gradients/test_general_shift_rules.py Show resolved Hide resolved
tests/interfaces/test_batch_autograd.py Show resolved Hide resolved
josh146 and others added 4 commits December 1, 2021 20:12
Co-authored-by: David Wierichs <davidwierichs@gmail.com>
Co-authored-by: Maria Schuld <mariaschuld@gmail.com>
@josh146
Copy link
Member Author

josh146 commented Dec 1, 2021

Thanks @mariaschuld @dwierichs! Your comments were super helpful. I've implemented every one. Two of the biggest changes are:

  • I've removed the term 'gradient recipe' everywhere, as I realize this is technical jargon that makes it harder for external developers. Instead, I've replaced it with 'gradient rule', which should be more familiar due to 'parameter-shift rule'.

  • I've updated frequencies_to_period to use np.gcd, and added tests for non-equidistant frequencies 🙂


Given a Hamiltonian's frequency spectrum of `R` unique frequencies, `qml.gradients.get_shift_rule`
returns the parameter shift rules to compute expectation value gradients of the Hamiltonian's
Assuming that :math:`H` has `R` unique frequencies, `qml.gradients.generate_shift_rule`
Copy link
Contributor

@mariaschuld mariaschuld Dec 1, 2021

Choose a reason for hiding this comment

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

The frequencies of an operator are not a standard term. I fear we have to link to something where people can find an explanation? Or say "frequencies (i.e., the unique differences of any two eigenvalues)"

Sorry for not picking this up before

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, I suppose we use it as a shortcut for

Fourier-series frequencies of the function f(t) = <0|U(t)† O U(t)|0>, for some observable O where U(t)=exp(iHt).

Copy link
Member Author

Choose a reason for hiding this comment

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

@mariaschuld I've updated the changelog here: 680164c Let me know if that reads better!

Copy link
Contributor

@mariaschuld mariaschuld left a comment

Choose a reason for hiding this comment

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

I'm approving to fast-track, but consider implementing the wording changes in the changelog!

I think "frequencies of an operator" is a Xanadu-only term :)

Copy link
Contributor

@dwierichs dwierichs left a comment

Choose a reason for hiding this comment

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

Happy to approve! The only open thing from my perspective is the simplification to skip abs in eigvals_to_frequencies but that's also a tiny detail and no blocker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review-ready 👌 PRs which are ready for review by someone from the core team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants