TESTING/EIG/csd.in: please add M=P=Q=1 (and small degenerate cases) — exposes a [CZ]UNCSD2BY1 bug already fixed between 3.12.1 and current

[nakatamaho]

Summary

The shipped TESTING/EIG/csd.in does not exercise the trivial M=P=Q=1 corner case. Adding it makes xeigtstz on LAPACK 3.12.1 report 6 failing test ratios on M=55, P=40, Q=20 (2-by-1 CSD, tests 10 and 11). The same modified input file passes cleanly on current master, and also passed on 3.9.1. So a bug was introduced after 3.9.1, was present in 3.12.1, and has since been fixed somewhere on master — but because the released test data never covered this seed sequence, the regression slipped through release testing for multiple versions.

I would like to request that the corresponding small/degenerate cases be added to csd.in upstream so this class of regression is caught by make check in future releases.

Reproducer

Diff against shipped TESTING/EIG/csd.in:

-10                                   Number of values of M, P, Q
-0  10 10 10 10 21 24 30 22 32 55     Values of M
-0  4   4 0  10 9  10 20 12 12 40     Values of P
-0  0  10 4  4  15 12 8  20 8  20     Values of Q
+12                                   Number of values of M, P, Q
+0 1  10 10 10 10 21 24 30 22 32 55     Values of M
+0 1  4   4 0  10 9  10 20 12 12 40     Values of P
+0 1  0  10 4  4  15 12 8  20 8  20     Values of Q

Run ./xeigtstz < csd.in.

LAPACK 3.12.1 (FAIL)

M= 55 P= 40, Q= 20, type 1, test 10, ratio= 0.152394E+15
M= 55 P= 40, Q= 20, type 1, test 11, ratio= 0.496574E+15
M= 55 P= 40, Q= 20, type 2, test 10, ratio= 0.324446E+12
M= 55 P= 40, Q= 20, type 2, test 11, ratio= 0.466353E+12
M= 55 P= 40, Q= 20, type 3, test 10, ratio= 0.443792E+15
M= 55 P= 40, Q= 20, type 3, test 11, ratio= 0.663765E+15
CSD:    6 out of  720 tests failed to pass the threshold

LAPACK current master (PASS)

All tests for CSD routines passed the threshold (    720 tests run)

LAPACK 3.9.1

Also passed (no failures with the modified csd.in).

Observations

• Failures are confined to [CZ]UNCSD2BY1 (2-by-1 CSD), tests 10 and 11. The 2-by-2 CSD tests (1–9) are unaffected.
• The failing configuration is always (M,P,Q) = (55,40,20), all three matrix types.
• The shipped csd.in already contains M=P=Q=0, but not M=P=Q=1. Inserting the trivial 1,1,1 problem advances the random seed and exposes the bug downstream — i.e. the bug is real and seed-sensitive, not an artifact of the 1,1,1 invocation itself.
• Since the bug is fixed on master but the regression existed across at least 3.10–3.12.1, the test data clearly needs strengthening.

Request

Please extend TESTING/EIG/csd.in to include the small degenerate cases, at minimum M=P=Q=1, and ideally also (M,P,Q) = (1,1,0) and (1,0,1), so that future regressions in [CZ]UNCSD2BY1 for this code path are caught by the standard test suite.

Question

Would the maintainers like me to open a pull request with the csd.in extension directly, or do you prefer to handle the test-data update in-house? If a PR is welcome, I am happy to also bisect master to identify the exact commit that fixed [CZ]UNCSD2BY1, so the fix can be referenced in the commit message for the test-data addition.

[martin-frbg]

I notice that you had to update the count to 12, although you added only one number - is it the advancing of the seed, or just that the final M,P,Q were never tested ?

[nakatamaho]

Good catch — let me clarify what happened.

Counting the original values: 0 10 10 10 10 21 24 30 22 32 55 — that's 11 entries including the leading 0, but the header said 10. So in the original file, the last case (M=55, P=40, Q=20) was silently dropped and never actually tested due to an off-by-one in the header.

Compare this with other input files, e.g. cbb.in:

20                Number of values of M
0 0 0 0 1 1 1 1 2 2 2 2 3 3 3 3 10 10 16 16    Values of M

Here the header 20 matches the actual element count exactly. Following that convention, the header in gbr.in should have been 11 to begin with.

So my diff is really doing two things at once:

1. Fixing the original header from 10 to the correct count 11, which "revives" the last M=55, P=40, Q=20 case that was previously skipped.
2. Adding one new case on top of that, bringing the total to 12.

It's not about advancing the RNG seed or intentionally trimming the tail — it's just that the original header was off by one, and the final triplet was effectively dead data. I folded the fix and the new case into the same patch.

[martin-frbg]

Right, ISTR there are/were a couple of tests where the highest triplets did not get used by default - I assume these were simply too big to run regularly on the hardware of ~25 years ago. The other aspect is that my (limited) understanding is that some seeds may create poorer quality inputs - sometimes leaving out intermediate matrix sizes to focus on a particular failure makes that test suddenly pass (and/or others after it fail). (Not sure if that applies to this specific test though)

[martin-frbg]

Quick question - is the addition of the M=P=Q=1 test case still desirable after the recent correction of the test count by your #1232 (and assuming that test gets added after the last one, so as not to upset the random number sequence and its relation to precalculated expectation values) ?