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

P1673: LWG review 2023/10/25 #421

Open
mhoemmen opened this issue Oct 25, 2023 · 1 comment
Open

P1673: LWG review 2023/10/25 #421

mhoemmen opened this issue Oct 25, 2023 · 1 comment

Comments

@mhoemmen
Copy link
Contributor

mhoemmen commented Oct 25, 2023

P1673: LWG review 2023/10/25

matrix_one_norm

Last week, we stopped after matrix_frob_norm and before matrix_one_norm.

Add Note for BLAS Standard function name, even though not in Reference BLAS.

(In theory, a user could have a matrix with a layout for which submdspan isn't defined.)

"28.9.13.13.2 One norm with default result type" doesn't have a stable name. Remove that heading and make this part of the whole section. (Christian already has that implemented in his branch.) Ditto for the "see-below" changes (delete "see-below" and just return auto); flatten those functions to one line (Christian already did).

matrix_inf_norm

Ditto.

[linalg.alg.blas2.gemv]

2.1: Do we always have the same names of A, x, y, and z, in (e.g.,) possibly-multiplyable(decltype(A), decltype(x), decltype(y))? Christian: Yes, that's the idea, though we should check.

Shorten Complexity wording in para 4. "A count of mdspan array accesses and arithmetic operations that is linear in x.extent(0) times A.extent(1)."

Delete "Overwriting matrix-vector product" and "Updating matrix-vector product" headers. Make those headers just a new paragraph underneath: "This function performs an overwriting matrix-vector product."

Para 1: Replace "in this section" with "in [linalg.algs.blas2.gemv]." (Also, it's a "subclause," not a "section.")

Example for GEMV: Change scaled_matvec_2 to scaled_transposed_matvec. Use * instead of "times" in the comment over the last function.

[linalg.algs.blas2.symv]

Change "in this section" to the stable name of the parent, as above.

Remove subheadings "Overwriting symmetric matrix-vector product" etc., as above.

Edit Complexity as above. "A count of ... x.extent(0) times A.extent(1)." Perhaps even just "O(x.extent(0) * A.extent(1))."

^^^ Generally change Complexity to use brief big-Oh notation. The Oh is curly, math font. Fix this generally.

Add a cross-reference to [linalg.general], that explains the Triangle template parameter. "This function performs an overwriting symmetric matrix-vector product accessing the elements specified by t ([linalg.general])."

[linalg.algs.blas2.hemv]

Analogous changes as above.

[linalg.algs.blas2.trmv]

Analogous changes as above.

"taking account of the Triangle and DiagonalStorage parameters ([linalg.general])."

Note: "... other ExecutionPolicy specific optimizations" (remove hyphen) -- [linalg.algs.blas2.trmv.in-place].

Stop before [linalg.algs.blas2.trsv].

@mhoemmen
Copy link
Contributor Author

mhoemmen commented Oct 25, 2023

Things to fix this time

  • "in this section" -> "in [stable.name]"
  • Complexity
  • Remove section headers like "28.9.13.13.2 One norm with default result type" and make them a sentence in the section. (Christian already has that implemented in his branch.)
  • Ditto for the "see-below" changes (delete "see-below" and just return auto); flatten those functions to one line (Christian already did).
  • Example for GEMV: Change scaled_matvec_2 to scaled_transposed_matvec. Use * instead of "times" in the comment over the last function.
  • "This function performs an overwriting matrix-vector product [accessing the elements specified by t ([linalg.general])]."
  • Note: "... other ExecutionPolicy specific optimizations" (remove hyphen) -- [linalg.algs.blas2.trmv.in-place].

Other things to fix

  • matrix_*_norm: Add Note for BLAS Standard function name, even though these functions are not in the Reference BLAS.
  • Dimension-checking for rank-2 updates and BLAS 2 functions
  • Packed layouts
  • layout_transpose(d) and transposed (haven't reviewed yet)

@mhoemmen mhoemmen mentioned this issue Oct 25, 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

No branches or pull requests

1 participant