Skip to content

Default tolerances for SVD-based nullspaces#172

Open
lkdvos wants to merge 2 commits intomainfrom
ld-null
Open

Default tolerances for SVD-based nullspaces#172
lkdvos wants to merge 2 commits intomainfrom
ld-null

Conversation

@lkdvos
Copy link
Member

@lkdvos lkdvos commented Feb 4, 2026

This PR changes the default notrunc to atol = defaulttol() for SVD-based nullspace computations.

Fixes #166.

@codecov
Copy link

codecov bot commented Feb 4, 2026

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/algorithms.jl 66.66% 1 Missing ⚠️
Files with missing lines Coverage Δ
src/interface/orthnull.jl 70.58% <100.00%> (ø)
src/algorithms.jl 89.78% <66.66%> (-0.59%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Yue-Zhengyuan
Copy link
Member

How about exactly following LinearAlgebra.nullspace to set a nonzero rtol instead?

@lkdvos
Copy link
Member Author

lkdvos commented Feb 5, 2026

Because I don't necessarily have access to the runtime information here, the algorithm selection procedure is running on types, so the whole scaling with the size of the input is a bit annoying. I can set both an absolute and relative tolerance if you like, but honestly I am not that convinced by this, it all feels incredibly arbitrary. For the truncated svd we also don't really provide a default truncation scheme, and I think here a similar thing is happening, by default the QR is chosen, and otherwise you just specify your own tolerances?

@Yue-Zhengyuan
Copy link
Member

If we don't have access to n (input size) then I'm fine with the current PR choice.

@lkdvos lkdvos requested a review from Jutho February 5, 2026 14:46
@Jutho
Copy link
Member

Jutho commented Feb 5, 2026

Isn't the fact that you don't have access to runtime information exactly a reason for setting rtol instead of atol? Ok the rtol value cannot include the size, but at least it still includes the norm of the tensor, which sounds more natural than just using atol.

@lkdvos
Copy link
Member Author

lkdvos commented Feb 5, 2026

I guess that is true, but I think what is weird about this to me is that the implication is slightly different.

I think what is bothering me is that for truncated decompositions, the natural notion of tolerances is something like the distance between U \cdot S \cdot Vh and \tilde{U} \cdot \tilde{S} \cdot \tilde{Vh}, or thus isapprox(USVh, USVhtrunc).

On the other hand, a nullspace seems like I want to use the distance between A \cdot Nh and 0, or in other words isapprox(norm(A * N'), 0).

What is annoying about this is that comparing to 0 doesn't really have a good meaning of relative tolerance, which is somehow different from the truncated SVD case?

Really what I want is the exact nullspace, and some tolerance about how precise my computations are that allows me to reasonably assume what is zero up to the tolerance of my computation.
In any case, I think I'm also nitpicking a bit about all of this, so it might be completely fine to use whatever default is most convenient, but this was at least my reasoning behind using atol = defaulttol(eltype), rather than trying a relative tolerance.

@Jutho
Copy link
Member

Jutho commented Feb 5, 2026

Ok I see what you mean. But still, if I rescale A, I would like to find the same nullspace, no? If A has large entries, but is not full rank, the smallest eigenvalues will not be machine precision (LAPACK might even scale A to have some norm being one before starting the svd calculation):

julia> A = randn(10,5); B = A*A'; [svd_vals(B) svd_vals(1000*B)]
10×2 Matrix{Float64}:
 24.7879       24787.9
 10.6469       10646.9
  9.05193       9051.93
  4.0803        4080.3
  2.28204       2282.04
  1.45695e-15      1.50662e-12
  8.17938e-16      7.77041e-13
  3.89231e-16      5.64928e-13
  2.58608e-16      2.41852e-13
  3.0498e-17       3.5025e-14

@lkdvos
Copy link
Member Author

lkdvos commented Feb 5, 2026

Yeah, that is also completely fair. So how do you feel about me changing it to atol = defaulttol(x), rtol = defaulttol(x), or do you want me to drop the absolute tolerance to avoid considering A / 10000 as zero?

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

Successfully merging this pull request may close these issues.

right_null for a simple matrix

3 participants