-
Notifications
You must be signed in to change notification settings - Fork 25
Description
P1673: LWG review 2023/11/06 (Kona) and follow-ons
Resuming from [linalg.alg.blas2.trsv], where we stopped last time (2023/10/25).
Done, either in the main branch or in PR #426
- Generally, we need to qualify use of
forwardasstd::forward. Change throughout. - Replace
multiplyablewithmultipliablethroughout. (Fix spelling.) - Fix parallel algorithms "element access function" wording in [algorithms.parallel.defns] 3, by adding "or
mdspantypes" after "iterators": "All operations of the categories of the iterators ormdspantypes that the algorithm is instantiated with." - Add
BinaryDivideOpto the list in [algorithms.parallel.user] (Requirements on user-provided function objects). - Generally, say "
ymay aliasx" instead of "xandymay alias." This is consistent with the definition of "alias" in the front matter. Generally, "out-argument may alias in-argument." (PR P1673: LWG review (follow-on to PR 424) #426) - Change label
xmmtoxxmm. (DONE) - When passing an ExecutionPolicy
execinto Effects-equivalent-to code, always usestd::forward<ExecutionPolicy>(exec). - Fix
matrix_rank_1_update*to use inout matrix as intended. Wording is wrong! - Change the updating wording to use
$C'$ instead of relying on [linalg.general] 1.4. "Computes a matrix$C'$ such that$C' = C + \alpha A A^T$ and assigns each element of$C'$ to the corresponding element of$C$ . Do this generally, e.g., fortriangular_matrix_vector_product. - Change
linalg.alg.blas3.*stable names tolinalg.algs.blas3.*. - Fix stable names for BLAS 2 rank-1 symmetric and Hermitian updates.
- For
symmetric_matrix_rank_k_updateandhermitian_matrix_rank_k_update, change the template parameter nameInMat1toInMat, because there's noInMat2. Fix this in the synopsis as well. - Fix everything under
transposed(e.g.,layout_transposed) to match wording of existing layouts. Remove references to the padded layouts. - Fix
layout_blas_packedanalogously. - Remove editorial comments and italics from [linalg.reqs.val].
- For [linalg.algs.reqs] Para 1, avoid ambiguity by changing "type requirements" to "Constraints." However, the first (1.1) is more than just a Constraint, so separate (1.1) to the top, and add the Constraint that ExecutionPolicy is an execution policy. (This indicates that the algorithm is a parallel algorithm.)
- Verify that the wording up front explains that ill-formed constructions (e.g., x and y complex, A real for hermitian rank-2 update) are forbidden (you may find an overload, but instantiating it would fail -- it's a precondition). (linalg.reqs.alg] expresses this for plus, times, etc. How about for complex number operations?) (See LWG question below.)
Check with LWG
- For [linalg.algs.reqs] Para 1 (changing "type requirements" to "Constraints"; see above): Fixed by changing "type requirements" to "Constraints." Added the Constraint that ExecutionPolicy is an execution policy. Removed the requirement that the algorithms that take
ExecutionPolicyare parallel algorithms, because that would be circular with [algorithms.parallel] 2 ("A parallel algorithm is a function template listed in this document with a template parameter namedExecutionPolicy"). - For [mdspan.layout.policy.reqmts], it's not clear whether this says that
MP::mapping<E>must be valid for ANY specializationEofextents. That was not our intent. Should we add a Note or just add clarifying normative language? (The latter would probably not break existing user code, unless users are somehow depending on generic code being able to take any layoutMPand anyextentsspecializationE, and getMP::mapping<E>from it.) This matters for P1673 becauselayout_transposed::mapping<E>requires thatEhave rank 2.- Christian asked some LWG people; they said we might want to add a Note, but the wording itself permits this.
- Remove constraint that
stride(r)requires rank 2? Note thatstrideinlayout_leftandlayout_rightimpose a Constraint thatrank() > 0, butlayout_strideonly has this as a precondition. - Rename
layout_transposetolayout_transposed?
Resolved (don't need to check with LWG)
- Verify that the wording up front explains that ill-formed constructions (e.g., x and y complex, A real for hermitian rank-2 update) are forbidden (you may find an overload, but instantiating it would fail -- it's a precondition). (linalg.reqs.alg] expresses this for plus, times, etc. How about for complex number operations?)
- We think [linalg.helpers.conj] partly covers this.
conj-if-needed(E)only evaluates toconj(E)ifconj(E)is a valid expression and the type ofEis not an arithmetic type. Otherwise, it evaluates toE.- If
conj(E)is a valid expression but "the function selected by overload resolution does not return the complex conjugate of its input, the program is ill-formed, no diagnostic required."
- The main question is whether
conj(E)is a linear algebra value type. - Ditto for the few algorithms that use
realandimag. - We don't need to ask this question.
- We think [linalg.helpers.conj] partly covers this.
Points for specific sections
[linalg.alg.blas2.trsv]
("take an x parameter" cannot be misread as "take a symbolic name of a parameter x"; "take a parameter x" can be misread in that way. Therefore, we prefer the existing wording, "take an x parameter.") (OK)
Para 6: "Computes a vector
Para 12: "Computes a vector
Para 13: Complexity method differs from in Para 7. Flip order of b and A in Para 13 to be consistent with Para 7. (DONE)
[linalg.alg.blas2.rank1]
Para 7: Put forward<ExecutionPolicy> around exec. Generally do this where needed. (DONE)
Split Para 7 into two sentences, to avoid possible confusion due to code-font semicolons. (DONE)
[linalg.alg.blas2.symrank1]
Para 3: Mandates here does not follow the decltype convention; change that. (DONE)
(LWG appreciates the Para 1 unambiguous self-reference : - ) . ) (OK)
Para 7: "Computes a matrix
Para 11: "Computes a matrix
[linalg.algs.blas2.rank2]
Verify that the wording up front explains that ill-formed constructions (e.g., x and y complex, A real for hermitian rank-2 update) are forbidden (you may find an overload, but instantiating it would fail -- it's a precondition). (linalg.reqs.alg] expresses this for plus, times, etc. How about for complex number operations?) (DONE -- see question for LWG)
[linalg.algs.blas3.gemm]
Mandates: the above and also? (see notes)
Scribe missed some time
The in-place triangular_matrix_right_product etc. need a complexity clause. (DONE)
[linalg.alg.blas3.rankk]
Change label from linalg.alg to linalg.algs here. (DONE)
For symmetric_matrix_rank_k_update function signature, change InMat1 to InMat, because there's no InMat2. Fix this in the synopsis as well. (DONE)
Para 6: Change the updating wording to use triangular_matrix_vector_product ("Computes
For [linalg.algs.reqs] Para 1, does "type requirements" mean constraints? Yes, it does, to avoid ambiguity. Therefore, change "type requirements" to "Constraints." However, the first (1.1) is more than just a Constraint. We can separate (1.1) to the top, add the Constraint that ExecutionPolicy is an execution policy. (This indicates that the algorithm is a parallel algorithm.) (DONE)
[linalg.alg.blas3.rank2k]
Change linalg.alg to linalg.algs. (DONE)
Fix Effects generally. (DONE)
For next time
Next starting point is BLAS 3 TRSM.