-
Notifications
You must be signed in to change notification settings - Fork 15
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
Basis expansion method with expand
#24
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remaining comments which cannot be posted as a review comment to avoid GitHub Rate Limit
@emstoudenmire ITensor/ITensorNetworks.jl#43 is now merged so this can be moved to that repository. |
@emstoudenmire seems like people are itching to use this, but that it became outdated since it is relying on an internal ITensors.jl function ( Should we try to update it and merge it, since it seems critical for allowing some people to use TDVP? |
Something I see that is missing is expanding by a sum of Hamiltonians, I guess that would be easy enough to add on later. |
@emstoudenmire is there a way to organize the |
@emstoudenmire I've done a pretty extensive rewrite, though it is mostly aesthetic changes and the basic algorithmic structure remains the same. I've also proposed a new interface, let me know what you think. I may go ahead and refactor by first making an MPS containing the expansion basis ( |
I think it looks really good. Definitely the names and the interface are much improved (and of course things like keyword handling are better). Yes, so the main other things left for this PR in my mind were:
Probably we can be more practical and just give it to users to try and learn from them whether it's working for their projects, or whether they need to e.g. use a very different krylov dim or other parameters. I think it ought to work in most cases as is. |
@emstoudenmire in the latest I changed I'm going back and forth about whether the MPO/MPS that are used to determine the expansion basis should be arguments or keyword arguments but I think arguments make more sense since then they can be dispatched on. I was also going back and forth about what the meaning of using KrylovKit
using LinearAlgebra
n = 10
A = hermitianpart(randn(n, n))
krylovdim = 3
vals, vecs, info = eigsolve(A, 1; krylovdim, maxiter=1)
@show krylovdim, info.numops outputs: (krylovdim, info.numops) = (3, 3) In the latest I went with the design that |
I tried that but gave up, I think it is trickier than I imagined because of how the basis of the state get modified during the algorithm (which I think you had found as well). We can investigate it more in future work. I was thinking it could be a good forward-looking design for implementing global basis expansion for trees and general tensor networks in ITensorNetworks.jl so we can investigate it there.
I kept the default of
Agreed. |
Support for this will also be added to ITensorMPS.jl with this PR: ITensor/ITensorMPS.jl#17. |
Great. I think I like just plain "expand" also for the reason you said. Hopefully it is clear from doc strings and arguments what exactly it does in case of any confusion. Like if later there is an expand which increases the number of sites it would take different arguments like an index array. Makes sense about delaying the sum or + design until we can figure out the right algorithm to use there. And about giving it to users now. For the krylovdim I don't have too strong an opinion. Maybe the best thing is to think about what krylovdim of 0 or 1 means. So in your convention it means the basis would not be expanded. In Juthos it would mean one non-trivial Krylov vector Hv gets generated. So in his convention 0 is the case where nothing happens. That doesn't say which one is better but I thought it might clarify the decision. |
My feeling is that the convention where |
I think But anyway, I can't think of an alternative meaning for Additionally, expanding the site dimensions of an MPS may be a reasonable operation but that is much more specialized so should probably be called something like |
Adds the Yang and White basis extension method [arxiv:2005.06104]. Currently only uses Krylov vectors instead of vectors defined by
(1-tau*H)|psi>
which is one minor difference from their paper.Also includes a test code that (intentionally) isn't yet included in the test suite.
Here are some possible improvements:
extend
and through
basis_extend
big when used in imaginary time evolution
instead of H|psi>. Needed?