P3371R5 Fix C++26 by making the rank-1, rank-2, rank-k, and rank-2k updates consistent with the BLAS #8550

eisenwave · 2025-11-17T16:21:42Z

Fixes #8479
Fixes NB US 168-277 (C++26 CD).

Also fixes cplusplus/papers#2028
Also fixes https://github.com/cplusplus/nbballot/issues/852

…pdates consistent with the BLAS Fixes NB US 168-277 (C++26 CD).

frederick-vs-ja · 2025-11-26T01:38:05Z

source/numerics.tex

+an \tcode{InMat} template parameter, and
+a function parameter \tcode{InMat E},
+\tcode{t} applies to accesses done through the parameter \tcode{E}.
+\tcode{F} only accesses the triangle of \tcode{E} specified by \tcodee{t}.


CI failure was caused by the typo.

Suggested change

\tcode{F} only accesses the triangle of \tcode{E} specified by \tcodee{t}.

\tcode{F} only accesses the triangle of \tcode{E} specified by \tcode{t}.

jwakely · 2025-11-28T11:45:44Z

source/numerics.tex

+  // updating nonsymmetric rank-1 matrix update
+  template<@\exposconcept{in-vector}@ InVec1, @\exposconcept{in-vector}@ InVec2, @\exposconcept{in-matrix}@ InMat, @\exposconcept{out-matrix}@ OutMat>
+    void matrix_rank_1_update(InVec1 x, InVec2 y, InMat E, OutMat A);
+  template<class ExecutionPolicy, @\exposconcept{in-vector}@ InVec1, @\exposconcept{in-matrix}@ InMat, @\exposconcept{in-vector}@ InVec2,


The incoming paper is wrong here and needs to be fixed. The \itemdecl on line 14980 has the template parameters in the correct order (matching the non-parallel overload, and matching the function parameters). It should be fixed here in the synopsis.

Suggested change

template<class ExecutionPolicy, @\exposconcept{in-vector}@ InVec1, @\exposconcept{in-matrix}@ InMat, @\exposconcept{in-vector}@ InVec2,

template<class ExecutionPolicy, @\exposconcept{in-vector}@ InVec1, @\exposconcept{in-vector}@ InVec2, @\exposconcept{in-matrix}@ InMat,

jwakely · 2025-11-28T12:13:28Z

Too late now, but this would have been much less difficult to review as two commits, one doing all the inout sub-word edits, and another adding and removing blocks of text 😢

Edit: three commits, even. One changing inout to in, one adding/removing blocks of text, and then one changing every "class Scalar" to "scalar Scalar", which needs to be done after adding the new functions.

jwakely · 2025-11-28T12:19:05Z

source/numerics.tex

  template<class ExecutionPolicy,
-           @\exposconcept{in-vector}@ InVec, @\exposconcept{possibly-packed-inout-matrix}@ InOutMat, class Triangle>
-    void hermitian_matrix_rank_1_update(ExecutionPolicy&& exec, InVec x, InOutMat A, Triangle t);
+           class Scalar, @\exposconcept{in-vector InVec}@, @\exposconcept{in-matrix}@ InMat, @\exposconcept{possibly-packed-out-matrix}@ OutMat,


Suggested change

class Scalar, @\exposconcept{in-vector InVec}@, @\exposconcept{in-matrix}@ InMat, @\exposconcept{possibly-packed-out-matrix}@ OutMat,

class Scalar, @\exposconcept{in-vector}@ InVec, @\exposconcept{in-matrix}@ InMat, @\exposconcept{possibly-packed-out-matrix}@ OutMat,

jwakely · 2025-11-28T12:20:48Z

source/numerics.tex

+template<in-matrix InMat1,
+         in-matrix InMat2,
+         in-matrix InMat3,
+         possibly-packed-out-matrix OutMat,


Suggested change

template<in-matrix InMat1,

in-matrix InMat2,

in-matrix InMat3,

possibly-packed-out-matrix OutMat,

template<@\exposconcept{in-matrix}@ InMat1,

@\exposconcept{in-matrix}@ InMat2,

@\exposconcept{in-matrix}@ InMat3,

@\exposconcept{possibly-packed-out-matrix}@ OutMat,

Too many newlines here and in the \itemdecls that follow.

jwakely · 2025-11-28T12:21:26Z

source/numerics.tex

+         in-matrix InMat1,
+         in-matrix InMat2,
+         in-matrix InMat3,
+         possibly-packed-out-matrix OutMat,


Suggested change

in-matrix InMat1,

in-matrix InMat2,

in-matrix InMat3,

possibly-packed-out-matrix OutMat,

@\exposconcept{in-matrix}@ InMat1,

@\exposconcept{in-matrix}@ InMat2,

@\exposconcept{in-matrix}@ InMat3,

@\exposconcept{possibly-packed-out-matrix}@ OutMat,

jwakely

@mhoemmen please confirm that the new class Scalar template parameters being added by this paper should actually be scalar Scalar instead, since that's part of this paper, right?

Edit: yes, the editor's note at https://wiki.edg.com/pub/Wg21kona2025/StrawPolls/P3371R5.html#constrain-all-class-scalar-template-parameters says to rename all class Scalar to scalar Scalar after applying the changes from the paper. So the wording in the paper is "wrong" but the paper gives instructions for fixing it.

jwakely · 2025-11-28T12:24:31Z

source/numerics.tex

+                                        Scalar alpha, InMat A, OutMat C, Triangle t);
+
+  // updating rank-k symmetric matrix update
+  template<class Scalar,


Surely this should be scalar

Suggested change

template<class Scalar,

template<@\exposconcept{scalar}@ Scalar,

jwakely · 2025-11-28T12:24:57Z

source/numerics.tex

+    void symmetric_matrix_rank_k_update(
+      Scalar alpha,
+      InMat1 A, InMat2 E, OutMat C, Triangle t);
+  template<class ExecutionPolicy, class Scalar,


Suggested change

template<class ExecutionPolicy, class Scalar,

template<class ExecutionPolicy, @\exposconcept{scalar}@ Scalar,

jwakely · 2025-11-28T12:25:06Z

source/numerics.tex

+      InMat1 A, InMat2 E, OutMat C, Triangle t);
+
+  // updating rank-k Hermitian matrix update
+  template<class Scalar,


Suggested change

template<class Scalar,

template<@\exposconcept{scalar}@ Scalar,

jwakely · 2025-11-28T12:25:12Z

source/numerics.tex

+    void hermitian_matrix_rank_k_update(
+      Scalar alpha,
+      InMat1 A, InMat2 E, OutMat C, Triangle t);
+  template<class ExecutionPolicy, class Scalar,


Suggested change

template<class ExecutionPolicy, class Scalar,

template<class ExecutionPolicy, @\exposconcept{scalar}@ Scalar,

jwakely · 2025-11-28T12:44:46Z

source/numerics.tex

+                                        Scalar alpha, InVec x, OutMat A, Triangle t);
+
+  // updating symmetric rank-1 matrix update
+  template<class Scalar, @\exposconcept{in-vector}@ InVec, @\exposconcept{in-matrix}@ InMat, @\exposconcept{possibly-packed-out-matrix}@ OutMat,


Suggested change

template<class Scalar, @\exposconcept{in-vector}@ InVec, @\exposconcept{in-matrix}@ InMat, @\exposconcept{possibly-packed-out-matrix}@ OutMat,

template<@\exposconcept{scalar}@ Scalar, @\exposconcept{in-vector}@ InVec, @\exposconcept{in-matrix}@ InMat, @\exposconcept{possibly-packed-out-matrix}@ OutMat,

and similarly in the other new functions below.

jwakely · 2025-11-28T12:53:22Z

source/numerics.tex

+           @\exposconcept{possibly-packed-out-matrix}@ OutMat,
+           class Triangle>
+    void symmetric_matrix_rank_k_update(
+      Scalar alpha,


The formatting of these new functions is inconsistent with the ones above and below them. I think we should ignore the incoming paper's formatting and be consistent.

jwakely · 2025-11-28T12:56:35Z

source/numerics.tex

  template<@\exposconcept{in-matrix}@ InMat1, @\exposconcept{in-matrix}@ InMat2,
-           @\exposconcept{possibly-packed-inout-matrix}@ InOutMat, class Triangle>
-    void symmetric_matrix_rank_2k_update(InMat1 A, InMat2 B, InOutMat C, Triangle t);
+           @\exposconcept{in-matrix}@ InMat3,


I think these InMat3 template parameters don't need to be on a new line, they can follow InMat2.

jwakely · 2025-11-28T13:03:19Z

source/numerics.tex

+\mandates
+\begin{itemize}
+\item
+\tcode{\exposconcept{possibly-multipliable}<OutMat, InVec2, InVec1>()}


Suggested change

\tcode{\exposconcept{possibly-multipliable}<OutMat, InVec2, InVec1>()}

\tcode{\exposid{possibly-multipliable}<OutMat, InVec2, InVec1>()}

This is just a function template, not a concept.

jwakely · 2025-11-28T13:03:39Z

source/numerics.tex

+\tcode{\exposconcept{possibly-multipliable}<OutMat, InVec2, InVec1>()}
+is \tcode{true}, and
+\item
+\tcode{\exposconcept{possibly-addable}<OutMat, InMat, OutMat>()}


Suggested change

\tcode{\exposconcept{possibly-addable}<OutMat, InMat, OutMat>()}

\tcode{\exposid{possibly-addable}<OutMat, InMat, OutMat>()}

Just a function template, not a concept.

jwakely · 2025-11-28T13:06:07Z

source/numerics.tex

+
+template<class T>
+  concept @\defexposconcept{scalar}@ =
+    @\libconcept{semiregular}@<T> && (!@\exposconcept{is-mdspan}@<T>) && (!is_execution_policy_v<T>);


Suggested change

@\libconcept{semiregular}@<T> && (!@\exposconcept{is-mdspan}@<T>) && (!is_execution_policy_v<T>);

@\libconcept{semiregular}@<T> && (!@\exposid{is-mdspan}@<T>) && (!is_execution_policy_v<T>);

This is just a variable template, not a concept.

jwakely · 2025-11-28T13:25:37Z

source/numerics.tex

 template<class ExecutionPolicy,
-         @\exposconcept{in-vector}@ InVec, @\exposconcept{possibly-packed-inout-matrix}@ InOutMat, class Triangle>
-  void symmetric_matrix_rank_1_update(ExecutionPolicy&& exec, InVec x, InOutMat A, Triangle t);
+         @\exposconcept{scalar}@ Scalar, @\exposconcept{in-vector}@ InVec, @\exposconcept{in-matrix}@ InMat, @\exposconcept{possibly-packed-out-matrix}@ OutMat, class Triangle>


Suggested change

@\exposconcept{scalar}@ Scalar, @\exposconcept{in-vector}@ InVec, @\exposconcept{in-matrix}@ InMat, @\exposconcept{possibly-packed-out-matrix}@ OutMat, class Triangle>

@\exposconcept{scalar}@ Scalar, @\exposconcept{in-vector}@ InVec, @\exposconcept{in-matrix}@ InMat, @\exposconcept{possibly-packed-out-matrix}@ OutMat,

class Triangle>

Overfull hbox

jwakely · 2025-11-28T13:30:53Z

source/numerics.tex

 \pnum
 These functions perform
-a Hermitian rank-1 update of the Hermitian matrix \tcode{A},
+an updating Hermitian rank-1 update of the Hermitian matrix \tcode{E},


Suggested change

an updating Hermitian rank-1 update of the Hermitian matrix \tcode{E},

an updating Hermitian rank-1 update of the Hermitian matrix \tcode{A}

using the Hermitian matrix \tcode{E},

jwakely · 2025-11-28T13:32:07Z

source/numerics.tex

 \effects
-Computes a matrix $A'$ such that $A' = A + x x^H$ and
-assigns each element of $A'$ to the corresponding element of $A$.
+Computes a matrix $A = E + \alpha x x^H$,


Suggested change

Computes a matrix $A = E + \alpha x x^H$,

Computes $A = E + \alpha x x^H$,

jwakely · 2025-11-28T13:33:28Z

source/numerics.tex

+For accesses \tcode{E[i, j]} outside the triangle specified by \tcode{t},
+\tcode{F} only uses the value
+\begin{itemize}
+\item \tcode{\exposid{conj-if-needed}(E[j, i])} if the name of F starts with hermitian, or


Suggested change

\item \tcode{\exposid{conj-if-needed}(E[j, i])} if the name of F starts with hermitian, or

\item \tcode{\exposid{conj-if-needed}(E[j, i])} if the name of F starts with \tcode{hermitian}, or

jwakely · 2025-11-28T13:33:40Z

source/numerics.tex

+\tcode{F} only uses the value
+\begin{itemize}
+\item \tcode{\exposid{conj-if-needed}(E[j, i])} if the name of F starts with hermitian, or
+\item \tcode{E[j, i]} if the name of F starts with symmetric.


Suggested change

\item \tcode{E[j, i]} if the name of F starts with symmetric.

\item \tcode{E[j, i]} if the name of F starts with \tcode{symmetric}.

jwakely

The paper has these line breaks in template parameter lists but they're unnecessary IMHO.

jwakely · 2025-11-28T13:36:18Z

source/numerics.tex

+template<@\exposconcept{in-vector}@ InVec1, @\exposconcept{in-vector}@ InVec2,
+         @\exposconcept{in-matrix}@ InMat,


Suggested change

template<@\exposconcept{in-vector}@ InVec1, @\exposconcept{in-vector}@ InVec2,

@\exposconcept{in-matrix}@ InMat,

template<@\exposconcept{in-vector}@ InVec1, @\exposconcept{in-vector}@ InVec2, @\exposconcept{in-matrix}@ InMat,

jwakely · 2025-11-28T13:36:44Z

source/numerics.tex

+template<class ExecutionPolicy, @\exposconcept{in-vector}@ InVec1, @\exposconcept{in-vector}@ InVec2,
+         @\exposconcept{in-matrix}@ InMat,


Suggested change

template<class ExecutionPolicy, @\exposconcept{in-vector}@ InVec1, @\exposconcept{in-vector}@ InVec2,

@\exposconcept{in-matrix}@ InMat,

template<class ExecutionPolicy, @\exposconcept{in-vector}@ InVec1, @\exposconcept{in-vector}@ InVec2, @\exposconcept{in-matrix}@ InMat,

jwakely · 2025-11-28T13:39:39Z

source/numerics.tex

+template<@\exposconcept{in-vector}@ InVec1, @\exposconcept{in-vector}@ InVec2,
+         @\exposconcept{in-matrix}@ InMat,


Suggested change

template<@\exposconcept{in-vector}@ InVec1, @\exposconcept{in-vector}@ InVec2,

@\exposconcept{in-matrix}@ InMat,

template<@\exposconcept{in-vector}@ InVec1, @\exposconcept{in-vector}@ InVec2, @\exposconcept{in-matrix}@ InMat,

jwakely · 2025-11-28T13:40:02Z

source/numerics.tex

+template<class ExecutionPolicy, @\exposconcept{in-vector}@ InVec1, @\exposconcept{in-vector}@ InVec2,
+         @\exposconcept{in-matrix}@ InMat,


Suggested change

template<class ExecutionPolicy, @\exposconcept{in-vector}@ InVec1, @\exposconcept{in-vector}@ InVec2,

@\exposconcept{in-matrix}@ InMat,

template<class ExecutionPolicy, @\exposconcept{in-vector}@ InVec1, @\exposconcept{in-vector}@ InVec2, @\exposconcept{in-matrix}@ InMat,

jwakely · 2025-11-28T13:41:05Z

source/numerics.tex

+
+\pnum
+\remarks
+\tcode{A} my alias \tcode{E}.


Suggested change

\tcode{A} my alias \tcode{E}.

\tcode{A} may alias \tcode{E}.

jwakely · 2025-11-28T13:45:20Z

source/numerics.tex

-\item
-\tcode{C.extent(0)} equals \tcode{C.extent(1)}, and
+\tcode{\exposid{multipliable}(A, transposed(A), C)}
+is \tcode{true};


The incoming paper has "and" after the semi-colon. I think we either want that, or just turn the semi-colon into a full stop. A list of preconditions has an implicit "and" between them anyway.

jwakely · 2025-11-28T14:02:48Z

source/numerics.tex

-\indexlibraryglobal{hermitian_matrix_rank_k_update}%
+\indexlibraryglobal{symmetric_matrix_rank_k_update}%
 \begin{itemdecl}
-template<class Scalar, @\exposconcept{in-matrix}@ InMat, @\exposconcept{possibly-packed-inout-matrix}@ InOutMat, class Triangle>


Something's gone wrong here, I think you've removed the hermitian_matrix_rank_k_update overloads with an alpha parameter, which should not have been removed.

The paper shows that we should end up with symmetric_matrix_rank_k_update with one input matrix, then hermitian_matrix_rank_k_update with one input matrix, then symmetric_matrix_rank_k_update with two input matrices, then hermitian_matrix_rank_k_update with two input matrices.

But these changes give us:

So we're missing the hermitian_matrix_rank_k_update overloads with one input matrix.

jwakely · 2025-11-28T14:03:46Z

[Editorial Note: The changes proposed here are rebased atop the changes proposed in [LWG4137], “Fix Mandates, Preconditions, and Complexity elements of [linalg] algorithms.” – end note]

Are we going to get conflicts when the LWG issues from Kona are applied?

eisenwave added this to the post-2025-11 milestone Nov 17, 2025

eisenwave force-pushed the motions-2025-11-lwg-20 branch from be411bc to df99190 Compare November 17, 2025 17:04

P3371R5 Fix C++26 by making the rank-1, rank-2, rank-k, and rank-2k u…

d933aae

…pdates consistent with the BLAS Fixes NB US 168-277 (C++26 CD).

eisenwave force-pushed the motions-2025-11-lwg-20 branch from df99190 to d933aae Compare November 17, 2025 18:20

eisenwave marked this pull request as ready for review November 17, 2025 18:23

frederick-vs-ja reviewed Nov 26, 2025

View reviewed changes

jwakely requested changes Nov 28, 2025

View reviewed changes

jwakely reviewed Nov 28, 2025

View reviewed changes

jwakely requested changes Nov 28, 2025

View reviewed changes

jwakely reviewed Nov 28, 2025

View reviewed changes

	\tcode{F} only accesses the triangle of \tcode{E} specified by \tcodee{t}.
	\tcode{F} only accesses the triangle of \tcode{E} specified by \tcode{t}.

	template<class ExecutionPolicy, @\exposconcept{in-vector}@ InVec1, @\exposconcept{in-matrix}@ InMat, @\exposconcept{in-vector}@ InVec2,
	template<class ExecutionPolicy, @\exposconcept{in-vector}@ InVec1, @\exposconcept{in-vector}@ InVec2, @\exposconcept{in-matrix}@ InMat,

	class Scalar, @\exposconcept{in-vector InVec}@, @\exposconcept{in-matrix}@ InMat, @\exposconcept{possibly-packed-out-matrix}@ OutMat,
	class Scalar, @\exposconcept{in-vector}@ InVec, @\exposconcept{in-matrix}@ InMat, @\exposconcept{possibly-packed-out-matrix}@ OutMat,

	template<class Scalar,
	template<@\exposconcept{scalar}@ Scalar,

	template<class ExecutionPolicy, class Scalar,
	template<class ExecutionPolicy, @\exposconcept{scalar}@ Scalar,

	template<class Scalar, @\exposconcept{in-vector}@ InVec, @\exposconcept{in-matrix}@ InMat, @\exposconcept{possibly-packed-out-matrix}@ OutMat,
	template<@\exposconcept{scalar}@ Scalar, @\exposconcept{in-vector}@ InVec, @\exposconcept{in-matrix}@ InMat, @\exposconcept{possibly-packed-out-matrix}@ OutMat,

	\tcode{\exposconcept{possibly-multipliable}<OutMat, InVec2, InVec1>()}
	\tcode{\exposid{possibly-multipliable}<OutMat, InVec2, InVec1>()}

	\tcode{\exposconcept{possibly-addable}<OutMat, InMat, OutMat>()}
	\tcode{\exposid{possibly-addable}<OutMat, InMat, OutMat>()}

	@\libconcept{semiregular}@<T> && (!@\exposconcept{is-mdspan}@<T>) && (!is_execution_policy_v<T>);
	@\libconcept{semiregular}@<T> && (!@\exposid{is-mdspan}@<T>) && (!is_execution_policy_v<T>);

	@\exposconcept{scalar}@ Scalar, @\exposconcept{in-vector}@ InVec, @\exposconcept{in-matrix}@ InMat, @\exposconcept{possibly-packed-out-matrix}@ OutMat, class Triangle>
	@\exposconcept{scalar}@ Scalar, @\exposconcept{in-vector}@ InVec, @\exposconcept{in-matrix}@ InMat, @\exposconcept{possibly-packed-out-matrix}@ OutMat,
	class Triangle>

	an updating Hermitian rank-1 update of the Hermitian matrix \tcode{E},
	an updating Hermitian rank-1 update of the Hermitian matrix \tcode{A}
	using the Hermitian matrix \tcode{E},

	Computes a matrix $A = E + \alpha x x^H$,
	Computes $A = E + \alpha x x^H$,

	\item \tcode{\exposid{conj-if-needed}(E[j, i])} if the name of F starts with hermitian, or
	\item \tcode{\exposid{conj-if-needed}(E[j, i])} if the name of F starts with \tcode{hermitian}, or

	\item \tcode{E[j, i]} if the name of F starts with symmetric.
	\item \tcode{E[j, i]} if the name of F starts with \tcode{symmetric}.

		template<@\exposconcept{in-vector}@ InVec1, @\exposconcept{in-vector}@ InVec2,
		@\exposconcept{in-matrix}@ InMat,

P3371R5 Fix C++26 by making the rank-1, rank-2, rank-k, and rank-2k updates consistent with the BLAS #8550

Are you sure you want to change the base?

P3371R5 Fix C++26 by making the rank-1, rank-2, rank-k, and rank-2k updates consistent with the BLAS #8550

Uh oh!

Conversation

eisenwave commented Nov 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jwakely commented Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jwakely left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jwakely left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jwakely commented Nov 28, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

jwakely commented Nov 28, 2025 •

edited

Loading

jwakely left a comment •

edited

Loading