Skip to content

Commit 1ca2c40

Browse files
committed
The issue submitter improves the issue discussion and adds proposed wording
1 parent 27248ad commit 1ca2c40

File tree

1 file changed

+99
-32
lines changed

1 file changed

+99
-32
lines changed

xml/issue4315.xml

Lines changed: 99 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -13,50 +13,117 @@
1313

1414
<discussion>
1515
<p>
16-
The wording for `vector_two_norm` <sref ref="[linalg.algs.blas1.nrm2]"/> and
17-
`matrix_frob_norm` <sref ref="[linalg.algs.blas1.matfrobnorm]"/> has two issues.
16+
The Returns clauses `vector_two_norm` <sref ref="[linalg.algs.blas1.nrm2]"/> and
17+
`matrix_frob_norm` <sref ref="[linalg.algs.blas1.matfrobnorm]"/> say that the
18+
functions return the "square root" of the sum of squares of the initial value and
19+
the absolute values of the elements of the input `mdspan`. However, nowhere in
20+
<sref ref="[linalg]"/> explains how to compute a square root.
1821
</p>
1922
<ol>
20-
<li><p>Their <i>Returns</i> clauses say that the functions return the "square
21-
root" of the sum of squares of the initial value and the absolute
22-
values of the elements of the input `mdspan`. However, nowhere in
23-
<sref ref="[linalg]"/> explains how to compute a square root.</p>
24-
<ol style="list-style-type: none">
25-
<li><p>1.a. The input `mdspan`'s `value_type` and the initial value type
26-
are not constrained in a way that would ensure that calling
27-
`std::sqrt` on this expression would be well-formed.</p></li>
28-
<li><p>1.b. There is no provision to find `sqrt` via argument-dependent
29-
lookup, even though <sref ref="[linalg]"/> has provisions to find `abs`, `conj`,
30-
`real`, and `imag` via argument-dependent lookup. There is no
31-
"`sqrt-if-needed`" analog to `abs-if-needed`, `conj-if-needed`,
32-
`real-if-needed`, and `imag-if-needed`.</p></li>
33-
</ol>
34-
</li>
35-
<li><p>The overloads that take an initial value parameter `Scalar init`
36-
return `Scalar`.</p>
37-
<ol style="list-style-type: none">
38-
<li><p>2.a. This may silently lose information if the function uses
39-
`std::sqrt` to compute square roots. For example, if `Scalar` and the
40-
input `mdspan`'s `value_type` are both `int`, the square root computed
41-
via `std::sqrt` would return `double`. However, `vector_two_norm` and
42-
`matrix_frob_norm` returning `Scalar` would force a rounding
43-
conversion back to `int`.</p></li>
44-
</ol>
45-
</li>
23+
<li><p>The input `mdspan`'s `value_type` and the initial value type are
24+
not constrained in a way that would ensure that calling `std::sqrt` on
25+
this expression would be well-formed.</p></li>
26+
<li><p>There is no provision to find `sqrt` via argument-dependent lookup,
27+
even though [linalg] has provisions to find `abs`, `conj`, `real`, and
28+
`imag` via argument-dependent lookup. There is no "`sqrt-if-needed`"
29+
analog to `abs-if-needed`, `conj-if-needed`, `real-if-needed`, and
30+
`imag-if-needed`.</p></li>
4631
</ol>
4732
<p>
48-
<b>Suggested fix:</b>
49-
<p/>
50-
The easiest fix for both issues is just to <i>Constrain</i> both `Scalar` and
33+
The easiest fix for both issues is just to Constrain both `Scalar` and
5134
the input `mdspan`'s `value_type` to be floating-point numbers or
5235
specializations of `std::complex` for these two functions. This
53-
presumes that relaxing this <i>Constraint</i> and fixing the above two issues
36+
presumes that relaxing this Constraint and fixing the above two issues
5437
later would be a non-breaking change. If that is <em>not</em> the case, then
5538
I would suggest removing the two functions entirely.
5639
</p>
5740
</discussion>
5841

5942
<resolution>
43+
<p>
44+
This wording is relative to <paper num="N5014"/>.
45+
</p>
46+
47+
<blockquote class="note">
48+
<p>
49+
[<i>Drafting note:</i> As a drive-by fix the proposed wording adds a missing closing parentheses in
50+
<sref ref="[linalg.algs.blas1.nrm2]"/> p2.]
51+
</p>
52+
</blockquote>
53+
54+
<ol>
55+
56+
<li><p>Modify <sref ref="[linalg.algs.blas1.nrm2]"/> as indicated:</p>
57+
58+
<blockquote>
59+
<pre>
60+
template&lt;<i>in-vector</i> InVec, class Scalar&gt;
61+
Scalar vector_two_norm(InVec v, Scalar init);
62+
template&lt;class ExecutionPolicy, <i>in-vector</i> InVec, class Scalar&gt;
63+
Scalar vector_two_norm(ExecutionPolicy&amp;&amp; exec, InVec v, Scalar init);
64+
</pre>
65+
<blockquote>
66+
<p>
67+
-1- [<i>Note 1</i>: [&hellip;] &mdash; <i>end note</i>]
68+
<p/>
69+
<ins>-?- <i>Constraints</i>: `InVec::value_type` and `Scalar` are either a floating-point type, or
70+
a specialization of `complex`.</ins>
71+
<p/>
72+
-2- <i>Mandates</i>: Let `a` be <tt><i>abs-if-needed</i>(declval&lt;typename InVec::value_type&gt;())</tt>.
73+
Then, <tt>decltype(init + a * a<ins>)</ins></tt> is convertible to `Scalar`.
74+
<p/>
75+
-3- <i>Returns</i>: The square root of the sum of the square of `init` and the squares of the
76+
absolute values of the elements of `v`.
77+
<p/>
78+
[<i>Note 2</i>: For `init` equal to zero, this is the Euclidean norm (also called 2-norm) of the vector
79+
`v`. &mdash; <i>end note</i>]
80+
<p/>
81+
-4- <i>Remarks</i>: If <del>`InVec::value_type`, and `Scalar` are all floating-point types or specializations of `complex`,
82+
and if</del> `Scalar` has higher precision than `InVec::value_type`, then intermediate terms in the sum use
83+
`Scalar`'s precision or greater.
84+
<p/>
85+
[<i>Note 3</i>: An implementation of this function for floating-point types `T` can use the `scaled_sum_of_squares`
86+
result `from vector_sum_of_squares(x, {.scaling_factor=1.0, .scaled_sum_of_squares=init})`. &mdash; <i>end note</i>]
87+
</p>
88+
</blockquote>
89+
</blockquote>
90+
91+
</li>
92+
93+
<li><p>Modify <sref ref="[linalg.algs.blas1.matfrobnorm]"/> as indicated:</p>
94+
95+
<blockquote>
96+
<pre>
97+
template&lt;<i>in-matrix</i> InMat, class Scalar&gt;
98+
Scalar matrix_frob_norm(InMat A, Scalar init);
99+
template&lt;class ExecutionPolicy, <i>in-matrix</i> InMat, class Scalar&gt;
100+
Scalar matrix_frob_norm(ExecutionPolicy&amp;&amp; exec, InMat A, Scalar init);
101+
</pre>
102+
<blockquote>
103+
<p>
104+
<ins>-?- <i>Constraints</i>: `InVec::value_type` and `Scalar` are either a floating-point type, or
105+
a specialization of `complex`.</ins>
106+
<p/>
107+
-2- <i>Mandates</i>: Let `a` be <tt><i>abs-if-needed</i>(declval&lt;typename InMat::value_type&gt;())</tt>.
108+
Then, <tt>decltype(init + a * a)</tt> is convertible to `Scalar`.
109+
<p/>
110+
-3- <i>Returns</i>: The square root of the sum of squares of `init` and the absolute values
111+
of the elements of `A`.
112+
<p/>
113+
[<i>Note 2</i>: For `init` equal to zero, this is the Frobenius norm of the matrix `A`. &mdash; <i>end note</i>]
114+
<p/>
115+
-4- <i>Remarks</i>: If <del>`InMat::value_type` and `Scalar` are all floating-point types or specializations of
116+
`complex`, and if</del> `Scalar` has higher precision than `InMat::value_type`, then intermediate terms in the
117+
sum use `Scalar`'s precision or greater.
118+
</p>
119+
<blockquote><pre>
120+
</pre></blockquote>
121+
</blockquote>
122+
</blockquote>
123+
124+
</li>
125+
126+
</ol>
60127
</resolution>
61128

62129
</issue>

0 commit comments

Comments
 (0)