Improve Amplitude Estimation Algos and Tests

[Cryoris]

Summary

Add more detailed tests, fix tiny bugs, improve comments and docstrings.

Details and comments

• Based on discussion with @dongreenberg more tests to all AE variants were added, that are not only end-to-end tests (as is the current state).
• MLAE was missing the 0 in the evaluation schedule.
• All AE variants now return the number of Q-oracle queries (important for benchmarks, @stefan-woerner).
• In general, docstrings and comments were improved.

[woodsp-ibm]

1. Naming changes:

• log_max_evals to m seems like going from a meaningful naming to non-descript
• ci_method to confint_method seems like it adds little for a user to help them
  both the above break compatibility for what does not seem substantial value.

2. Ordering of parameter changes - seems the intent is to have all the Amplitude Estimation algos have same order of a/q_factory and i_objective but original AE is still not there

3. Variable naming exception 'ae' to be removed that was added to list - use 'qae' in the unit test

[Cryoris]

Thanks for the review!

1. Naming changes:

• log_max_evals to m seems like going from a meaningful naming to non-descript

I renamed it to num_oracle_circuits, which is more accurate than log_max_evals.

• ci_method to confint_method seems like it adds little for a user to help them
  both the above break compatibility for what does not seem substantial value.

As discussed, I'm leaving this since we have breaking changes through reordering the input arguments to a consistent order.

2. Ordering of parameter changes - seems the intent is to have all the Amplitude Estimation algos have same order of a/q_factory and i_objective but original AE is still not there

Thanks! They all have the same order now.

3. Variable naming exception 'ae' to be removed that was added to list - use 'qae' in the unit test

Changed to qae.

Also I changed the tests that checked the number of gates by a check for the unitary the circuit produces. Comparing the circuits directly didn't work, since this doesn't capture things such as U1(pi) == U1(-pi).

[dongreenberg]

Approved, but please address renaming comment for the QPE-based AE.

[dongreenberg]

Can we rename this to be QPEAmplitudeEstimation, and say in the comment on line 38 that this is "The original, Quantum Phase Estimation-based Amplitude Estimation Algorithm."? I think it may be strange to have one algorithm called AmplitudeEstimation but then a base class called AmplitudeEstimationAlgorithm. It would be better to be descriptive. If so, we should also rename the file qpe_ae.py, yes?

[woodsp-ibm]

The alternative might be to rename the base class, which would be less disruptive - otherwise notebooks, test cases and any users code would need to alter too. I do not know how much, or if at all, externally that base class type might be used at present outside of the package. I suspect it could be renamed without the same impact as the algorithm derived from it.

[Cryoris]

AmplitudeEstimation is used quite a bit in notebooks (and our codes), I also prefer to keep it. As discussed on Slack I'll update the the docstring though.

[dongreenberg]

Thanks Julien, I agree that the docstring should suffice.