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 quantum phase estimation template #1095

Merged
merged 37 commits into from Mar 2, 2021
Merged

Conversation

trbromley
Copy link
Contributor

@trbromley trbromley commented Feb 19, 2021

Context:

Adds a QPE template.

Description of the Change:

Added a new template to the subroutines folder.

Benefits:

Users can now perform QPE!

Possible Drawbacks:

QPE is only available in matrix format. Hopefully that should be clear by the documentation.

@codecov
Copy link

codecov bot commented Feb 19, 2021

Codecov Report

Merging #1095 (5187414) into master (c267cd0) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1095   +/-   ##
=======================================
  Coverage   97.74%   97.74%           
=======================================
  Files         154      155    +1     
  Lines       11734    11753   +19     
=======================================
+ Hits        11469    11488   +19     
  Misses        265      265           
Impacted Files Coverage Δ
pennylane/templates/subroutines/__init__.py 100.00% <100.00%> (ø)
pennylane/templates/subroutines/qpe.py 100.00% <100.00%> (ø)

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 c267cd0...5187414. Read the comment docs.

@trbromley trbromley marked this pull request as ready for review February 25, 2021 20:36
Copy link
Contributor

@ixfoduap ixfoduap left a comment

Choose a reason for hiding this comment

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

Looks great! 🎉

I'm slightly worried that this implementation is overfitting for the quantum monte carlo application and may not be the optimal way of implementing the template for such a widely-used routine as quantum phase estimation.

The tests are great, but perhaps they could benefit from also testing correctness for different phases and unitaries



@template
def QuantumPhaseEstimation(unitary, target_wires, estimation_wires):
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to discuss a bit more whether we want to pass the unitary or its generator, i.e., the Hamiltonian. I'm a bit afraid that we're narrowing the scope to how QPE is used in quantum Monte Carlo. But QPE is used in a lot of places, for example in versions of Shor's algorithm, for preparing approximate ground states of Hamiltonians, and more.

Maybe we can compile a list of applications of QPE and that can help guide how best to create a template around it? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really like the suggestion about supporting input Hamiltonians, and yes you're right I hadn't been thinking so much in that direction.

So we have the two options for input:

  1. unitary, eventually as a circuit
  2. Hamiltonian, input as a qml.Hamiltonian, and evolution time t, and we can do exp(2 pi i H t).

For me, the best option would be to have two separate templates, given that the inputs and how they will be dealt with are quite different. In option 2, we'd need to use ApproxTimeEvolution, whereas option 1 would directly apply controlled versions of the circuit. In either case, we need access to a qml.control() transform.

For now, we've done option 1 with a matrix-based input. I'm not sure if it's worth supporting a matrix-based version of option 2, since one could just calculate the matrix exponential themselves. On the other hand, users typically have access to a qml.Hamiltonian and I couldn't work out how to get the matrix version of that! But with qml.Hamiltonian I can only see how to do it with qml.control().

So overall, I totally agree, but think we'd be better to wait for supporting functionality to come online.

One question might be what we call the two options, e.g.,

  1. QuantumPhaseEstimation or UnitaryPhaseEstimation?
  2. HamiltonianPhaseEstimation?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm happy to wait for more functionality to come online before supporting Hamiltonian inputs. I'm not sure if having two templates is preferable to supporting to different types of arguments, but we can discuss that in the future. Re names: let's also discuss in the future. I like keeping QuantumPhaseEstimation for this form of the template

pennylane/templates/subroutines/qpe.py Outdated Show resolved Hide resolved
pennylane/templates/subroutines/qpe.py Outdated Show resolved Hide resolved
pennylane/templates/subroutines/qpe.py Show resolved Hide resolved
pennylane/templates/subroutines/qpe.py Show resolved Hide resolved
pennylane/templates/subroutines/qpe.py Show resolved Hide resolved
tests/templates/test_subroutines.py Show resolved Hide resolved
tests/templates/test_subroutines.py Outdated Show resolved Hide resolved
tests/templates/test_subroutines.py Outdated Show resolved Hide resolved
Copy link
Contributor Author

@trbromley trbromley left a comment

Choose a reason for hiding this comment

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

Thanks @ixfoduap for the review!

tests/templates/test_subroutines.py Outdated Show resolved Hide resolved
tests/templates/test_subroutines.py Outdated Show resolved Hide resolved
pennylane/templates/subroutines/qpe.py Outdated Show resolved Hide resolved
pennylane/templates/subroutines/qpe.py Outdated Show resolved Hide resolved


@template
def QuantumPhaseEstimation(unitary, target_wires, estimation_wires):
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I really like the suggestion about supporting input Hamiltonians, and yes you're right I hadn't been thinking so much in that direction.

So we have the two options for input:

  1. unitary, eventually as a circuit
  2. Hamiltonian, input as a qml.Hamiltonian, and evolution time t, and we can do exp(2 pi i H t).

For me, the best option would be to have two separate templates, given that the inputs and how they will be dealt with are quite different. In option 2, we'd need to use ApproxTimeEvolution, whereas option 1 would directly apply controlled versions of the circuit. In either case, we need access to a qml.control() transform.

For now, we've done option 1 with a matrix-based input. I'm not sure if it's worth supporting a matrix-based version of option 2, since one could just calculate the matrix exponential themselves. On the other hand, users typically have access to a qml.Hamiltonian and I couldn't work out how to get the matrix version of that! But with qml.Hamiltonian I can only see how to do it with qml.control().

So overall, I totally agree, but think we'd be better to wait for supporting functionality to come online.

One question might be what we call the two options, e.g.,

  1. QuantumPhaseEstimation or UnitaryPhaseEstimation?
  2. HamiltonianPhaseEstimation?

@trbromley trbromley requested a review from ixfoduap March 1, 2021 17:54


@template
def QuantumPhaseEstimation(unitary, target_wires, estimation_wires):
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm happy to wait for more functionality to come online before supporting Hamiltonian inputs. I'm not sure if having two templates is preferable to supporting to different types of arguments, but we can discuss that in the future. Re names: let's also discuss in the future. I like keeping QuantumPhaseEstimation for this form of the template

@trbromley trbromley merged commit 94cf6d4 into master Mar 2, 2021
@trbromley trbromley deleted the quantum_phase_estimation branch March 2, 2021 14:13
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.

None yet

3 participants