|
| 1 | +<?xml version='1.0' encoding='utf-8' standalone='no'?> |
| 2 | +<!DOCTYPE issue SYSTEM "lwg-issue.dtd"> |
| 3 | + |
| 4 | +<issue num="4302" status="New"> |
| 5 | +<title>Problematic `vector_sum_of_squares` wording</title> |
| 6 | +<section><sref ref="[linalg.algs.blas1.ssq]"/></section> |
| 7 | +<submitter>Mark Hoemmen</submitter> |
| 8 | +<date>23 Jul 2025</date> |
| 9 | +<priority>99</priority> |
| 10 | + |
| 11 | +<discussion> |
| 12 | +<p> |
| 13 | +The current wording for `vector_sum_of_squares` |
| 14 | +<sref ref="[linalg.algs.blas1.ssq]"/> has two problems with its specification of the |
| 15 | +value of `result.scaling_factor`. |
| 16 | +</p> |
| 17 | +<ol> |
| 18 | +<li><p>The function permits `InVec::value_type` and `Scalar` to be any |
| 19 | +linear algebra value types. However, computing |
| 20 | +`result.scaling_factor` that satisfies both (3.1) and (3.2) requires |
| 21 | +more operations, such as division. Even if those operations are |
| 22 | +defined, they might not make `result.scaling_factor` satisfy the |
| 23 | +required properties. For example, integers have division, but integer |
| 24 | +division won't help here.</p></li> |
| 25 | +<li><p>LAPACK's xLASSQ (the algorithm to which Note 1 in |
| 26 | +<sref ref="[linalg.algs.blas1.ssq]"/> refers) changed its algorithm recently |
| 27 | +(see <a href="https://github.com/Reference-LAPACK/lapack/pull/494">Reference-LAPACK/lapack/pull/#494</a>) |
| 28 | +so that the scaling factor is no longer necessarily the maximum of the input |
| 29 | +scaling factor and the absolute value of all the input elements. It's |
| 30 | +a better algorithm and we would like to be able to use it.</p></li> |
| 31 | +</ol> |
| 32 | +<p> |
| 33 | +Problem (1) means that the current wording is broken. I suggest two |
| 34 | +different ways to fix this. |
| 35 | +</p> |
| 36 | +<ol> |
| 37 | +<li><p>Remove `vector_sum_of_squares` entirely (both overloads from |
| 38 | +<sref ref="[linalg.syn]"/>, and the entire |
| 39 | +<sref ref="[linalg.algs.blas1.ssq]"/>). That way, we |
| 40 | +won't be baking an old, less good algorithm into the Standard. Remove |
| 41 | +Note 3 from <sref ref="[linalg.algs.blas1.nrm2]"/>, which is the only |
| 42 | +other reference to `vector_sum_of_squares` in the Standard.</p></li> |
| 43 | +<li><p>Fix <sref ref="[linalg.algs.blas1.ssq]"/> by adding to the |
| 44 | +<i>Mandates</i> element (para 2) that `InVec::value_type` and `Scalar` |
| 45 | +are both floating-point types (so that we could fix this later if |
| 46 | +we want), and remove <sref ref="[linalg.algs.blas1.ssq]"/> 3.1. |
| 47 | +Optionally add <i>Recommended Practice</i>, though Note 1 already |
| 48 | +suggests the intent.</p></li> |
| 49 | +</ol> |
| 50 | +<p> |
| 51 | +I prefer just removing `vector_sum_of_squares`. Implementers who care |
| 52 | +about QoI of `vector_two_norm` should already know what to do. If |
| 53 | +somebody cares sufficiently, they can propose it back for C++29 and |
| 54 | +think about how to make it work for generic number types. |
| 55 | +</p> |
| 56 | +</discussion> |
| 57 | + |
| 58 | +<resolution> |
| 59 | +<p> |
| 60 | +This wording is relative to this |
| 61 | +<a href="https://github.com/cplusplus/draft/actions/runs/16433597877/artifacts/3583518547">CD preview draft</a>. |
| 62 | +</p> |
| 63 | + |
| 64 | +<blockquote class="note"> |
| 65 | +<p> |
| 66 | +[<i>Drafting note:</i> The wording below implements option 1 of the issue discussion] |
| 67 | +</p> |
| 68 | +</blockquote> |
| 69 | + |
| 70 | +<ol> |
| 71 | + |
| 72 | +<li><p>Modify <sref ref="[linalg.syn]"/>, header <tt><linalg></tt> synopsis, as indicated:</p> |
| 73 | + |
| 74 | +<blockquote> |
| 75 | +<pre> |
| 76 | +namespace std::linalg { |
| 77 | + […] |
| 78 | + <del>// <i><sref ref="[linalg.algs.blas1.ssq]"/>, scaled sum of squares of a vector’s elements</i> |
| 79 | + template<class Scalar> |
| 80 | + struct sum_of_squares_result { |
| 81 | + Scalar scaling_factor; |
| 82 | + }; |
| 83 | + template<<i>in-vector</i> InVec, class Scalar> |
| 84 | + sum_of_squares_result<Scalar> |
| 85 | + vector_sum_of_squares(InVec v, sum_of_squares_result<Scalar> init); |
| 86 | + template<class ExecutionPolicy, <i>in-vector</i> InVec, class Scalar> |
| 87 | + sum_of_squares_result<Scalar> |
| 88 | + vector_sum_of_squares(ExecutionPolicy&& exec, |
| 89 | + InVec v, sum_of_squares_result<Scalar> init);</del> |
| 90 | + […] |
| 91 | +} |
| 92 | +</pre> |
| 93 | +</blockquote> |
| 94 | +</li> |
| 95 | + |
| 96 | +<li><p>Delete the entire <sref ref="[linalg.algs.blas1.ssq]"/> as indicated:</p> |
| 97 | + |
| 98 | +<blockquote> |
| 99 | +<p> |
| 100 | +<del><b>29.9.13.8 Scaled sum of squares of a vector's elements [linalg.algs.blas1.ssq]</b></del> |
| 101 | +</p> |
| 102 | +<pre> |
| 103 | +<del>template<<i>in-vector</i> InVec, class Scalar> |
| 104 | + sum_of_squares_result<Scalar> vector_sum_of_squares(InVec v, sum_of_squares_result<Scalar> init); |
| 105 | +template<class ExecutionPolicy, <i>in-vector</i> InVec, class Scalar> |
| 106 | + sum_of_squares_result<Scalar> vector_sum_of_squares(ExecutionPolicy&& exec, |
| 107 | + InVec v, sum_of_squares_result<Scalar> init);</del> |
| 108 | +</pre> |
| 109 | +<blockquote> |
| 110 | +<p> |
| 111 | +<del>-1- [<i>Note 1</i>: These functions correspond to the LAPACK function xLASSQ[20]. — <i>end note</i>]</del> |
| 112 | +<p/> |
| 113 | +<del>-2- <i>Mandates</i>: <tt>decltype(<i>abs-if-needed</i>(declval<typename InVec::value_type>()))</tt> is convertible |
| 114 | +to `Scalar`.</del> |
| 115 | +<p/> |
| 116 | +<del>-3- <i>Effects</i>: Returns a value `result` such that</del> |
| 117 | +</p> |
| 118 | +<ol style="list-style-type: none"> |
| 119 | +<li><p><del>(3.1) — `result.scaling_factor` is the maximum of `init.scaling_factor` and |
| 120 | +<tt><i>abs-if-needed</i>(x[i])</tt> for all `i` in the domain of `v`; and</del></p></li> |
| 121 | +<li><p><del>(3.2) — let `s2init` be</del></p> |
| 122 | +<blockquote><pre> |
| 123 | +<del>init.scaling_factor * init.scaling_factor * init.scaled_sum_of_squares</del> |
| 124 | +</pre></blockquote> |
| 125 | +<p> |
| 126 | +<del>then `result.scaling_factor * result.scaling_factor * result.scaled_sum_of_squares` |
| 127 | +equals the sum of `s2init` and the squares of <tt><i>abs-if-needed</i>(x[i])</tt> for all `i` |
| 128 | +in the domain of `v`.</del></p></li> |
| 129 | +</ol> |
| 130 | +<p> |
| 131 | +<del>-4- <i>Remarks</i>: If `InVec::value_type`, and `Scalar` are all floating-point types or specializations of |
| 132 | +`complex`, and if `Scalar` has higher precision than `InVec::value_type`, then intermediate terms in the sum |
| 133 | +use `Scalar`'s precision or greater.</del> |
| 134 | +</p> |
| 135 | +</blockquote> |
| 136 | +</blockquote> |
| 137 | +</li> |
| 138 | + |
| 139 | +<li><p>Modify <sref ref="[linalg.algs.blas1.nrm2]"/> as indicated:</p> |
| 140 | + |
| 141 | +<blockquote> |
| 142 | +<pre> |
| 143 | +template<<i>in-vector</i> InVec, class Scalar> |
| 144 | + Scalar vector_two_norm(InVec v, Scalar init); |
| 145 | +template<class ExecutionPolicy, <i>in-vector</i> InVec, class Scalar> |
| 146 | + Scalar vector_two_norm(ExecutionPolicy&& exec, InVec v, Scalar init); |
| 147 | +</pre> |
| 148 | +<blockquote> |
| 149 | +<p> |
| 150 | +-1- [<i>Note 1</i>: […] ] |
| 151 | +<p/> |
| 152 | +-2- <i>Mandates</i>: […] |
| 153 | +<p/> |
| 154 | +-3- <i>Returns</i>: […] |
| 155 | +<p/> |
| 156 | +[<i>Note 2</i>: […] ] |
| 157 | +<p/> |
| 158 | +-4- <i>Remarks</i>: […] |
| 159 | +<p/> |
| 160 | +<del>[<i>Note 3</i>: An implementation of this function for floating-point types `T` |
| 161 | +can use the `scaled_sum_of_squares` result from |
| 162 | +`vector_sum_of_squares(x, {.scaling_factor=1.0, .scaled_sum_of_squares=init})`. |
| 163 | +— <i>end note</i>]</del> |
| 164 | +</p> |
| 165 | +</blockquote> |
| 166 | +</blockquote> |
| 167 | +</li> |
| 168 | + |
| 169 | +</ol> |
| 170 | +</resolution> |
| 171 | + |
| 172 | +</issue> |
0 commit comments