goodchemistryco / Tangelo

A python package for exploring end-to-end chemistry workflows on quantum computers and simulators.
https://goodchemistryco.github.io/Tangelo/
Other
99 stars 27 forks source link

Unitaryhack issue 386: Augment the Rotosolve optimizer to support Rotoselect #392

Closed cburdine closed 1 month ago

cburdine commented 1 month ago

This is a PR to address Issue 386: Augment the Rotosolve optimizer to support Rotoselect

:sparkles: New Features:

:construction: Changes to Existing Features:


:memo: Footnote on optimizing rotosolve():

The existing rotosolve_step() function used $\phi = 0$ by default, which is reasonable because it allows for the removal of the gate at compile time. However, if $\phi = \theta{d-1}^\ast$ is used, then the extrapolated expectation value of $\langle M \rangle{\theta_{d-1}^\ast}$ returned from the previous call to rotosolve_step() can be used in place of a direct evaluation of it. rotosolve_step() has been modified so that the gate angle $\phi$ and a previously extrapolated expectation value $\langle M \rangle{\theta{d-1}^\ast}$ can be passed into rotosolve_step() via the optional arguments phi and m_phi respectively. By selecting $\phi = \theta{d-1}^\ast$ and using the extrapolated $\langle M \rangle{\phi} = \langle M \rangle{\theta{d-1}^\ast}$ from the previous optimized parameter, we only need to evaluate $\langle M \rangle{\phi + \pi/2}$ and $\langle M \rangle{\phi + \pi/2}$ to optimize the current parameter. In total, this requires 2/3 the number of evaluations, as long as the extrapolated value is accurate.

On an ideal quantum computer with no sampling error, this optimized rotosolve() (i.e. with extrapolate=True) should behave identically to the previous unoptimized version. However, I anticipate that it may perform slightly worse than the unoptimized rotosolve() on noisy devices for the same number of iterations. While the unoptimized version (i.e. with extrapolate=False) is more stable, it requires 50% more evaluations than the optimized version. In terms of the total number of evaluations (which is probably the best basis of comparison), I am not sure which will perform better under noise. If in the future you find that the optimized version is too unstable for noisy devices, then you may consider removing the extrapolate argument.

ValentinS4t1qbit commented 1 month ago

@cburdine Thank you so much for the PR and the detailed description ! @elloyd-1qbit and I will take a look at it.

In the meantime, it seems our linter (pycodetest) picked up some stuff and will make your tests fail. If you ask for details, and open the menus you'll see the code files and lines that were brought up.

It seems at first glance all unit tests pass though, which is rad :)

cburdine commented 1 month ago

@ValentinS4t1qbit and @elloyd-1qbit, there's one additional point that I would like your feedback on.

Currently rotoselect requires a function of the form

def exp_rotoselect(var_params, var_rot_axes, ansatz, qubit_hamiltonian):
            ansatz.update_var_params(var_params)
            for i, axis in enumerate(var_rot_axes):
                ansatz.circuit._variational_gates[i].name = axis
            energy = sim.get_expectation_value(qubit_hamiltonian, ansatz.circuit)
            return energy

to be passed in, which requires the user to manually modify the protected member ansatz.circuit._variational_gates.

I'm thinking we could add a ansatz.update_var_gates() function or perhaps add a gates argument to ansatz.update_var_params() to make this more seamless. Any thoughts?

elloyd-1qbit commented 1 month ago

Well done! I agree adding a ansatz.update_var_gates() function would be really handy.

ValentinS4t1qbit commented 1 month ago

@cburdine To clarify: do we intend to add ansatz.update_var_gates() in this PR ? Does it feel straightforward enough to implement ?

Or would the recommendation be opening a "feature suggestion" issue ? I know you're working on the hackathon, I want to be mindful of your time.

cburdine commented 1 month ago

@ValentinS4t1qbit It looks like adding an update_var_gates() function will require a change to the interface in the abstract class Ansatz. Functions will then need to be implemented for each ansatz class individually. Test cases will also need to be added to each of the ansatzes, which may take some time. (If you can think of a better way of doing this, let me know! :bulb:)

I'm currently working on getting the first draft of a PR for issue 384 submitted, which is similar in that it requires extending the functionality of each ansatz class individually. Once I get that submitted I will circle back to this PR and add this feature.