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.

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

About the warning for svd AD #168

Open
Confusio opened this issue Oct 31, 2024 · 7 comments
Open

About the warning for svd AD #168

Confusio opened this issue Oct 31, 2024 · 7 comments

Comments

@Confusio
Copy link

For the SVD AD, there is a warning:

Warning: "svd cotangents sensitive to gauge choice: (|Δgauge| = 1.2888961545189787e-16)"

Is this warning related to the autodifferentiation of the SVD for complex numbers? If we are only concerned with gauge-invariant results, this warning should not pose any issues. Is that correct?

@Jutho
Copy link
Owner

Jutho commented Oct 31, 2024

Yes it comes from the AD applied to the SVD; it tells you that in the backward process, there is component entering the adjoint Jacobian of the SVD process that depends on the gauge. However, that component is only 1.28e-16 in size, so it effectively zero. Therefore, I am surprised that the warning is even printed. What did you set for the tol keyword in the primal algorithm?

@Confusio
Copy link
Author

I used the default settings without setting any tol. I am also surprised about the "warning" with the tiny value.

@Jutho
Copy link
Owner

Jutho commented Oct 31, 2024

I was a bit confused; this is coming from the full svd in TensorKit, right? I had KrylovKit in mind. In TensorKit, the current cutoff for printing this warning if no defaults are specified is: eps(eltype(S))^(3 / 4) * S[1, 1] with S[1,1] the largest singular value. I guess that is not a really good default: when S[1,1] is for some reason small, it should still not be expected that the tolerance gets of the order or smaller than machine precision. I will change the default to something like eps(eltype(S))^(3 / 4) * max(S[1,1], one(S[1,1]).

@Confusio
Copy link
Author

Yes, it is from full tsvd in TensorKit.

@Jutho
Copy link
Owner

Jutho commented Nov 1, 2024

Ok, this is now changed on master, hopefully this fixes it. I will tag a new release this weekend, but there are some breaking changes.

@Confusio
Copy link
Author

The new tol may cause a problem in safe_inv.(Sp' .- Sp, tol)'. Here we need a relative tol. Otherwise, it causes an incorrect result when Sp` is small.

@Jutho
Copy link
Owner

Jutho commented Dec 8, 2024

I agree that tol is perhaps a bit overused in svd_rrule!, although it did seem consistent. However, I am open to better suggestions. Currently, tol is used to decide on a number of different things:

  1. if two singular values S[i] and S[j] are closer in distance than tol, they are considered the same; in safe_inv the inverse inv(S[i]-S[j]) will be replaced with zero.

  2. if a singular value S[i] is smaller than tol, it is assumed equal to zero and is truncated away (note that an external truncation by an other criterion may have happened also; that is compatible with the implementation; the current truncation just happens on top of that). I would argue that this is consistent with 1).

  3. The rule for ignoring inv(S[i]-S[j]) in 1) if they are the same comes is motivated by the assumption that the associated numerator (namely the (ij) entry in the antisymmetric part of U'*ΔU and V'*ΔV') is zero. This is tested for with Δgauge. Testing for zero currently again amounts to imposing that these entries are smaller than tol` (in absolute value). I think there is also some rationale for this choice, in that if the numerator is just slightly larger than tol, but the denominator also, then an order 1 contribution is obtained.

I am happy to consider other choices or implement suggestions to improve this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants