Recursive LAUUM

[jprhyne]

Description
As discussed in PR #1373, here is the implementation of a recursive LAUUM. Since some may prefer a blocked algorithm, we are proposing that we use *lauum.f as a dispatch to whatever implementation is desired in a given environment. It's possible to somehow utilize ilaenv to do the dispatch or to have this logic inside lauum.f directly.

In addition, since it is hardware dependent, I default to NX=64 as a crossover to lauu2. Due to how ILAENV parses the (sub)-routine names, any LAUUM routine that queries for NX will return the same value. This could be relevant if an NX parameter is to be introduced into the blocked variant. I'm not sure what the preferred method to deal with this is, so I propose that we defer this decision until it becomes relevant.

But, if it is better to solve this issue now, I propose adding another variable that checks for whatever comes after underscores in ILAENV. However, since this type of naming is not convention, I do not believe it is a good idea unless there becomes a standardized way of having different variant names in the main source directory.

Since this is a change to an auxiliary routine without a change in any public facing APIs, I do not think it is important to solve at this moment.
Checklist

• The documentation has been updated.
• If the PR solves a specific issue, it is set to be closed on merge. (N/A)

[codecov]

Codecov Report

❌ Patch coverage is 44.82759% with 160 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.50%. Comparing base (9e518f1) to head (f44603f).
⚠️ Report is 51 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
SRC/clauum_blocked.f	0.00%	35 Missing ⚠️
SRC/dlauum_blocked.f	0.00%	35 Missing ⚠️
SRC/slauum_blocked.f	0.00%	35 Missing ⚠️
SRC/zlauum_blocked.f	0.00%	35 Missing ⚠️
SRC/clauum_recursive.f	83.33%	5 Missing ⚠️
SRC/dlauum_recursive.f	83.33%	5 Missing ⚠️
SRC/slauum_recursive.f	83.33%	5 Missing ⚠️
SRC/zlauum_recursive.f	83.33%	5 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1402      +/-   ##
==========================================
+ Coverage   69.36%   69.50%   +0.14%     
==========================================
  Files        6122     6184      +62     
  Lines      486717   494962    +8245     
  Branches    23268    23426     +158     
==========================================
+ Hits       337590   344026    +6436     
- Misses     148689   150502    +1813     
+ Partials      438      434       -4     

Components	Coverage Δ
BLAS	99.13% <ø> (+1.19%)	⬆️
CBLAS	96.98% <ø> (ø)
LAPACK	82.36% <44.82%> (-0.03%)	⬇️
LAPACKE	2.15% <ø> (-0.02%)	⬇️
TMGLIB	55.69% <ø> (ø)
BLAS testing	88.54% <ø> (+0.20%)	⬆️
CBLAS testing	89.63% <ø> (ø)
LAPACK testing	82.21% <ø> (+<0.01%)	⬆️
LAPACKE testing	∅ <ø> (∅)

Files with missing lines	Coverage Δ
SRC/clauu2.f	78.57% <100.00%> (-3.79%)	⬇️
SRC/clauum.f	68.75% <100.00%> (-15.13%)	⬇️
SRC/dlauum.f	68.75% <100.00%> (-15.13%)	⬇️
SRC/ilaenv.f	27.56% <100.00%> (+1.35%)	⬆️
SRC/slauum.f	68.75% <100.00%> (-15.13%)	⬇️
SRC/zlauu2.f	78.57% <100.00%> (-3.79%)	⬇️
SRC/zlauum.f	68.75% <100.00%> (-15.13%)	⬇️
SRC/clauum_recursive.f	83.33% <83.33%> (ø)
SRC/dlauum_recursive.f	83.33% <83.33%> (ø)
SRC/slauum_recursive.f	83.33% <83.33%> (ø)
... and 5 more

... and 111 files with indirect coverage changes

---

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e518f1...f44603f. Read the comment docs.

[ACSimon33]

Hi @jprhyne,

could you please merge the current master back into all your PRs? We fixed numerical issues and errors on all platforms that are tested in CI, and the CI jobs are now configured to fail if new errors occur. That will help us to quickly determine if the changes in the PRs introduce new errors or can be merged.

[mohawk2]

I don't have visibility of the setting for this PR, but the default on this repo is to give permission to those with write permission, to push (including force-push) to a PR branch. That would mean that someone with permissions on this repo could rebase this branch past the master updates, and force-push it. (Rebasing is often considered better practice than merging, though there is debate on this)

[jprhyne]

I am working on lallm (another variant similar to this routine) and noticed that there is no statement on the unused triangular component of A inside LAUUM. I have opened an issue asking for clarification (Issue #1415). I'm more than happy to merge changes for that into this PR (after I update this branch and ensure all tests pass of course)

[jprhyne]

I still need to fix documentation, I'm just running the tests after merging the new master branch before doing any other changes.

[ACSimon33]

We could also put the old blocked lauum implementation into SRC/VARIANTS/lauum/... and make the new recursive implementation the default one in the ?lauum.f files. That way, we could get rid of the driver routines.

SRC/VARIANTS is currently not wired into the CMake build harness, but that is on my TODO list to investigate. We could give the user CMake options to select which variants are compiled.

[langou]

@ACSimon33 #1402 (comment)

We could also put the old blocked lauum implementation into SRC/VARIANTS/lauum/... and make the new recursive implementation the default one in the ?lauum.f files. That way, we could get rid of the driver routines.

SRC/VARIANTS is currently not wired into the CMake build harness, but that is on my TODO list to investigate. We could give the user CMake options to select which variants are compiled.

We are trying a new design here where variants of an algorithm are in SRC, as opposed to the VARIANTS folder.

I would like to keep it that way and try.

LAUMM is an entry point and then developers, vendors can deflect to a variant. Right now, @jprhyne chose LAUMM_RECURSIVE as a default for the reference version of LAPACK. However, we would like to expose as well LAUUM_BLOCKED. (LAUMM_BLOCKED is the current LAUUM.)

Actually LAUMM_BLOCKED could be renamed to LAUMM_BLOCKED_LL. If there is some traction for this renaming, I won't be opposed to it. (One could write a LAUMM_BLOCKED_RL, the right-looking variant of LAUUM.)

[langou]

Hi @ACSimon33, have a look. I think @jprhyne addressed all your feedback. (Thanks for the feedback.) I could either ask you to "re-review" or to "dismiss your review". I chose "re-review". Cheers, Julien.