- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 35
 
          Use BLAS.trsm! instead of LAPACK.trtrs! in left-triangular solves
          #1194
        
          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
Conversation
Co-authored-by: Alexis Montoison <[email protected]>
| 
           @nanosoldier   | 
    
| 
           In a manual 1000x1000 test, I see slight to no improvement, but certainly no regression.  | 
    
| 
           It turns out that the LAPACK function is calling the BLAS function internally (https://netlib.org/lapack/explore-html/d4/dc1/group__trtrs_gab0b6a7438a7eb98fe2ab28e6c4d84b21.html), but checks for exact singularity upfront, nothing else. It seems we could rather switch to the "augmented" LAPACK functions throughout instead?  | 
    
| 
           Nevermind, there is no triangular right-solve in LAPACK.  | 
    
| 
           @dkarrasch I was thinking of LBT if we switch the backend, like BLIS. Sometimes, we only have the BLAS routine available.  | 
    
| 
           Ok, we can of course switch, but then we should perhaps add the exact singularity check?  | 
    
| 
           Yes, and we can do that for both   | 
    
          Codecov ReportAll modified and coverable lines are covered by tests ✅ 
 Additional details and impacted files@@            Coverage Diff             @@
##           master    #1194      +/-   ##
==========================================
- Coverage   91.89%   91.86%   -0.03%     
==========================================
  Files          34       34              
  Lines       15360    15371      +11     
==========================================
+ Hits        14115    14121       +6     
- Misses       1245     1250       +5     ☔ View full report in Codecov by Sentry.  | 
    
…#1194) Co-authored-by: Alexis Montoison <[email protected]>
Backported PRs: - [x] #1194 - [x] #1207 - [x] #1196 <!-- Explicitly declare type constructor imports --> - [x] #1202 <!-- Add fast path in generic matmul --> - [x] #1203 <!-- Restrict Diagonal sqrt branch to positive diag --> - [x] #1210 <!-- Indirection in matrix multiplication to avoid ambiguities -->
| 
           This appears to have introduced a bug into the benchmarking tests (I cannot vouch for their quality, only that this commit seems to have introduced a regression in them): JuliaLang/julia@c8ed17f#commitcomment-153581870  | 
    
We have used that in right-triangular solves already.