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

Add qml.generator(op) function #2256

Merged
merged 60 commits into from
Mar 3, 2022
Merged

Add qml.generator(op) function #2256

merged 60 commits into from
Mar 3, 2022

Conversation

josh146
Copy link
Member

@josh146 josh146 commented Mar 1, 2022

Context:

Currently, to get the generator of an operator, you must do op.generator().

However, there are many downsides to this:

  • op.generator() is overwritten by each operator, and thus does not take into account behaviour like operator inverses

  • Operators can return arbitrary operators as generators. Until this is standardized, we need to consider Hamiltonians and matrices separately!

  • If the generator is a Pauli word, it is sometimes nice to be able to extract the 'prefactor'.

Currently, qml.utils.get_generator() is available which is a temporary function to address the above.

Description of the Change:

  • Adds qml.generator(), which generalizes the temporary qml.utils.get_generator()

  • Remove qml.utils.get_generator().

  • Generalize qml.generator() to return generators in various different formats:

    • "prefactor", as per the old system
    • "observable", as per the operator defines
    • "hamiltonian", always return a Hamiltonian.

Benefits: As above.

Possible Drawbacks: Not a drawback per se, but this function will become a lot simpler once the operator subclasses are merged.

Related GitHub Issues: n/a

@github-actions
Copy link
Contributor

github-actions bot commented Mar 1, 2022

Hello. You may have forgotten to update the changelog!
Please edit doc/releases/changelog-dev.md with:

  • A one-to-two sentence description of the change. You may include a small working example for new features.
  • A link back to this PR.
  • Your name (or GitHub username) in the contributors section.

@josh146 josh146 changed the title [WIP] Add qml.generator(op) function Add qml.generator(op) function Mar 1, 2022
@codecov
Copy link

codecov bot commented Mar 1, 2022

Codecov Report

Merging #2256 (dcf79f4) into master (d1a03a9) will increase coverage by 0.02%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2256      +/-   ##
==========================================
+ Coverage   99.28%   99.31%   +0.02%     
==========================================
  Files         236      237       +1     
  Lines       18869    18899      +30     
==========================================
+ Hits        18734    18769      +35     
+ Misses        135      130       -5     
Impacted Files Coverage Δ
pennylane/fourier/utils.py 100.00% <100.00%> (ø)
pennylane/operation.py 96.71% <100.00%> (+0.72%) ⬆️
pennylane/ops/functions/__init__.py 100.00% <100.00%> (ø)
pennylane/ops/functions/generator.py 100.00% <100.00%> (ø)
pennylane/tape/reversible.py 100.00% <100.00%> (ø)
pennylane/transforms/adjoint_metric_tensor.py 100.00% <100.00%> (ø)
pennylane/transforms/metric_tensor.py 99.45% <100.00%> (ø)
pennylane/transforms/op_transforms.py 100.00% <100.00%> (ø)
pennylane/utils.py 97.97% <100.00%> (-0.26%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d1a03a9...dcf79f4. Read the comment docs.

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.

Nice addition @josh146 ! :)
Once the minor comments are addressed I'm happy to approve.
Do you plan to include the changes that allow multi-version compatibility which we discussed earlier in this PR or a new one?

@@ -63,8 +63,7 @@ def get_spectrum(op, decimals):
Returns:
set[float]: non-negative frequencies contributed by this input-encoding gate
"""
matrix, coeff = get_generator(op, return_matrix=True)
matrix = coeff * matrix
matrix = qml.matrix(qml.generator(op, format="observable"))
Copy link
Contributor

Choose a reason for hiding this comment

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

This could have been done in #2182 already, but now that we're touching this code, we could simply use parameter_frequencies instead of this whole utility function? Might be a separate PR, though (maybe together with Rotosolve using parameter_frequencies, grouping it into an "application" PR).

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 idea!

Copy link
Member Author

Choose a reason for hiding this comment

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

@dwierichs which would you prefer, doing it here, or doing it in a separate PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess a separate one that uses parameter_frequencies where applicable is more clean? :)

Copy link
Member Author

Choose a reason for hiding this comment

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

yep :)

pennylane/operation.py Outdated Show resolved Hide resolved
pennylane/ops/functions/generator.py Outdated Show resolved Hide resolved
pennylane/ops/functions/generator.py Outdated Show resolved Hide resolved
pennylane/ops/functions/generator.py Outdated Show resolved Hide resolved
pennylane/transforms/op_transforms.py Outdated Show resolved Hide resolved
pennylane/transforms/op_transforms.py Show resolved Hide resolved
pennylane/utils.py Show resolved Hide resolved
Comment on lines -1320 to +1325
op = qml.Rot(0.1, 0.2, 0.3, wires=0)

class CustomOp(qml.operation.Operation):
num_wires = 1
num_params = 1

op = CustomOp(0.5, wires=0)
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiousity: Why was this change required? :)

Copy link
Member Author

Choose a reason for hiding this comment

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

@cvjjm suggested (very wisely) to move the num_params validation before getting the generator. So this test started raising 'Generator only supports single param operations' instead!

I couldn't think of another gate that would fit the test, and probably better practise to have test data designed for the test.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the explanation :)

pennylane/ops/functions/generator.py Outdated Show resolved Hide resolved
Comment on lines +1295 to +1302
with warnings.catch_warnings():
warnings.filterwarnings(
action="ignore", message=r".+ eigenvalues will be computed numerically\."
)
eigvals = qml.eigvals(qml.generator(self, format="observable"))

if isinstance(gen, qml.SparseHamiltonian):
mat = gen.sparse_matrix().toarray()
eigvals = tuple(np.round(np.linalg.eigvalsh(mat), 8))
return qml.gradients.eigvals_to_frequencies(eigvals)
eigvals = tuple(np.round(eigvals, 8))
return qml.gradients.eigvals_to_frequencies(eigvals)
Copy link
Member Author

Choose a reason for hiding this comment

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

@dwierichs I made the change here because I realized the old version was simply duplicating qml.eigvals(); this makes the method cleaner, and less likely to break in the future.

Copy link
Contributor

@dwierichs dwierichs Mar 2, 2022

Choose a reason for hiding this comment

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

Oh great, this does look very nice :)

@josh146 josh146 requested a review from dwierichs March 2, 2022 14:25
Base automatically changed from eigvals-function to master March 2, 2022 17:48
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.

Looks good to me! :)

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.

3 participants