-
Notifications
You must be signed in to change notification settings - Fork 45
Compatibility with multifusion categories #297
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
base: main
Are you sure you want to change the base?
Conversation
…iteMPOHamiltonian`
… boris-MTK-compat
…nto boris-MTK-compat
…es for consistency" This reverts commit 2114cdc.
|
Wow it's been a while since I messed up so much with git 😅 |
…o boris-MTK-compat
|
I may have added some confusing terminology with the recent commits, so let me clarify as much as possible. First of all, I defined a Connected to the last remark, many functions construct an MPS given an MPO based on the latter's virtual spaces. Since the MPOs I've considered up till now don't have different colorings on its virtual spaces, there is currently no difference between the left unit and right unit of an MPO. Thus, there's no difference between the I've also replaced the remaining |
…o boris-MTK-compat
|
|
||
| V_left = left_virtualspace(H[1]) | ||
| V_left′ = ⊞(V_left, oneunit(V_left), oneunit(V_left)) | ||
| V_left′ = ⊞(V_left, rightunitspace(V_left), rightunitspace(V_left)) |
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.
Could this also be leftunitspace?
This looks a bit funny to have V_left call rightunitspace.
To be completely fair, I'm actually surprised it would have to be left or right to begin with, since I always understood that as something you would multiply on the left or right respectively, and not something you can direct sum with, so I would guess that in this case you would have to have \boxtimes is only possible wheneveer leftunitspace = rightunitspace which is just unitspace?
I might be completely wrong here though, this is a bit more intuition based than actual strong thinking based
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.
The Hamiltonian itself is diagonal, meaning its virtual spaces are graded the same way as the physical space. In that sense, taking the left or right unit space is the same. I haven't thought too much about direct summing between different gradings and whether that makes sense.
Either way, unitspace is not the way to go, because we've designed that now to return all the units of a given sector.
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.
Ah right, I forgot about that! Would this mean that we could take the left unit space of the physical space instead? That at least sounds spatially correct in my head 😂
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 can do that, and will do so in other places where MPOHamiltonian is involved. I'd like to add an assertion to make sure this is fine. However, for MPOs I'd like to keep the freedom of potential off-diagonal virtual spaces, even though I haven't tested those yet.
src/operators/mpohamiltonian.jl
Outdated
| # avoid using one(S) | ||
| somempo = local_mpos[1].second[1] | ||
| _rightunit = space(somempo, 1) # should be rightunitspace, but MPOHamiltonians are always diagonal for now |
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.
Would this be equal to leftunitspace(physicalspace)?
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.
Yes, in this case that does give the right space. I'd be tempted to put rightunitspace to emphasise the physical coloring, but if you're talking about the physical space anyway it's the same. I changed it to something cleaner now.
src/utility/utility.jl
Outdated
| """ | ||
| function add_util_leg(tensor::AbstractTensorMap{T, S, N1, N2}) where {T, S, N1, N2} | ||
| ou = oneunit(_firstspace(tensor)) | ||
| ou = unitspace(_firstspace(tensor)) |
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.
Can we replace this function with insertleftunit etc?
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'm looking at this and I think the additional function add_util_mpoleg is now redundant if I use insertleft/rightunit here correctly. I can't seem to remember why I had to split them up in the first place. Maybe a remnant of an older version of MPSKit 🤔
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 any chance these things can easily be interspersed with the other tests?
In any case, we should really clean up the tests of MPSKit at some point, so if this isn't too easy I'm fine keeping it like this and tackling it in the future
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 definitely easy to just copy-paste test sets to other related tests, but some test unique properties of the multifusion sectors themselves, and not necessarily the consistency of the algorithms dealing with this sector. I separated the multifusion tests since the other tests don't necessarily probe how the symmetric tensor data play a role. How do you think we should proceed?
|
Your PR no longer requires formatting changes. Thank you for your contribution! |
This PR provides the changes required to be able to consider the multifusion categories of (Multi)TensorKit.
All changes boil down to either
unitspaceof aBimoduleSector, orBimoduleSectorGradedSpaces,and finding ways around to this.
A potential thing to add is tests withBimoduleSectorMPS/MPOs or so? For this, I would need to first complete benchmarking MultiTensorKit, hence the draft PR. I might also need to format.