IDMRG updates #294
Merged
IDMRG updates #294
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the IDMRG implementations to support multi-leg MPS, unifies edge-handling logic, introduces a new AC2 utility across MPS types, and fixes environment normalization to prevent exponential norm growth.
- Remove hardcoded
@plansorcalls and unify edge updates infind_groundstateandapproximate! - Add a generic
AC2two-site tensor accessor forAbstractMPSand concrete MPS types - Change environment normalization to always scale the right env to norm 1 and update tests for the new convention
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/algorithms.jl | Updated leading_boundary tests with dynamic bond dims and IDMRG2 case |
| src/states/multilinemps.jl | Added AC2 methods for MultilineMPS |
| src/states/infinitemps.jl | Added AC2 for InfiniteMPS |
| src/states/finitemps.jl | Added AC2 for FiniteMPS |
| src/states/abstractmps.jl | Documented AC2 on AbstractMPS |
| src/environments/infinite_envs.jl | Updated normalize! to avoid norm blow-up |
| src/algorithms/statmech/idmrg.jl | Implemented leading_boundary for IDMRG and IDMRG2 with unified edge logic |
| src/algorithms/derivatives/derivatives.jl | Extended projection macros to pass kwargs to AC2 |
| src/algorithms/approximate/idmrg.jl | Refactored approximate! sweeps to use AC2 |
| src/MPSKit.jl | Included the new statmech idmrg.jl |
Comments suppressed due to low confidence (1)
src/environments/infinite_envs.jl:110
- The new normalization logic in
normalize!(right envs to norm 1, left envs to overlap 1) isn't covered by existing tests. Consider adding unit tests, including cases with negative overlaps, to ensure both branches behave as intended.
normalize!(envs.GRs[i])
Codecov ReportAttention: Patch coverage is
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
Member
VictorVanthilt
left a comment
VictorVanthilt
left a comment
There was a problem hiding this comment.
Should we write some extra tests to test the handling of the edges of find_groundstate and approximate?
Member
Author
|
Yeah probably some checks are a good idea, especially given the reported issues in #292, which I still need to further delve into. |
lkdvos
force-pushed
the
ac2
branch
2 times, most recently
from
b69a3c5 to
7606dd8
Compare
This was a mistake, in `approximate` the top mps doesn't change so there is no need to update the gauges
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR refactors a bunch of the IDMRG implementations.
In particular, I've removed all hardcoded
@plansorcalls to make sure that the implementations are also working for MPS with multiple legs.Additionally, somehow the edge handling for the
find_groundstateandapproximateimplementations was very different.This is now all the same, although I'm not entirely sure if this is correct, and in the future we should really look into de-duplicating this code, similar to what was done for VUMPS/VOMPS.
I've also added
leading_boundaryimplementations to allow the PEPS algorithms to function.Finally, I've fixed the following issue when working with IDMRG environments for statmech systems: because the expectation value scales multiplicative with system size, we have to renormalize the environments to ensure the values don't blow up. Previously, we did this in an asymmetric manner, absorbing the norm in the left environment. This was mostly to avoid taking square roots of negative real values, which would lead to complex environments whenever we split the norm equally over left and right.
However, this leads to a different problem: the norm of the right environment grows exponentially, while the left environment shrinks exponentially to ensure their product remains one. At some point, LAPACK complains and starts failing.
I've changed this to now always normalize the right environment to 1, and the left environment as before to ensure the inner product is 1.