Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Make sdiag_inv_sqrt use pseudo-inverse #78

Merged
merged 8 commits into from
Oct 29, 2024
Merged

Make sdiag_inv_sqrt use pseudo-inverse #78

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
6e1a32f
Add _safe_inv to sdiag_inv_sqrt and add field to ProjectorAlg
pbrehmer Oct 24, 2024
5c337d8
Merge branch 'master' into pb-inv-sqrt-tolerance
pbrehmer Oct 25, 2024
840cf5a
Add TensorKit-like rtol and revert inv_sqrt_tol changes
pbrehmer Oct 28, 2024
cd309f7
Resolve merge conflict
pbrehmer Oct 28, 2024
6ba61dc
Fix gauge-fix test
pbrehmer Oct 28, 2024
2bfcad6
Replace first S element with Inf-norm, update pwave tests
pbrehmer Oct 29, 2024
150abeb
Fix formatting
pbrehmer Oct 29, 2024
6bcb4e4
Add back nothing to gradients tests
pbrehmer Oct 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 14 additions & 5 deletions src/utility/util.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,33 @@ function _elementwise_mult(a::AbstractTensorMap, b::AbstractTensorMap)
end

# Compute √S⁻¹ for diagonal TensorMaps
function sdiag_inv_sqrt(S::AbstractTensorMap)
_safe_inv(a, tol) = abs(a) < tol ? zero(a) : inv(a)
function sdiag_inv_sqrt(S::AbstractTensorMap; tol::Real=eps(eltype(S))^(3 / 4))
tol *= norm(S, Inf) # Relative tol w.r.t. largest singular value (use norm(∘, Inf) to make differentiable)
invsq = similar(S)

if sectortype(S) == Trivial
copyto!(invsq.data, LinearAlgebra.diagm(LinearAlgebra.diag(S.data) .^ (-1 / 2)))
copyto!(
invsq.data,
LinearAlgebra.diagm(_safe_inv.(LinearAlgebra.diag(S.data), tol) .^ (1 / 2)),
)
else
for (k, b) in blocks(S)
copyto!(
blocks(invsq)[k], LinearAlgebra.diagm(LinearAlgebra.diag(b) .^ (-1 / 2))
blocks(invsq)[k],
LinearAlgebra.diagm(_safe_inv.(LinearAlgebra.diag(b), tol) .^ (1 / 2)),
)
end
end

return invsq
end

function ChainRulesCore.rrule(::typeof(sdiag_inv_sqrt), S::AbstractTensorMap)
invsq = sdiag_inv_sqrt(S)
function ChainRulesCore.rrule(
::typeof(sdiag_inv_sqrt), S::AbstractTensorMap; tol::Real=eps(eltype(S))^(3 / 4)
)
tol *= norm(S, Inf)
invsq = sdiag_inv_sqrt(S; tol)
function sdiag_inv_sqrt_pullback(c̄)
return (ChainRulesCore.NoTangent(), -1 / 2 * _elementwise_mult(c̄, invsq'^3))
end
Expand Down
9 changes: 2 additions & 7 deletions test/ctmrg/gradients.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,7 @@ names = ["Heisenberg", "p-wave superconductor"]

gradtol = 1e-4
boundary_algs = [
CTMRG(;
tol=1e-10,
verbosity=0,
ctmrgscheme=:simultaneous,
svd_alg=SVDAdjoint(; fwd_alg=TensorKit.SVD(), rrule_alg=GMRES(; tol=1e-10)),
),
CTMRG(; tol=1e-10, verbosity=0, ctmrgscheme=:simultaneous),
CTMRG(; tol=1e-10, verbosity=0, ctmrgscheme=:sequential),
]
gradmodes = [
Expand All @@ -36,7 +31,7 @@ gradmodes = [
LinSolver(; solver=KrylovKit.GMRES(; tol=gradtol), iterscheme=:fixed),
LinSolver(; solver=KrylovKit.GMRES(; tol=gradtol), iterscheme=:diffgauge),
],
[
[ # Only use :diffgauge due to high gauge-sensitivity (perhaps due to small χenv?)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you re check this after the gaugefixing fixes?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay I checked again and the gauge-fixing fixes don't make a difference to the high gauge-sensitivity. I will have to investigate and fix that in a different PR. I just haven't played around that much with fermions yet.

I put back in nothing to the test, since I accidentally left that commented out as I was trying around.

nothing,
GeomSum(; tol=gradtol, iterscheme=:diffgauge),
ManualIter(; tol=gradtol, iterscheme=:diffgauge),
Expand Down
8 changes: 7 additions & 1 deletion test/pwave.jl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@ unitcell = (2, 2)
H = pwave_superconductor(InfiniteSquare(unitcell...))
χbond = 2
χenv = 16
ctm_alg = CTMRG(; tol=1e-8, maxiter=150, verbosity=2, ctmrgscheme=:sequential)
ctm_alg = CTMRG(;
tol=1e-8,
maxiter=150,
verbosity=2,
ctmrgscheme=:simultaneous,
svd_alg=SVDAdjoint(; rrule_alg=Arnoldi(; tol=1e-9, krylovdim=χenv + 30)),
)
Comment on lines +13 to +19
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at the risk of sounding like a broken record, I really don't like that we have to change the settings of all algorithms on every single update of the package. We really have to get some better defaults in, because this is not really feasible in the long run. (not something that has to be fixed in this pr)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You definitely have a point in repeating that and I completely agree :-) Actually, for most of the tests we should already be able to replace all settings with the default ones I just haven't gotten around to change them. I just thought that I would wait until we anyway redesign our interface and code up some kind of default parameter selection thing.

opt_alg = PEPSOptimize(;
boundary_alg=ctm_alg,
optimizer=LBFGS(4; maxiter=10, gradtol=1e-3, verbosity=2),
Expand Down