Add CTMRG support for regular 2D partition functions #111
Conversation
There was a problem hiding this comment.
Starting to look good!
I left some remarks alreayd, but some general remarks here:
For the docstrings, I don't think, it makes sense to have separate docstrings for both the partition function and PEPS (and eventually PEPO) implementations. Rather, we could use A in the figure, and then specify that A might be either bra-ket, partition function or bra-pepo-ket combinations. As a result, we should probably also agree on a style of drawings (let's do this in a separate PR) that uses the same character for drawing tensor connections, and maybe either never draw boxes around tensors or always draw boxes.
(I am assuming that you left some of the docstrings unattached to functions on purposes as this is still a WIP port?)
For naming things, InfinitePartitionFunction seems like the best we can do, since I really cannot come up with anything better. Given that it is rather long, how would you feel about InfinitePF, similar to InfinitePEPS? (Along with PFTensor etc)
src/algorithms/toolbox.jl
Outdated
| @@ -57,6 +57,46 @@ function LinearAlgebra.norm(peps::InfinitePEPS, env::CTMRGEnv) | |||
| return total | |||
| end | |||
|
|
|||
| function LinearAlgebra.norm(partfunc::InfinitePartitionFunction, env::CTMRGEnv) | |||
There was a problem hiding this comment.
I'm not really convinced that this should overload norm for this purpose, given that it's not really a state, and there is no overlap or conj etc...
There was a problem hiding this comment.
Fair. Any other name would be fine for me as well, but I'm having a bit of trouble thinking of a suitable one. Just value might be the most 'correct' since the network just represents a number in the end?
There was a problem hiding this comment.
I settled on value for now and added a corresponding export, let me know if you prefer something else.
There was a problem hiding this comment.
Well, this didn't quite work as expected since norm is called inside the CTMRG leading_boundary to give the current PEPS norm in the logging output. For one, it seems a bit wasteful to do the norm contraction every iteration regardless of the logging verbosity. It would probably be good to make the computation conditional on the logging level, no?
While I can then make the partition function tests pass by setting verbosity=0, this is clearly not a fix. So we should probably come up with a name that makes sense for any network type and use that in the CTMRG logging. Does value still make sense in the more general case?
There was a problem hiding this comment.
I think the cleaner way is to redefine the ctmrg_log... functions to take in the state instead, and specialize on what kind it is to determine what should be printed? (along with hiding this behind a verbosity check to avoid unnecessary computations)
There was a problem hiding this comment.
I agree, but to do this I would need to be able to access the current verbosity in from the ctmrg_log... functions inside the withlevel block, and I don't immediately see how to do that. I could spend some more time, but since I'm getting warnings that the verbosity macros from LoggingExtras.jl have been deprecated maybe it would be better to reconsider the logging more thoroughly anyway?
To patch this for now I added a network-dependent ctmrg_objective and made its computation verbosity dependent. I'm very annoyed at the explicit verbosity level integers inside the leading_boundary body now, but I don't immediately see a way around this unless I can manage to figure out the above issue with accessing the verbosity.
Do you think this is sufficient for this PR where we address the remaining issues in a follow-up?
Sounds good, I'll try to standardize as much as I can here. I agree we should be more systematic about the diagrams, I'm happy to put some effort into that soon.
I don't really like the 'PF' acronym, for the simple reason that no one really uses it so there's no way of knowing what it means without actually reading the docstring. This is quite different for 'PEPS' I think. I'd be happy to use the shorter names as internal aliases, but I wouldn't like to export |
Add internal convenience aliases for different flavors of edge tensors Co-authored-by: Lukas Devos <[email protected]>
|
Thanks a lot for the effort! To me the PR mostly looks quite nice already. On the topic of naming things: I am a bit confused by calling the Also: let me know if I can contribute or finish up something! I anyway wanted to add 2D partition function support at some point so I'm happy to help. |
I agree using 'partition function' might imply some additional physical properties rather than just a network structure, which is really only what we mean here. The reason we didn't just reuse I would personally be okay with using the term 'partition function' for things with no stat mech origin, or at least more than I would be with using 'MPO' for 2D network structures. We could also go for |
|
Well, in principle partition functions could also be a 1D or 3D thing, right? So I feel a bit icky about using I do think it could make sense to use the |
|
To be fair, matrix product operator really does not make too much sense either in the 2D grid case... |
|
I think my preference for the drawings is unicode lines, without arrows. In order to add the missing information about which spaces are domain-codomain, we would then have to specify this in the docs of the individual tensor types, and dedicate a page in the docs for this |
test/ctmrg/partition_function.jl
Outdated
| function local_contraction( | ||
| O::AbstractTensorMap{S,2,2}, env::CTMRGEnv{C,<:CTMRG_PF_EdgeTensor} | ||
| ) where {S,C} | ||
| return @autoopt @tensor env.corners[NORTHWEST, 1, 1][C_WNW; C_NNW] * | ||
| env.edges[NORTH, 1, 1][C_NNW D_N; C_NNE] * | ||
| env.corners[NORTHEAST, 1, 1][C_NNE; C_ENE] * | ||
| env.edges[EAST, 1, 1][C_ENE D_E; C_ESE] * | ||
| env.corners[SOUTHEAST, 1, 1][C_ESE; C_SSE] * | ||
| env.edges[SOUTH, 1, 1][C_SSE D_S; C_SSW] * | ||
| env.corners[SOUTHWEST, 1, 1][C_SSW; C_WSW] * | ||
| env.edges[WEST, 1, 1][C_WSW D_W; C_WNW] * | ||
| O[D_W D_S; D_N D_E] | ||
| end |
There was a problem hiding this comment.
Just a small detail, but in order for @autoopt to work here, the index labels need to follow our convention: the environment indices have to start with χ.
Also, instead of defining this contraction manually, it would probably pay of to adapt contract_localnorm to work for InfinitePartitionFunctions. That would make it much easier to work with unit cells or more complicated operators.
I can get that in since I'm anyway going the through the code right now.
There was a problem hiding this comment.
I was just in the middle of generalizing and adding this to src. Didn't know about the @autoopt convention, thanks for that!
I added a generic contraction for a single local tensor (AbstractTensorMap{S,2,2}) at a given site in a partition function. Unit cells should be fine, more complicated operators not at this point. It's not immediately clear to me how to do this, since the 'operator' here is contracted into some set of virtual indices instead of being applied to physical indices. For the simplest case it's obvious what this means, but for more complicated cases you would need some conventions for the index ordering on the bigger local tensor.
There was a problem hiding this comment.
Well, the operator in principle is just a InfinitePartitionFunction again, right? Just looking at the contraction, local_contraction is the same as contract_localnorm. The only difference is the object that is contracted inside the environment. So in my head, the user should construct an operator as a InfinitePartitionFunction and then just call expectation_value which only uses contract_localnorm under the hood.
There was a problem hiding this comment.
I don't think I agree with interpreting O as an InfinitePartitionFunction, it's just a single different local tensor surrounded by some given partition function on all sides. I don't pass the surrounding partition function here since it's not needed in the contraction, but it is really inserting a single (potentially) different O tensor at a given site surrounded by the original partition function. In other words, the environment given is not necessarily an environment of a network of all Os, so I passing the operator as an InfinitePartitionFunction does not make sense I think. It's quite different from the local norm in spirit even though the contraction is the same. Does that make sense?
There was a problem hiding this comment.
Yes that does make sense to me. My point was more that given a unit cell of AbstractTensorMap{S,2,2} operators (which corresponds to our InfinitePartitionFunction) and some environment, the correct contraction to obtain the expectation value can be performed only using contract_localnorm. I just wanted to reuse that structure since we already have that. Making a special case for a single operator via local_contraction perhaps seems too specific given that we already have all this cool generic machinery that should be easy to adapt to partition functions.
Maybe we could make a method for expectation_value that dispatches on Matrix{AbstractTensorMap{S,2,2}} and even a special method that dispatches on a single AbstractTensorMap{S,2,2} operator and then internally use contract_localnorm?
There was a problem hiding this comment.
Just adding on to this: replacing a single tensor is fundamentally different from InfinitePartitionFunction, since that one repeats the unitcell over the entire grid, while here you really want to change a single site in the entire grid.
I'm definitely happy with sharing some of the contraction code between the implementation of contract_localnorm, but this should probably be decoupled from the actual API functions, since the interpretation really is different.
There was a problem hiding this comment.
As far as API goes, expectation_value calling contract_local_tensor sounds fairly 'correct' to me in the current implementation, given that it's really contracting in a single different local tensor. If we want more general 'expectation values', I agree that it would be nice if we were able to reuse the implementation of contract_localnorm. However:
- To be able to reuse the actual code, we would need to split out the part of
contract_localnormthat handles indices and conjugates for the edges and the 'effective MPO part' of the contraction inside the environment. This is basically everything besides the corners. While reusing the machinery in spirit is of course what we want, this will still require separate dedicated code to handle contraction expressions for different types of network stacks, right? Of course this is not a real issue at all, except that - I really don't know how useful an
expectation_valueforMatrix{AbstractTensorMap{S,2,2}}actually is. Usually there's two purposes for 'local' contractions in partition functions, namely really local expectation values or correlation functions. If it's a simple local expectation value, it's a singleAbstractTensorMap{S,2,2}being contracted which is currently covered. If it's a correlation function, this generically involves two tensor which each have an extra index being contracted across the pair (when imposing symmetries, this is almost always the case). Even for more involved local observables, this would usually involve a single higher-rank tensor (e.g. a plaquette observable) rather than a collection ofAbstractTensorMap{S,2,2}s. None of these really require or can even be written in terms of contracting a genericMatrix{AbstractTensorMap{S,2,2}}inside a given environment.
In summary, while I would be fine with adding support for expectation_values of Matrix{AbstractTensorMap{S,2,2}} and then reuse the existing machinery to implement this, I'm really not sure this is what we actually need.
There was a problem hiding this comment.
Alright, I wasn't sure if there is some use to the Matrix{AbstractTensorMap{S,2,2}} contraction since I've not yet used 2D partition function PEPS much. And of course I agree that the interpretation of inserting a single operator is different from InfinitePF. Then let's keep it the way it is right now.
But if I understood correctly we can't yet contract correlation functions or operator strings with contract_local_tensor, right? In that case we would need a version of contract_localnorm (with adjusted edge/MPO indices) to expand the environment in the corresponding directions.
There was a problem hiding this comment.
No, contract_local_tensor doesn't support correlation functions,. For that case we would definitely need the logic of contract_localnorm, but adapted for MPO-style indices and with an extra contraction for the additional stringy indices. All very doable, but a bit tricky so I was trying to procrastinate this until someone explicitly needed it :)
There was a problem hiding this comment.
Okay I'll give it a try :)
|
The I'll finish up the rest tomorrow :-) |
|
There is a design decision to be made if we want to generalize our CTMRG code to also support different ket and bras, as well as PEPOs, and I'm not sure what the best way forward is. Right now, all CTMRG contractions support different The problem is that there is no |
I think it makes sense to have all the lowest level methods for things like
I don't know if having these as actual types is necessary, it might be but it's not clear to me. Just a union type of tuples for all different possible sandwiches would do the job to start. I certainly don't think it makes sense to make these potential full sandwich types subtypes of
I'm a bit biased of course since I want to start using this as soon as I can, but purely in terms of compartmentalizing features I think it would make sense to just restrict this PR to working partition functions. Just to keep additions from growing bigger and bigger I would break out additional discussions as issues or follow up PRs. Of course I'm not in that much of a rush, but it seems we're moving away from the initial purpose a bit already. |
|
I'm certainly not against that. I know we previously have done so, and called it PEPSSandwich, and PEPOSandwich. I do quite like those names 😉 [edit] |
I have also considered this but the problem with tuples as sandwiches is that you need to be able to index the sandwich. But I do agree that it doesn't make sense to subtype the sandwiches as Anyways, I agree that we're putting too many additions into this PR and should probably generalize the CTMRG interface in another PR - I will open up an issue! I will finish up the contraction business and then the PR is good to go for me. |
|
Alright, I'd be fine with merging this as is :-) (once the lights turn green) |
Rebased from @Sander-De-Meyer's initial work here.
Some todos left:
W ⊗ S ← N ⊗ Eand document