Numerical Integration methods for real and imaginary time evolution.

[MarcDrudis]

This addresses #8342. The goal of this PR is to implement 2 classes that inherit from RealEvolver and ImaginaryEvolver and solve the time evolution of a state using classical methods. Both classes will be very similar, but the RealEvolver will use a biconjugate gradient descent method to make each step (this allows the step evolution operator to be unitary) and for the ImaginaryEvolver a taylor expansion of the time evolution operator will be used at each timestep (and the state renormalized).

The user can specify a list, or dictionary, of observables and their expectancy value will be computed at each timestep and returned as a list or a dictionary respectively.

Lastly, there is a method that allows to estimate the number of timesteps needed to reach a certain error threshold given the norm of the hamiltonian to evolve under. I am not completely sure if this feature is over complicating the interface unnecessarily, so let me know what you think about it. Basically it consists on taking the next taylor expansion term and using it to compute the minimum ammount of timesteps necessary to yield a given error.

[qiskit-bot]

Thank you for opening a new pull request.

Before your PR can be merged it will first need to pass continuous integration tests and be reviewed. Sometimes the review process can be slow, so please be patient.

While you're waiting, please feel free to review other open PRs. While only a subset of people are authorized to approve pull requests for merging, everyone is encouraged to review open pull requests. Doing reviews helps reduce the burden on the core team and helps make the project's code better for everyone.

One or more of the the following people are requested to review this:

• @Qiskit/terra-core
• @manoelmarques
• @woodsp-ibm

[CLAassistant]

All committers have signed the CLA.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

:white_check_mark: MarcDrudis
:x: Marc Sanz Drudis

---

Marc Sanz Drudis seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[woodsp-ibm]

The evolvers package will be pending deprecation as of this upcoming release and will be deprecated and removed later. These files should now be introduced into the time_evolvers package.

[MarcDrudis]

The evolvers package will be pending deprecation as of this upcoming release and will be deprecated and removed later. These files should now be introduced into the time_evolvers package.

Yes, I am working on it. Do you still want me to do this?:

I guess I missed this when the evolution result was done but we normally have private fields and setter/getter 
properties to document these as an end user does not use the constructor normally rather they will access the
fields. We could do the Attributes here too so as to have them documented properly

[woodsp-ibm]

The evolvers package will be pending deprecation as of this upcoming release and will be deprecated and removed later. These files should now be introduced into the time_evolvers package.

Yes, I am working on it. Do you still want me to do this?:

I am not quite sure I follow. The time_evolvers module exists now in main - the files in this PR will need to be moved over there before they are finally included/merged. I believed you were aware evolvers was being deprecated, just wanted to ensure that by commenting here since there was recent activity in this PR. They will need to be moved at some point but it can be done when you are ready.

Maybe you were referring to the documentation comment below the "Do you still want me to do this?:" and not the prior quoted comment. Yes any public instance vars need to be documented via Attributes. Here's an example in PVQD https://github.com/Qiskit/qiskit-terra/blob/a5a8657fdcf4c94da7b1311e1b041b0f34cae7a0/qiskit/algorithms/time_evolvers/pvqd/pvqd.py#L50

[coveralls]

Pull Request Test Coverage Report for Build 3743481918

• 103 of 110 (93.64%) changed or added relevant lines in 7 files are covered.
• 4 unchanged lines in 2 files lost coverage.
• Overall coverage increased (+0.04%) to 84.58%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
qiskit/algorithms/time_evolvers/classical_methods/evolve.py	70	77	90.91%
<!--	Total:	103	110

Files with Coverage Reduction	New Missed Lines	%
qiskit/extensions/quantum_initializer/squ.py	2	79.78%
src/sabre_swap/layer.rs	2	98.95%
<!--	Total:	4

Totals
Change from base Build 3743356210:	0.04%
Covered Lines:	63897
Relevant Lines:	75546

---

💛 - Coveralls

[MarcDrudis]

The evolvers package will be pending deprecation as of this upcoming release and will be deprecated and removed later. These files should now be introduced into the time_evolvers package.

Yes, I am working on it. Do you still want me to do this?:

I am not quite sure I follow. The time_evolvers module exists now in main - the files in this PR will need to be moved over there before they are finally included/merged. I believed you were aware evolvers was being deprecated, just wanted to ensure that by commenting here since there was recent activity in this PR. They will need to be moved at some point but it can be done when you are ready.

Maybe you were referring to the documentation comment below the "Do you still want me to do this?:" and not the prior quoted comment. Yes any public instance vars need to be documented via Attributes. Here's an example in PVQD

https://github.com/Qiskit/qiskit-terra/blob/a5a8657fdcf4c94da7b1311e1b041b0f34cae7a0/qiskit/algorithms/time_evolvers/pvqd/pvqd.py#L50

Yes, I was refering to the documentation. I thought we had to make getters and setters in order to properly document the class, but I am seeing that there has been a Attributes bit added to the documentation in time_evolver_result with respect to evolver_result. I guess that ought to do it, right?

[woodsp-ibm]

Yes, I was refering to the documentation. I thought we had to make getters and setters in order to properly document the class, but I am seeing that there has been a Attributes bit added to the documentation in time_evolver_result with respect to evolver_result. I guess that ought to do it, right?

We have been trying to get away from having simple getter/setters than do nothing more that give access to some private var, with no further computation, whose main purpose was really documentation, in favor of just having public instance vars and documenting them via Attributes. You should see this more and more in the new (& refactored) algorithms like PVQD example I gave,

[MarcDrudis]

Yes, I was refering to the documentation. I thought we had to make getters and setters in order to properly document the class, but I am seeing that there has been a Attributes bit added to the documentation in time_evolver_result with respect to evolver_result. I guess that ought to do it, right?

We have been trying to get away from having simple getter/setters than do nothing more that give access to some private var, with no further computation, whose main purpose was really documentation, in favor of just having public instance vars and documenting them via Attributes. You should see this more and more in the new (& refactored) algorithms like PVQD example I gave,

I personally like it better as well. I am still changing documentation and so on. I will let you know once I am done. Thank you very much.

[MarcDrudis]

I know there is a realease comming, so feel free to ignore this until November (I am going to be away anyhow). The last version I merged should be running correctly. I was having an issue because my version of scipy was 1.9.2 and the CI has 1.7.3 which is lacking some features that I was using. The workaround is practically as fast, so no problem from that side. Apart from that the rest of the code has been changed and documented. Now instead of using the unitary approximation that we were using we are just using single steps of scipy.expm_multiply which is faster and still much more precise (basically double precision). For that reason there is not a way of adjusting the tolerance of the algorithm. It would still be interesting to decide whether we want to renormalize the state at each timestep for the RealTimeEvolution or not, and whether we want to rename the classes to ScipyRealTimeEvolver or it is too long.

[MarcDrudis]

Hello, I am finally reviewing @dlasecki comments on the PR. I am almost done with most of them, but I wanted to bring up something. Some time ago there was a comment by @woodsp-ibm saying that since the methods that are currently in utils.py were not using any attributes of a class they should be standalone functions (as they currently are). Now, @dlasecki is proposing that since most of the logic of the algorithms resides in the functions from utills.py it should all become a class that both ScipyRealEvolver and ScipyImaginaryEvolver would inherit from. That would mean that ScipyRealEvolver, for example, would inherit from RealEvolver and ScipyEvolver. I see pros and cons to both options, could you give me some insight on what would be a better practice? My gut tells me to go with ScipyEvolver as an abstract class, but since it wouldn't have any attributes I also see how all it would be doing is just wrapping arround a bunch of methods.

[woodsp-ibm]

My gut tells me to go with ScipyEvolver as an abstract class, but since it wouldn't have any attributes I also see how all it would be doing is just wrapping around a bunch of methods.

Would we ever be using ScipyEvolver as a type so that encapsulating the methods as functions within such a type makes sense. Currently utils is a collection of functions that are not instance (self) dependent and they are all private right. If such a type existed I assume nothing would be made part of public API so whatever the type was would more be for internal/implementation usage. Unless there is more to it than just make a class to put these into in some way (static methods?) I would question why we would be doing that.

[MarcDrudis]

My gut tells me to go with ScipyEvolver as an abstract class, but since it wouldn't have any attributes I also see how all it would be doing is just wrapping around a bunch of methods.

Would we ever be using ScipyEvolver as a type so that encapsulating the methods as functions within such a type makes sense. Currently utils is a collection of functions that are not instance (self) dependent and they are all private right. If such a type existed I assume nothing would be made part of public API so whatever the type was would more be for internal/implementation usage. Unless there is more to it than just make a class to put these into in some way (static methods?) I would question why we would be doing that.

I can agree with that. In that case I am just going to leave it as it is.

[Cryoris]

@mergifyio requeue

[mergify[bot]]

requeue

✅ The queue state of this pull request has been cleaned. It can be re-embarked automatically