[WIP] Testsuite

[lkdvos]

This is an attempt to tackle #91 .

The main goal is to provide some way to reduce the amount of code duplication in our tests here, but additionally to think about how this might affect downstream adopters of this package.

I've set up the QR tests to give an idea of what I have in mind, feedback greatly appreciated!

[codecov]

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/implementations/qr.jl	0.00%	2 Missing ⚠️

Files with missing lines	Coverage Δ
src/implementations/qr.jl	71.77% <0.00%> (-24.85%)	⬇️

... and 16 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Jutho]

With the LAPACK methods, there is some interplay between pivoted and blocksize, but I guess the new tests do not test them simultaneously so everything is fine.

[lkdvos]

I was considering adding some dedicated LAPACK methods to test these very specific behaviors, so have a large testsuite to do basic testing, but then add additional tests where necessary.

[kshyatt]

I think this should be relatively easy to make work with the GPU code, but we might want to break up some of the files into multiple functions that implement subsets of the tests to avoid scalar indexing/unsupported operations in some cases.

[github-actions]

Your PR no longer requires formatting changes. Thank you for your contribution!

[Jutho]

Suggested change

	T = eltype(A)
	return if T <: Real
	all(≥(zero(T)), diagview(A))
	else
	all(≥(zero(real(T))), real(diagview(A))) &&
	all(≈(zero(real(T))), imag(diagview(A)))
	end
	all(x -> x ≈ abs(x), diagview(A))

[lkdvos]

We can do this, but it does change the tests, -eps() would have this property but it wouldn't have been properly "positive". sign(x) == 1 might work but I'm not sure how accurate that is

[Jutho]

julia> eps() ≈ -eps()
false

[lkdvos]

Fair enough, but how about:

julia> 1.0 + 1e-12im ≈ abs(1.0 + 1e-12im)
true

😉

Jokes aside, I think our current implementation is really such that the imaginary part is hard zero and the real part is hard positive, I don't have a strong opinion about whether or not this is a requirement either (i.e. are we assuming that we can always make this exactly true for weird Number types, or is it sometimes only approximately positive?), so I'll just change this :)

[Jutho]

Well indeed, I also think we currently require strict positivity, but I was confused by all(≈(zero(real(T))), imag(diagview(A))). Is this equivalent to all(iszero, imag(diagview(A))). If you want the latter, then clearly we want x == abs(x).

[Jutho]

or simply all(isposdef, diagview(A)) (but this requires LinearAlgebra.isposdef)

[kshyatt]

eigh_trunc! tests are failing, I'll look into why

Edit: Diagonal strikes again

[kshyatt]

I think the only factorization left to cover is schur, and then of course the AD tests