Skip to content
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

new doxygen group structure #802

Merged
merged 7 commits into from
Jul 4, 2023
Merged

new doxygen group structure #802

merged 7 commits into from
Jul 4, 2023

Conversation

mgates3
Copy link
Contributor

@mgates3 mgates3 commented Mar 1, 2023

Description
The current Doxygen groups are organized in 4 nested layers:

  1. Matrix type (general, symmetric, ...)
  2. Operation (linear solve, eigenvalues, ...)
  3. Precision (s, d, c, z)
  4. Routine

I propose essentially swapping layers 1 & 2 and 3 & 4, which I believe is more intuitive for users trying to find functionality:

  1. Operation (linear solve, eigenvalues, ...)
  2. Matrix type (general, symmetric, ...)
  3. Routine
  4. Precision (s, d, c, z)

Further, in the current organization, many routines are lumped into "Computational Routines" or "Other Auxiliary Routines" or "Other Computational Routines". For instance,
LAPACK > General Matrix > Computational Routines > double
contains 50-some routines (gelqt, geqrf, gecon, geequ, ...) that are not very related. (There are also some miscategorized there like sgelqt, zgelqt.)

Instead, I propose grouping these by their related driver-level routine. For example, getrf, getrs, gecon, gerfs are all related to LU factorization solving AX = B.

Sample HTML output is on my page. A few options like graphs are disabled, just for development purposes. They would be re-enabled in the final version.

Design questions
There are a few options that I'm looking for feedback about. I implemented several different organizations for different matrix types.

  1. Should categories be based on matrix sub-type (e.g., packed, banded, tridiagonal), or put these sub-types in one category with rule lines separating them? The first option is demonstrated by
    LAPACK > Linear solve driver > Hermitian/symmetric positive definite matrix: Cholesky,
    which has categories for each sub-type:
  • Hermitian/symmetric positive definite matrix: Cholesky
  • Hermitian/symmetric positive definite matrix: Cholesky, RFP
  • Hermitian/symmetric positive definite matrix: Cholesky, packed
  • Hermitian/symmetric positive definite matrix: Cholesky, banded
  • Hermitian/symmetric positive definite matrix: Cholesky, tridiagonal
    The second option is demonstrated by
    LAPACK > Linear solve driver > General matrix: LU,
    which is a single category:
  • General matrix: LU
    with rule lines for --- banded --- and --- tridiagonal ---.
  1. Should driver and computational and auxiliary routines be separate categories, or put them into one category with rule line separators? The first option is demonstrated by
    LAPACK > Linear solve driver > General matrix: LU and
    LAPACK > Linear solve computational routines > General matrix: LU.
    This highlights the drivers over the computational routines. The second option is demonstrated by
    LAPACK > Linear solve driver > Hermitian/symmetric positive definite matrix: Cholesky.
    (In which case the top-level category shouldn't say "driver".)
    This may be affected by the answer to (1.), and may have different answers for different sections of LAPACK, e.g., it may make sense for LU (many users use gesv, getrf, getrs, etc.), but less sense for Eigenvalue and SVD routines (many users use gesvd, few would use gebrd).

Design decisions
Feedback is welcome on these, too.

  1. Unitary (un) and orthogonal (or) matrices are grouped in the same category.
  2. Hermitian (he) and symmetric (sy) matrices are grouped in the same category. In some cases this puts 6 related routines together, with both complex-symmetric and complex-Hermitian: ssymv, dsymv, csymv, chemv, zsymv, zhemv.
  3. Orthogonal factorizations (QR, LQ, CS, etc.) are computational routines used in various drivers (least squares, SVD, etc.), so these are grouped as their own top-level category, unrelated to a driver.
  4. LAPACK has some BLAS-like routines such as lascl to scale a matrix, lacrm to multiply complex and real matrices. These are in LAPACK > BLAS-like.
  5. LAPACK also has some routines that exactly match BLAS, but extended to complex precision, namely csymv and zsymv. These are grouped in BLAS, even though they technically exist in LAPACK.
  6. There are some scalar operations, such as ladiv and lapy2, which are put into a "Level 0 BLAS (scalar ops)" category.

Implementation notes
Generally, each routine is in a category based on its name minus the precision, so sgemm, dgemm, cgemm, zgemm are all in the gemm category. The groups-usr.dox is reorganized as nested Doxygen groups to show the exact structure. Adding a routine requires adding a single line to this groups-usr.dox file. If that is forgotten, Doxygen does not report an error, but it can be easily detected by grepping for ingroup and defgroup, and comparing the results.

git grep -P -h 'defgroup \w+' | perl -pe 's/.*defgroup (\w+) .*/$1/' | sort        > ! defgroup.txt

git grep -P -h 'ingroup \w+'  | perl -pe 's/.*ingroup //'            | sort | uniq > ! ingroup.txt

diff ingroup.txt defgroup.txt | grep '<'

Checklist

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

@mgates3 mgates3 marked this pull request as draft March 1, 2023 16:38
@langou
Copy link
Contributor

langou commented Mar 4, 2023

First off, this is a lots of great work there. So thanks a lot @mgates3! This is great, great, great. I believe this will have a huge impact on our users and is a very important contribution. It might end up being one of our most impactful update. I am very excited by this PR!

Some comment below.

  1. Should categories be based on matrix sub-type (e.g., packed, banded, tridiagonal), or put these sub-types in one category with rule lines separating them?

Rule lines separating them as my preference. But not a strong opinion.

  1. Should driver and computational and auxiliary routines be separate categories, or put them into one category with rule line separators?

Driver and computational and auxiliary routines in separate categories. But not a strong opinion. One reason is that this makes the category more fluid. So a computational routine for LU can be used for something else. And if we hide it in the meander of LU, then it belongs more to LU, and I do not like it. I think having separate categories for computational and auxiliary makes the category more fluid (which is good IMHO).

We can also restart the debate on what driver is, what a driver is, what a computational is and what an auxiliary is. At some point, we said that "auxiliary" do not perform any check on entry data.

This may be affected by the answer to (1.), and may have different answers for different sections of LAPACK, e.g., it may make sense for LU (many users use gesv, getrf, getrs, etc.), but less sense for Eigenvalue and SVD routines (many users use gesvd, few would use gebrd).

I agree. I think it is fine to have only the design: "driver and computational and auxiliary routines in separate categories." Although I agree with your point.

  1. Unitary (un) and orthogonal (or) matrices are grouped in the same category.

Perfect.

  1. Hermitian (he) and symmetric (sy) matrices are grouped in the same category. In some cases this puts 6 related routines together, with both complex-symmetric and complex-Hermitian: ssymv, dsymv, csymv, chemv, zsymv, zhemv.

I would rather have DSY grouped with ZHE, but ZSY to be a stand alone by itself. It bothers me that ZSY is mixed with ZHE and DSY. Not sure if this is a possible design though. Well, we can disagree on this. What you did might make it easier for users. It is indeed complicated to explain that ZSY has nothing to do with DSY. Argh. It does bother me, but I see the interest for users of how you did it. So fine with me.

  1. LAPACK also has some routines that exactly match BLAS, but extended to complex precision, namely csymv and zsymv. These are grouped in BLAS, even though they technically exist in LAPACK.

My preference is to have these in BLAS-like, and not in BLAS. But same I understand why you did it. I think this might create some major confusion of what the BLAS is. We already had the issue a while back with xROT, where we had BLAS libraries which had it, and BLAS libraries which did not. I am afraid we end up in the same situation. Overall, it would be good if BLAS libraries provide csymv and zsymv, so fine with me. But I am getting ready for headaches.

  1. There are some scalar operations, such as ladiv and lapy2, which are put into a "Level 0 BLAS (scalar ops)" category.

Dang it. This is not what I call Level 0!!!! We are going to have a trademark war here!!! Oh well, all good with me. Yes, this is a good idea. Maybe “scalar operations” instead of level 0? Oh well. Good with me. Maybe "scalar operations".

@thijssteel
Copy link
Collaborator

I agree with @langou , this is amazing work

Should categories be based on matrix sub-type (e.g., packed, banded, tridiagonal), or put these sub-types in one category with rule lines separating them?

Rule lines, I think this is especially nice if special solvers are not available.

Should driver and computational and auxiliary routines be separate categories, or put them into one category with rule line separators?

I think separate categories. I agree with the fluidity @langou pointed out. I would also like to point out that an inexperienced user would be more likely to find the right routine if we split it. I've seen some students use xHSEQR without realizing that it requires a Hessenberg matrix.

Until I saw this PR, I had no idea that the ingroup comment was for Doxygen. Perhaps we need a contributing.md to explain stuff like this.

langou
langou previously approved these changes Mar 18, 2023
@codecov
Copy link

codecov bot commented Jul 4, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (e2428ec) 0.00% compared to head (79c1f02) 0.00%.

❗ Current head 79c1f02 differs from pull request most recent head 7107955. Consider uploading reports for the commit 7107955 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #802   +/-   ##
=======================================
  Coverage    0.00%    0.00%           
=======================================
  Files        1916     1916           
  Lines      188485   188485           
=======================================
  Misses     188485   188485           
Impacted Files Coverage Δ
INSTALL/dlamch.f 0.00% <ø> (ø)
INSTALL/droundup_lwork.f 0.00% <ø> (ø)
INSTALL/dsecnd_INT_ETIME.f 0.00% <ø> (ø)
INSTALL/ilaver.f 0.00% <ø> (ø)
INSTALL/lsame.f 0.00% <ø> (ø)
INSTALL/second_INT_ETIME.f 0.00% <ø> (ø)
INSTALL/slamch.f 0.00% <ø> (ø)
INSTALL/sroundup_lwork.f 0.00% <ø> (ø)
SRC/cbbcsd.f 0.00% <ø> (ø)
SRC/cbdsqr.f 0.00% <ø> (ø)
... and 107 more

... and 37 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@mgates3
Copy link
Contributor Author

mgates3 commented Jul 4, 2023

I finished my changes. I posted the latest output (minus dot graphs and source browsing, because those take a long time to generate and a lot of space). Also tested man pages to work correctly, so man dgetrf gets dgetrf docs. (The current manpages are annoyingly lumped into groups.)

I adopted separating matrix types by rule lines.

I used @langou's suggestion of "Scalar operations", instead of inventing my own "Level 0".
@langou, what do you call "Level 0"?
(Intel also has "Level 0", but that's for GPU computing, unrelated to BLAS.)

There's more to do, especially in the TESTING directory, but this is a good first step.

@mgates3 mgates3 marked this pull request as ready for review July 4, 2023 07:19
@langou
Copy link
Contributor

langou commented Jul 4, 2023

I used @langou's suggestion of "Scalar operations", instead of inventing my own "Level 0".

I think I like level 0.

@langou, what do you call "Level 0"?

A LAPACK code (for example) that only uses scalar to scalar operations. So something like EISPACK for example. Level 1 is only vector to vector, Level 2 is matrix to vector, Level 3 is matrix to matrix. So for example, GETF2 would be Level 2, GETRF would be Level 3. A level 0 code does not use the concept of submatrices.

@langou langou merged commit d19d6f5 into Reference-LAPACK:master Jul 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants