Browse Source

Math: remove std::tuple usage from Algorithms::svd().

There's Containers::Triple now for this instead. Printing a message to
Error in case of a failure might have made sense back in 2010, but now
it absolutely doesn't, so it's additionally wrapped in an Optional now.

Also looks like the actual use without "convenient" std::tie() is a lot
less verbose. Haha.
pull/168/head
Vladimír Vondruš 3 years ago
parent
commit
162f4fd6ec
  1. 5
      doc/changelog.dox
  2. 8
      doc/snippets/MagnumMathAlgorithms.cpp
  3. 12
      src/Magnum/Math/Algorithms/Svd.h
  4. 44
      src/Magnum/Math/Algorithms/Test/SvdTest.cpp
  5. 2
      src/singles/MagnumMath.hpp

5
doc/changelog.dox

@ -1480,6 +1480,11 @@ See also:
for implicit conversions from/to a @ref std::string and/or @ref std::pair, for implicit conversions from/to a @ref std::string and/or @ref std::pair,
but in some cases you may be forced to change the code that uses those but in some cases you may be forced to change the code that uses those
APIs. APIs.
- @ref Math::Algorithms::svd() that used to return a @ref std::tuple and
printed a message to @relativeref{Magnum,Error} if it didn't converge now
returns a @ref Containers::Tuple wrapped in an @ref Containers::Optional.
As the optional makes this a breaking change, no backwards compatibility
header for tuple conversion is included.
- @ref Image, @ref ImageView and @ref Trade::ImageData now look for a - @ref Image, @ref ImageView and @ref Trade::ImageData now look for a
@cpp pixelFormatSize() @ce API via ADL instead of @cpp pixelSize() @ce. In @cpp pixelFormatSize() @ce API via ADL instead of @cpp pixelSize() @ce. In
case you were passing a custom pixel format enum to the image classes, you case you were passing a custom pixel format enum to the image classes, you

8
doc/snippets/MagnumMathAlgorithms.cpp

@ -62,11 +62,9 @@ enum: std::size_t { cols = 3, rows = 4 };
/* [svd] */ /* [svd] */
Math::RectangularMatrix<cols, rows, Double> m; Math::RectangularMatrix<cols, rows, Double> m;
Math::RectangularMatrix<cols, rows, Double> uPart; auto svd = Math::Algorithms::svd(m);
Math::Vector<cols, Double> wDiagonal; Math::RectangularMatrix<cols, rows, Double> uPart = svd->first();
Math::Matrix<cols, Double> v; Math::Vector<cols, Double> wDiagonal = svd->second();
std::tie(uPart, wDiagonal, v) = Math::Algorithms::svd(m);
// Extend U // Extend U
Math::Matrix<rows, Double> u{Math::ZeroInit}; Math::Matrix<rows, Double> u{Math::ZeroInit};

12
src/Magnum/Math/Algorithms/Svd.h

@ -29,7 +29,8 @@
* @brief Function @ref Magnum::Math::Algorithms::svd() * @brief Function @ref Magnum::Math::Algorithms::svd()
*/ */
#include <tuple> #include <Corrade/Containers/Optional.h>
#include <Corrade/Containers/Triple.h>
#include "Magnum/Math/Functions.h" #include "Magnum/Math/Functions.h"
#include "Magnum/Math/Matrix.h" #include "Magnum/Math/Matrix.h"
@ -65,7 +66,7 @@ on given matrix where @p rows >= `cols`: @f[
Returns first @p cols column vectors of @f$ \boldsymbol{U} @f$, diagonal of Returns first @p cols column vectors of @f$ \boldsymbol{U} @f$, diagonal of
@f$ \boldsymbol{\Sigma} @f$ and non-transposed @f$ \boldsymbol{V} @f$. If the @f$ \boldsymbol{\Sigma} @f$ and non-transposed @f$ \boldsymbol{V} @f$. If the
solution doesn't converge, returns zero matrices. solution doesn't converge, returns @ref Containers::NullOpt.
Full @f$ \boldsymbol{U} @f$, @f$ \boldsymbol{\Sigma} @f$ matrices and original Full @f$ \boldsymbol{U} @f$, @f$ \boldsymbol{\Sigma} @f$ matrices and original
@f$ \boldsymbol{M} @f$ matrix can be reconstructed from the values as @f$ \boldsymbol{M} @f$ matrix can be reconstructed from the values as
@ -81,7 +82,7 @@ for an example. Implementation based on *Golub, G. H.; Reinsch, C. (1970).
@see @ref qr(), @ref Matrix3::rotationShear(), @ref Matrix4::rotationShear() @see @ref qr(), @ref Matrix3::rotationShear(), @ref Matrix4::rotationShear()
*/ */
/* The matrix is passed by value because it is changed inside */ /* The matrix is passed by value because it is changed inside */
template<std::size_t cols, std::size_t rows, class T> std::tuple<RectangularMatrix<cols, rows, T>, Vector<cols, T>, Matrix<cols, T>> svd(RectangularMatrix<cols, rows, T> m) { template<std::size_t cols, std::size_t rows, class T> Containers::Optional<Containers::Triple<RectangularMatrix<cols, rows, T>, Vector<cols, T>, Matrix<cols, T>>> svd(RectangularMatrix<cols, rows, T> m) {
static_assert(rows >= cols, "Unsupported matrix aspect ratio"); static_assert(rows >= cols, "Unsupported matrix aspect ratio");
static_assert(T(1)+TypeTraits<T>::epsilon() > T(1), "Epsilon too small"); static_assert(T(1)+TypeTraits<T>::epsilon() > T(1), "Epsilon too small");
constexpr T tol = Implementation::smallestDelta<T>()/TypeTraits<T>::epsilon(); constexpr T tol = Implementation::smallestDelta<T>()/TypeTraits<T>::epsilon();
@ -255,8 +256,7 @@ template<std::size_t cols, std::size_t rows, class T> std::tuple<RectangularMatr
/* Exceeded iteration count, done */ /* Exceeded iteration count, done */
} else if(iteration >= maxIterations-1) { } else if(iteration >= maxIterations-1) {
Error{} << "Magnum::Math::Algorithms::svd(): no convergence"; return {};
return std::make_tuple(RectangularMatrix<cols, rows, T>{}, Vector<cols, T>{}, Matrix<cols, T>{ZeroInit});
} }
/* Shift from bottom 2x2 minor */ /* Shift from bottom 2x2 minor */
@ -316,7 +316,7 @@ template<std::size_t cols, std::size_t rows, class T> std::tuple<RectangularMatr
} }
} }
return std::make_tuple(m, q, v); return Containers::triple(m, q, v);
} }
}}} }}}

44
src/Magnum/Math/Algorithms/Test/SvdTest.cpp

@ -62,37 +62,35 @@ template<class T> void SvdTest::test() {
Vector8<T>{T{ 7}, T{ 8}, T{ 3}, T{ 4}, T{ 4}, T{-1}, T{ 1}, T{ 2}}}; Vector8<T>{T{ 7}, T{ 8}, T{ 3}, T{ 4}, T{ 4}, T{-1}, T{ 1}, T{ 2}}};
const Vector5<T> expected(std::sqrt(T{1248}), T{0}, T{20}, std::sqrt(T{384}), T{0}); const Vector5<T> expected(std::sqrt(T{1248}), T{0}, T{20}, std::sqrt(T{384}), T{0});
Matrix5x8<T> u{Magnum::NoInit}; Containers::Optional<Containers::Triple<Matrix5x8<T>, Vector5<T>, Matrix5<T>>> uwv = Algorithms::svd(a);
Vector5<T> w{Magnum::NoInit}; CORRADE_VERIFY(uwv);
Matrix5<T> v{Magnum::NoInit};
std::tie(u, w, v) = Algorithms::svd(a);
/* Test composition */ /* Test composition */
Matrix8<T> u2{u[0], u[1], u[2], u[3], u[4], Vector8<T>{}, Vector8<T>{}, Vector8<T>{}}; Matrix8<T> u2{uwv->first()[0], uwv->first()[1], uwv->first()[2], uwv->first()[3], uwv->first()[4], Vector8<T>{}, Vector8<T>{}, Vector8<T>{}};
Matrix5x8<T> w2 = Matrix5x8<T>::fromDiagonal(w); Matrix5x8<T> w2 = Matrix5x8<T>::fromDiagonal(uwv->second());
{ {
#ifdef CORRADE_TARGET_EMSCRIPTEN #ifdef CORRADE_TARGET_EMSCRIPTEN
CORRADE_EXPECT_FAIL_IF((std::is_same<T, Double>::value) && u2*w2*v.transposed() != a, CORRADE_EXPECT_FAIL_IF((std::is_same<T, Double>::value) && u2*w2*uwv->third().transposed() != a,
"Some strange problems with Double on recent Emscripten versions " "Some strange problems with Double on recent Emscripten versions "
"(1.36.5 worked fine, 1.37.1 works fine on larger optimization " "(1.36.5 worked fine, 1.37.1 works fine on larger optimization "
"levels, not on -O1)."); "levels, not on -O1).");
#endif #endif
CORRADE_COMPARE(u2*w2*v.transposed(), a); CORRADE_COMPARE(u2*w2*uwv->third().transposed(), a);
} }
/* Test that V is unitary */ /* Test that V is unitary */
CORRADE_COMPARE(v*v.transposed(), Matrix5<T>{IdentityInit}); CORRADE_COMPARE(uwv->third()*uwv->third().transposed(), Matrix5<T>{IdentityInit});
CORRADE_COMPARE(v.transposed()*v, Matrix5<T>{IdentityInit}); CORRADE_COMPARE(uwv->third().transposed()*uwv->third(), Matrix5<T>{IdentityInit});
/* Test W */ /* Test W */
{ {
#ifdef CORRADE_TARGET_EMSCRIPTEN #ifdef CORRADE_TARGET_EMSCRIPTEN
CORRADE_EXPECT_FAIL_IF((std::is_same<T, Double>::value && w != expected), CORRADE_EXPECT_FAIL_IF((std::is_same<T, Double>::value && uwv->second() != expected),
"Some strange problems with Double on recent Emscripten versions " "Some strange problems with Double on recent Emscripten versions "
"(1.36.5 worked fine, 1.37.1 worked fine on larger optimization " "(1.36.5 worked fine, 1.37.1 worked fine on larger optimization "
"levels, not on -O1, 1.37.5 works fine again)."); "levels, not on -O1, 1.37.5 works fine again).");
#endif #endif
CORRADE_COMPARE(w, expected); CORRADE_COMPARE(uwv->second(), expected);
} }
} }
@ -105,17 +103,14 @@ void SvdTest::decomposeRotationScaling() {
Matrix4 a = Matrix4::rotationZ(35.0_degf)*Matrix4::scaling({1.5f, 2.0f, 1.0f}); Matrix4 a = Matrix4::rotationZ(35.0_degf)*Matrix4::scaling({1.5f, 2.0f, 1.0f});
Matrix3x3 u{Magnum::NoInit}; Containers::Triple<Matrix3x3, Vector3, Matrix3x3> uwv{*Algorithms::svd(a.rotationScaling())};
Vector3 w{Magnum::NoInit};
Matrix3x3 v{Magnum::NoInit};
std::tie(u, w, v) = Algorithms::svd(a.rotationScaling());
CORRADE_COMPARE(u*Matrix3x3::fromDiagonal(w)*v.transposed(), a.rotationScaling()); CORRADE_COMPARE(uwv.first()*Matrix3x3::fromDiagonal(uwv.second())*uwv.third().transposed(), a.rotationScaling());
/* V contains flipped signs for the whole matrix, use it to fix the /* V contains flipped signs for the whole matrix, use it to fix the
signs for U */ signs for U */
CORRADE_COMPARE(w, (Vector3{1.5f, 2.0f, 1.0f})); CORRADE_COMPARE(uwv.second(), (Vector3{1.5f, 2.0f, 1.0f}));
CORRADE_COMPARE(Matrix4::from(u*v.transposed(), {}), Matrix4::rotationZ(35.0_degf)); CORRADE_COMPARE(Matrix4::from(uwv.first()*uwv.third().transposed(), {}), Matrix4::rotationZ(35.0_degf));
} }
void SvdTest::decomposeRotationShear() { void SvdTest::decomposeRotationShear() {
@ -128,17 +123,14 @@ void SvdTest::decomposeRotationShear() {
/* Like above, but with order flipped, which results in a shear */ /* Like above, but with order flipped, which results in a shear */
Matrix4 a = Matrix4::scaling({1.5f, 2.0f, 1.0f})*Matrix4::rotationZ(35.0_degf); Matrix4 a = Matrix4::scaling({1.5f, 2.0f, 1.0f})*Matrix4::rotationZ(35.0_degf);
Matrix3x3 u{Magnum::NoInit}; Containers::Triple<Matrix3x3, Vector3, Matrix3x3> uwv{*Algorithms::svd(a.rotationScaling())};
Vector3 w{Magnum::NoInit};
Matrix3x3 v{Magnum::NoInit};
std::tie(u, w, v) = Algorithms::svd(a.rotationScaling());
CORRADE_COMPARE(u*Matrix3x3::fromDiagonal(w)*v.transposed(), a.rotationScaling()); CORRADE_COMPARE(uwv.first()*Matrix3x3::fromDiagonal(uwv.second())*uwv.third().transposed(), a.rotationScaling());
/* U contains a flipped sign for Z, use it to remove the sign from the /* U contains a flipped sign for Z, use it to remove the sign from the
transposed rotation matrix V */ transposed rotation matrix V */
CORRADE_COMPARE(w, (Vector3{1.5f, 2.0f, 1.0f})); CORRADE_COMPARE(uwv.second(), (Vector3{1.5f, 2.0f, 1.0f}));
CORRADE_COMPARE(Matrix4::from(u*v.transposed(), {}), Matrix4::rotationZ(35.0_degf)); CORRADE_COMPARE(Matrix4::from(uwv.first()*uwv.third().transposed(), {}), Matrix4::rotationZ(35.0_degf));
} }
}}}}} }}}}}

2
src/singles/MagnumMath.hpp

@ -393,7 +393,7 @@ typedef Math::Frustum<Double> Frustumd;
#include "Magnum/Math/Algorithms/GramSchmidt.h" #include "Magnum/Math/Algorithms/GramSchmidt.h"
#include "Magnum/Math/Algorithms/KahanSum.h" #include "Magnum/Math/Algorithms/KahanSum.h"
#include "Magnum/Math/Algorithms/Qr.h" #include "Magnum/Math/Algorithms/Qr.h"
//#include "Magnum/Math/Algorithms/Svd.h" // TODO: uses <tuple> //#include "Magnum/Math/Algorithms/Svd.h" // TODO needs Optional and Triple
#ifdef MAGNUM_MATH_GLM_INTEGRATION #ifdef MAGNUM_MATH_GLM_INTEGRATION
// {{includes}} // {{includes}}
#include "Magnum/GlmIntegration/Integration.h" #include "Magnum/GlmIntegration/Integration.h"

Loading…
Cancel
Save