Conversation
…l-valued C4v Heisenberg test
…tor` and add QR-CTMRG optimization tests
Codecov Report✅ All modified and coverable lines are covered by tests.
... and 6 files with indirect coverage changes 🚀 New features to boost your workflow:
|
|
After the build completes, the updated documentation will be available here |
Yue-Zhengyuan
left a comment
There was a problem hiding this comment.
Minor comments on the example.
docs/src/examples/c4v_ctmrg/index.md
Outdated
| [ Info: LBFGS: iter 20, Δt 250.4 ms: f = -6.602282359103e-01, ‖∇f‖ = 1.2155e-03, α = 1.00e+00, m = 19, nfg = 1 | ||
| [ Info: LBFGS: iter 21, Δt 126.2 ms: f = -6.602299515427e-01, ‖∇f‖ = 1.0743e-03, α = 1.00e+00, m = 20, nfg = 1 | ||
| [ Info: LBFGS: iter 22, Δt 253.0 ms: f = -6.602310402232e-01, ‖∇f‖ = 4.7766e-04, α = 1.00e+00, m = 20, nfg = 1 | ||
| [ Info: LBFGS: converged after 23 iterations and time 2.06 s: f = -6.602310919804e-01, ‖∇f‖ = 5.4688e-05 |
There was a problem hiding this comment.
Honestly, QR-CTMRG performance is quite mundane in the example...
There was a problem hiding this comment.
Very true, so I didn't want to highlight this here. I think the reason is that at
Co-authored-by: Yue Zhengyuan <yuezy1997@icloud.com>
leburgel
left a comment
There was a problem hiding this comment.
The example itself looks great, I just have a few comments on the src changes. If you prefer merging the example first and then addressing these separately that would be fine by me. Having the example in is the most important, but then we should sort these out immediately after.
| # TODO: what's a meaningful way to compute a truncation error/condition number in this scheme? | ||
| return Q, (; Q, R, truncation_error = zero(scalartype(Q)), condition_number = 0) |
There was a problem hiding this comment.
I think this kind of exposes how the fixedpoint workflow still very much assumes we're always optimizing using asymmetric CTMRG, as we just hardcode the info this would give like here:
PEPSKit.jl/src/algorithms/optimization/peps_optimization.jl
Lines 243 to 244 in b56a5fb
Since there's other contraction routines that don't give a meaningful truncation error or condition number (boundary MPS also doesn't have these metrics for example), I'm wondering if it might be better to go the other way. So rather than adding a trivial truncation error and condition number here, we could instead make less assumptions on what contraction algorithm we're using when building the info returned by fixedpoint. Could we reserve one field in the info tuple for "contraction-algorithm-specific metric", and then set these depending on the algorithm without assuming everything returns the same info?
Otherwise, I think removing the truncation error and condition number from the fixedpoint info might still be better than adding dummy values here. So only return info that does not assume a specific contraction algorithm.
There was a problem hiding this comment.
f you prefer merging the example first and then addressing these separately that would be fine by me.
I think first addressing these issues makes more sense since the example relies on these changes.
Since there's other contraction routines that don't give a meaningful truncation error or condition number (boundary MPS also doesn't have these metrics for example), I'm wondering if it might be better to go the other way.
That's a good point. In any case it's better to keep the info tuple as generic as possible so I would be in favor of not assuming the existence of a truncation error and condition number. I like having a variable inside the info tuple that is reserved to contraction-algorithm-specific data. I will try to cook something up and maybe we can merge that first.
| # :fixed mode compatibility (C4v CTMRG with real numbers supports :fixed mode) | ||
| if !isnothing(alg.gradient_alg) && iterscheme(alg.gradient_alg) == :fixed && !(alg.boundary_alg isa C4vCTMRG) | ||
| if scalartype(env₀) <: Real # incompatible with real environments | ||
| env₀ = complex(env₀) | ||
| @warn "the provided real environment was converted to a complex environment \ |
There was a problem hiding this comment.
I think this might be better handled by a dedicated check_input method that dispatches on the algorithms, rather than checking explicit algorithm types here. Just because it's a bit more extensible that way.
We now already have a check_input for leading_boundary that was added rather quickly. I think we could generalize that to check_input(::typeof(leading_boundary), args...) and then use a check_input(::typeof(fixedpoint, args...) here, similar to how MatrixAlgebraKit.check_input is used (I assume using the same name is not really a problem since the method is not public).
Also, I would much prefer throwing an error with the same message instead of changing the scalartype of the environment on the go and only giving a warning. The error message says exactly what to do and why, so I don't think it would be a problem to just error out and let the user fix their configuration.
There was a problem hiding this comment.
Indeed, a check_input method does seem like the cleaner solution. I can also try to take care of that in a separate PR. Also, I agree that throwing an error instead of warning and then changing the scalartype seems more reasonable... I'm not sure why we opted for a warning to begin with.
Co-authored-by: Lukas Devos <ldevos98@gmail.com>
Here I add a small example where we demonstrate how to work with C4v-symmetric PEPS and environments and what to look out for when contracting and optimizing with C4v CTMRG. I also added a section on QR-CTMRG.
In order to make C4v optimization work with real numbers I needed to modify a small check in
fixedpoint(which wrongly stopped us from using real numbers with:fixedmode differentiation for C4v CTMRG). Also, I had to introduce a dummy truncation error and condition number to the returninfotuple inc4v_projector!to make QR-CTMRG work withfixedpoint. For both modifications I added tests. Maybe there is also a meaningful way to compute a truncation error/condition number for QR-CTMRG?