added a test for mul_tr to verify that we get m.mul_tr(v) is m*v.transpose(). also changed the name of the previous test for tr_mul to clarify which side of the multiplication is being transposed.
Fixed a (presumed) typo in documentation in matrix_view.rs.
Previously it says fixed_view and fixed_slice returns a matrix with `(RView, CView)` consecutive components. As per-input, and after re-reading the documentation and source code. Seems like RView and CView should be used instead of RView::dim() and CViem::dim().
Fixed a (presumed) typo in documentation in matrix_view.rs.
Previously it says fixed_view and fixed_slice returns a matrix with `(R::dim(), CView::dim())` consecutive components. I just changed R to RView, because presumably this is a typo.
* 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.
fangs124
Fangs
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