In the documentation of RawStorage::stride it was falsely claimed that
the storage of the matrix happens in a row-major fashion. In fact it is
column-major.
* fix: Normalize the column once more
The column may not be normalized if the `factor` is on a scale of 1e-40.
Possibly, f32 just runs out of precision.
There is likely a better solution to the problem.
* chore: Add test that fails before fix
* chore: add comment providing details on the householder fix.
* chore: rename regression test
---------
Co-authored-by: Sébastien Crozet <sebcrozet@dimforge.com>
The existing comparison bound of $\epsilon^2$ is improperly scaled for
testing an epsilon of the squared vector magnitude. Let $\epsilon$ be
our specified epsilon and $\delta$ be the permissible delta of the
squared magnitude. Thus, for a nearly-normalized vector, we have
$$\begin{align}
\sqrt{1 + \delta} &= 1 + \epsilon \\
\delta &= (1 + \epsilon)^2 - 1 \\
\delta &= \epsilon^2 + 2\epsilon
\text{ .}\end{align}$$
Since we only care about small epsilon, we can assume
that $\epsilon^2$ is small and just use $\delta = 2\epsilon$. And in
fact, [this is the bound used by GLM][GLM#isNormalized] (MIT license)
... except they're using `length` and not `length2` for some reason.
[GLM#isNormalized]: b06b775c1c/glm/gtx/vector_query.inl (L102)
If we stick an epsilon of `1.0e-6` into the current implementation,
this gives us a computed delta of `1.0e-12`: smaller than the `f32`
machine epsilon, and thus no different than direct float comparison
without epsilon. This also gives an effetive epsilon of `5.0e-13`;
*much* less than the intended `1.0e-6` of intended permitted slack!
By doing a bit more algebra, we can find the effective epsilon is
$\sqrt{\texttt{epsilon}^2 + 1} - 1$. This patch makes the effective
epsilon $\sqrt{2\times\texttt{epsilon} + 1} - 1$ which still isn't
*perfect*, but it's effectively linear in the domain we care about,
only really making a practical difference above an epsilon of 10%.
TL;DR: the existing `is_normalized` considers a vector normalized if
the squared magnitude is within `epsilon*epsilon` of `1`. This is wrong
and it should be testing if it's within `2*epsilon`. This PR fixes it.
For absence of doubt, a comparison epsilon of $\texttt{epsilon}^2$ is
correct when comparing squared magnitude against zero, such as when
testing if a displacement vector is nearly zero.
Lishen
Andreas Borgen Longva
Adrian H
Derek Christ
Sébastien Crozet
Vollkornaffe
tpdickso
Yotam Ofek
Joona Aalto
Moritz Hedtke
Christopher Durham
Ababwa
Benjamin Saunders
Benjamin De Roeck
Kurt Lawrence
Julian Knodt
zachs18
Bruce Mitchener
Sébastien Crozet
Rasmus Brönnegård
Alessandro Rocco Scisca
Fabio Valentini