-
Notifications
You must be signed in to change notification settings - Fork 12
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 simple and full update algorithms #91
Conversation
Pb improve sequential
Thanks for this work! I will do my best to review in a lot more detail as soon as possible, but I want to thank you in advance for wanting to contribute. This is definitely appreciated! |
Pb improve sequential 2
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.
My comments consist mostly of some minor naming preferences, and maybe slight alterations to some of the code organization, but overall this looks great!
For function names as well as files, I would favor to just write names out in full, rather than having shorter names. This is definitely not entirely consistent with everything that already exists, as whenever functions in Base
are defined this is also sometimes shortened (i.e. prod
, ...).
Otherwise, let me focus on the simple update code first. (It might even be convenient to split this PR into two separate PRs, for SU and FU separately).
I think the it makes sense to combine the peps tensors and weights into a single struct, perhaps InfiniteWeightPEPS
could make sense? Considering that the state itself really consists of both, I think it is better to keep this data together, as the PEPS tensors don't really make up a state anymore without the weights, so keeping them in InfinitePEPS
might be slightly misleading. This would also make it somewhat more explicit to absorb and split off weights again, as you can convert between the types that way.
Code-organisation wise, it would be more in line with the other algorithms to define an algorithm struct that holds the settings:
struct SimpleUpdate
dt
trscheme
tol
maxiter
...
end
const SU = SimpleUpdate
which could then be used in perhaps fixedpoint
to have an equivalent interface as the gradient-based methods.
For the expectation values, you could leverage the existing functionality by first absorbing the weights, constructing a regular InfinitePEPS
, and then making use of the contract_localoperator
function defined here. That function combines constructing the reduced density matrices and applying the operators, hopefully to arrive at a better contraction order.
This is also how the models that are currently in PEPSKit are defined, as a list of local operators (coordinates => tensors). In that way, you should be able to extract the "gates" from these structures as the elements of that list.
If you prefer, I can definitely have a look and bring your implementation a bit in line with the rest. I can implement some changes and suggest them to you via a different PR, so you could have a look and let me know what you think?
@@ -371,6 +371,44 @@ function Base.rotl90(env::CTMRGEnv{C,T}) where {C,T} | |||
return CTMRGEnv(copy(corners′), copy(edges′)) | |||
end | |||
|
|||
# in-place rotations (incompatible with autodiff) |
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.
Is there much use to having these in-place? The tensors will still be allocated, so effectively this is only re-using the outer arrays of pointers, which are typically very small.
I have no issues with having more in-place operations defined (in fact it would be great to see to what extent this is feasible in CTMRG while keeping the AD parts working), but here I would expect that also the permutations would take place in-place, ie something of the form rotl90!(dest, src)
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.
It's mainly used to avoid writing many things to return... I would appreciate it if you can help optimize my current implementations of the in-place functions (which are indeed just reusing the original pointers).
src/states/suweight.jl
Outdated
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.
If I understand everything correctly, it might make sense to bundle this together with the peps itself, ie something like this:
struct InfiniteWeightPEPS{T,E} <: AbstractPEPS
vertices::Matrix{T}
horizontal_weights::Matrix{E}
vertical_weights::Matrix{E}
end
This might be easier to work with?
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.
I'll try to implement this. However, I think it is better to keep SUWeight
since there are some operations that only involve the weights. For example, averaging the weights after every SU step can speed up the convergence a little for the Heisenberg model. I will define the struct as
struct InfiniteWeightPEPS{T<:PEPSTensor,E<:PEPSWeight} <: AbstractPEPS
vertices::Matrix{T}
weights::SUWeight{E}
end
where
const PEPSWeight{S} = AbstractTensorMap{S,1,1} where {S<:ElementarySpace}
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.
Sounds great to me, I mostly care about bundling it with the state, less about the names and structures of the fields.
test/heisenberg_sufu.jl
Outdated
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.
I would definitely be okay with merging this with the test/heisenberg.jl
file.
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.
Is there a recommended time limit for one test file? On my Mac with M1 Pro chip, it will take about 10 minutes.
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.
I don't think there is really an obvious time limit. For these PEPS things, it's really tricky to find things that are representative but still reasonably quick... Maybe by tweaking the tolerances we can make it run a little faster, as it doesn't need to be so precise?
Co-authored-by: Lukas Devos <[email protected]>
Co-authored-by: Lukas Devos <[email protected]>
@lkdvos I prefer not to unify the simple/full update under the I now close this PR and divert future discussions to #97 focusing solely on the simple update. Full update will be added later after the SU code is merged. |
This PR adds the core part of two (imaginary) time evolution algorithms: simple update (arXiv 0806.3719) and full update (arXiv 1503.05345). Despite being (much) less accurate than gradient-based ground state searching algorithms, states obtained from imaginary time evolution can usually serve as a good initialization for the former.
Main changes:
SUWeight
, which are diagonal Schmidt weight matrices on the bonds of the iPEPS and serves as a crude approximation of the environment of each site tensor.absorb_wt
is used to absorb theSUWeight
into the iPEPS site tensors;simpleupdate!
update the iPEPS site tensors andSUWeight
in place;fullupdate!
update the iPEPS and theCTMRGEnv
in place.:sequential
act column-wise #90) recently implemented by @pbrehmer. I picked out a sub-functionctmrg_leftmove
fromctmrg_iter
, which only perform the left move for one column, and is used in the full update code.Limitations:
Improvements that can be done more easily:
Comments and suggestions are welcome! Hopefully these can be useful additions to the package.