TESTING: scale the xGEEV eigenvalue-consistency check by n ulp |W| - #1401
Open
ACSimon33 wants to merge 1 commit into
Open
TESTING: scale the xGEEV eigenvalue-consistency check by n ulp |W|#1401ACSimon33 wants to merge 1 commit into
ACSimon33 wants to merge 1 commit into
Conversation
xGEEV does not promise identical eigenvalues whether or not eigenvectors are asked for: the two paths cover different index ranges, so rounding that depends on the trip count makes them differ. Test 5 now measures the difference against n |W| ulp, as tests 1 and 2 of the same driver already do, instead of requiring bit equality. Closes Reference-LAPACK#732 item 3.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1401 +/- ##
=======================================
Coverage 69.36% 69.37%
=======================================
Files 6122 6122
Lines 486337 486409 +72
Branches 23268 23268
=======================================
+ Hits 337330 337426 +96
+ Misses 148569 148545 -24
Partials 438 438
Continue to review full report in Codecov by Harness.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
xDRVEVandxGET23test 5 requirexGEEVto return bit-identical eigenvalues whether or not eigenvectors were asked for, setting the ratio to1/ulpon any difference at all. That is stronger than anything the routine promises: the two JOB values run different code paths, and nothing obliges those to round alike. This PR measures the difference againstn |W| ulpinstead, the same normalisation tests 1 and 2 of the same drivers already use.Closes #732.
Why bit equality is not available
xGEEVwithJOBVL = JOBVR = 'N'callsxHSEQRwithJOB = 'E', which is free to skip work the Schur form would need. InxLAHQR,xLAQR3andxLAQR5that shows up as narrower update ranges -I1,I2,JTOPandJBOTall depend onWANTT- so the two JOB values reach the same eigenvalues through a different sequence of BLAS calls. Nothing inxGEEV's specification promises that two such sequences round alike, and under LLVM Flang on aarch64 they do not.The divergence is FMA contraction inside the reference
ZGEMM, and it can be isolated to that one file. RecompilingBLAS/SRC/zgemm.falone, with the rest of the library untouched, clears every failing row:zgemm.fcompiled with.2dops-ffp-contract=fast1/ulp-ffp-contract=on-ffp-contract=off-fno-vectorizeWhat is not established is which individual
ZGEMMcall in the sweep is the first to differ.ZGEMMitself is deterministic. The two paths simply perform different sequences of fused arithmetic. The measured gap is 2 to 14.5 ulp, and every residual test passes, so nothing here suggests a wrong answer - only that the last bits are not reproducible across code paths.Note finally that any optimised BLAS is free to do the same, and a threaded one certainly will, so the test's premise does not hold for a real deployment either.
The fix
Each of the three comparisons in
xDRVEV, and the three inxGET23, becomeswith
ABS( WR )+ABS( WI )in the real drivers,SMLNUMforUNFLinxGET23, andDBLE( N )in double precision. IncludingWDIFin the denominator caps the ratio at1/(n ulp), so a genuinely wrong eigenvalue still reports about2e14and the test keeps its full power. The printed description inxDRVEVandxDRVVXchanges to match.Results
Max
ZEVratio over all 1102 cases ofzed.inon aarch64:Only test 5 moves, and only under flang, where
4.5e+15is the1/ulpsentinel the old code assigned on any difference at all. After the change it reads 0.725. Every other ratio is unchanged to three figures on both compilers, which is the point: nothing but the one over-strict comparison is affected.Note also that flang's residuals in tests 1 and 2 are better than gfortran's, 4.60 against 5.35 and 4.21 against 3.89 respectively, so the eigenvalues and eigenvectors flang computes are not in question. Only the demand that two code paths agree bit-for-bit was.
Notes
znep_64still reportsZHS: 1 out of 2016under ATfL 22.1. That isxCHKHStest 8, a different check, and it is left alone deliberately: it belongs with SEP test threshold #1350 rather than here.