From 5bcb880759b1177e5c00ccef8d9488386e564dd0 Mon Sep 17 00:00:00 2001 From: Andrew Melim Date: Tue, 2 Dec 2014 18:51:37 -0500 Subject: [PATCH 1/6] Issue #151 --- gtsam/nonlinear/Expression-inl.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/gtsam/nonlinear/Expression-inl.h b/gtsam/nonlinear/Expression-inl.h index 332c23ca7..1655057ca 100644 --- a/gtsam/nonlinear/Expression-inl.h +++ b/gtsam/nonlinear/Expression-inl.h @@ -560,8 +560,8 @@ struct GenerateFunctionalNode: Argument, Base { /// Given df/dT, multiply in dT/dA and continue reverse AD process // Cols is always known at compile time - template - void reverseAD4(const Eigen::Matrix & dFdT, + template + void reverseAD4(const SomeMatrix & dFdT, JacobianMap& jacobians) const { Base::Record::reverseAD4(dFdT, jacobians); This::trace.reverseAD1(dFdT * This::dTdA, jacobians); From 701dcc1c99a69e3c7bee3cbac5811c50d41be685 Mon Sep 17 00:00:00 2001 From: Andrew Melim Date: Mon, 8 Dec 2014 21:39:12 -0500 Subject: [PATCH 2/6] Correcting VLA issue, and add template specification for between Pose2 objects. More template specifcations will be needed on windows, unless another fix is found --- gtsam/nonlinear/Expression.h | 9 +++++++++ gtsam_unstable/nonlinear/expressions.h | 15 ++++++++++++++- 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/gtsam/nonlinear/Expression.h b/gtsam/nonlinear/Expression.h index c7f022724..f0c1751cf 100644 --- a/gtsam/nonlinear/Expression.h +++ b/gtsam/nonlinear/Expression.h @@ -206,11 +206,20 @@ private: // with an execution trace, made up entirely of "Record" structs, see // the FunctionalNode class in expression-inl.h size_t size = traceSize(); +#ifdef _MSC_VER + ExecutionTraceStorage* traceStorage = new ExecutionTraceStorage[size]; + ExecutionTrace trace; + T value(traceExecution(values, trace, traceStorage)); + trace.startReverseAD1(jacobians); + delete[] traceStorage; + return value; +#else ExecutionTraceStorage traceStorage[size]; ExecutionTrace trace; T value(traceExecution(values, trace, traceStorage)); trace.startReverseAD1(jacobians); return value; +#endif } // be very selective on who can access these private methods: diff --git a/gtsam_unstable/nonlinear/expressions.h b/gtsam_unstable/nonlinear/expressions.h index 2490100d6..5ac229b7a 100644 --- a/gtsam_unstable/nonlinear/expressions.h +++ b/gtsam_unstable/nonlinear/expressions.h @@ -8,13 +8,26 @@ #pragma once #include +#include +#include #include namespace gtsam { +// 2D Geometry + typedef Expression Pose2_; + + Pose2_ between(const Pose2_& x, const Pose2_& p) { + Pose2(Pose2::*transform)(const Pose2& p, + boost::optional H1, + boost::optional H2) const = &Pose2::between; + + return Pose2_(x, transform, p); + } + // Generics -template +template Expression between(const Expression& t1, const Expression& t2) { return Expression(t1, &T::between, t2); } From cea76a28481bbcf624f2b77e0a98e7e7a2ced57c Mon Sep 17 00:00:00 2001 From: Andrew Melim Date: Mon, 8 Dec 2014 21:48:59 -0500 Subject: [PATCH 3/6] More template handholding for windows --- gtsam_unstable/slam/expressions.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/gtsam_unstable/slam/expressions.h b/gtsam_unstable/slam/expressions.h index 009be46a1..55f9093ce 100644 --- a/gtsam_unstable/slam/expressions.h +++ b/gtsam_unstable/slam/expressions.h @@ -62,7 +62,11 @@ Point2_ project3(const Pose3_& x, const Point3_& p, const Cal3_S2_& K) { template Point2_ uncalibrate(const Expression& K, const Point2_& xy_hat) { - return Point2_(K, &CAL::uncalibrate, xy_hat); + Point2(CAL::*uncal)(const Point2& p, + boost::optional Dpose, + boost::optional Dpoint) const = &CAL::uncalibrate; + + return Point2_(K, uncal, xy_hat); } } // \namespace gtsam From 90676199dd564914e84c550ee8aaf4f5dfde3233 Mon Sep 17 00:00:00 2001 From: Andrew Melim Date: Tue, 9 Dec 2014 11:49:13 -0500 Subject: [PATCH 4/6] Adding comment relating to issue and move around fix preprocessor locations --- gtsam/nonlinear/Expression.h | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/gtsam/nonlinear/Expression.h b/gtsam/nonlinear/Expression.h index 980c47cd1..e63fbc686 100644 --- a/gtsam/nonlinear/Expression.h +++ b/gtsam/nonlinear/Expression.h @@ -206,20 +206,25 @@ private: // with an execution trace, made up entirely of "Record" structs, see // the FunctionalNode class in expression-inl.h size_t size = traceSize(); + + // Windows does not support variable length arrays, so memory must be dynamically + // allocated on Visual Studio. For more information see the issue below + // https://bitbucket.org/gtborg/gtsam/issue/178/vlas-unsupported-in-visual-studio #ifdef _MSC_VER ExecutionTraceStorage* traceStorage = new ExecutionTraceStorage[size]; - ExecutionTrace trace; - T value(traceExecution(values, trace, traceStorage)); - trace.startReverseAD1(jacobians); - delete[] traceStorage; - return value; #else ExecutionTraceStorage traceStorage[size]; +#endif + ExecutionTrace trace; T value(traceExecution(values, trace, traceStorage)); trace.startReverseAD1(jacobians); - return value; + +#ifdef _MSC_VER + delete[] traceStorage; #endif + + return value; } // be very selective on who can access these private methods: From 137ea642001531241461c72fe79daf0e5b8ce1eb Mon Sep 17 00:00:00 2001 From: Andrew Melim Date: Tue, 9 Dec 2014 11:49:47 -0500 Subject: [PATCH 5/6] Updating with OptionalJacobian --- gtsam_unstable/nonlinear/expressions.h | 4 ++-- gtsam_unstable/slam/expressions.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/gtsam_unstable/nonlinear/expressions.h b/gtsam_unstable/nonlinear/expressions.h index 5ac229b7a..960371ed2 100644 --- a/gtsam_unstable/nonlinear/expressions.h +++ b/gtsam_unstable/nonlinear/expressions.h @@ -19,8 +19,8 @@ namespace gtsam { Pose2_ between(const Pose2_& x, const Pose2_& p) { Pose2(Pose2::*transform)(const Pose2& p, - boost::optional H1, - boost::optional H2) const = &Pose2::between; + OptionalJacobian<3, 3> H1, + OptionalJacobian<3, 3> H2) const = &Pose2::between; return Pose2_(x, transform, p); } diff --git a/gtsam_unstable/slam/expressions.h b/gtsam_unstable/slam/expressions.h index ed53d7465..a45e8f1b4 100644 --- a/gtsam_unstable/slam/expressions.h +++ b/gtsam_unstable/slam/expressions.h @@ -60,8 +60,8 @@ Point2_ project3(const Pose3_& x, const Point3_& p, const Cal3_S2_& K) { template Point2_ uncalibrate(const Expression& K, const Point2_& xy_hat) { Point2(CAL::*uncal)(const Point2& p, - boost::optional Dpose, - boost::optional Dpoint) const = &CAL::uncalibrate; + OptionalJacobian<2, 5> Dpose, + OptionalJacobian<2, 2> Dpoint) const = &CAL::uncalibrate; return Point2_(K, uncal, xy_hat); } From 1e778cf77b2621f84cd830abd0a29e74d25201b3 Mon Sep 17 00:00:00 2001 From: Andrew Melim Date: Tue, 9 Dec 2014 11:53:35 -0500 Subject: [PATCH 6/6] No longer need to cast function pointers for expressions on Windows, with Optional Jacobian --- gtsam_unstable/nonlinear/expressions.h | 14 -------------- gtsam_unstable/slam/expressions.h | 17 +++-------------- 2 files changed, 3 insertions(+), 28 deletions(-) diff --git a/gtsam_unstable/nonlinear/expressions.h b/gtsam_unstable/nonlinear/expressions.h index 960371ed2..18d0d5d8f 100644 --- a/gtsam_unstable/nonlinear/expressions.h +++ b/gtsam_unstable/nonlinear/expressions.h @@ -8,25 +8,11 @@ #pragma once #include -#include -#include #include namespace gtsam { -// 2D Geometry - typedef Expression Pose2_; - - Pose2_ between(const Pose2_& x, const Pose2_& p) { - Pose2(Pose2::*transform)(const Pose2& p, - OptionalJacobian<3, 3> H1, - OptionalJacobian<3, 3> H2) const = &Pose2::between; - - return Pose2_(x, transform, p); - } - // Generics - template Expression between(const Expression& t1, const Expression& t2) { return Expression(t1, &T::between, t2); diff --git a/gtsam_unstable/slam/expressions.h b/gtsam_unstable/slam/expressions.h index a45e8f1b4..7badc9dd7 100644 --- a/gtsam_unstable/slam/expressions.h +++ b/gtsam_unstable/slam/expressions.h @@ -20,10 +20,7 @@ typedef Expression Rot2_; typedef Expression Pose2_; Point2_ transform_to(const Pose2_& x, const Point2_& p) { - Point2 (Pose2::*transform)(const Point2& p, OptionalJacobian<2, 3> H1, - OptionalJacobian<2, 2> H2) const = &Pose2::transform_to; - - return Point2_(x, transform, p); + return Point2_(x, &Pose2::transform_to, p); } // 3D Geometry @@ -33,11 +30,7 @@ typedef Expression Rot3_; typedef Expression Pose3_; Point3_ transform_to(const Pose3_& x, const Point3_& p) { - - Point3 (Pose3::*transform)(const Point3& p, OptionalJacobian<3, 6> Dpose, - OptionalJacobian<3, 3> Dpoint) const = &Pose3::transform_to; - - return Point3_(x, transform, p); + return Point3_(x, &Pose3::transform_to, p); } // Projection @@ -59,11 +52,7 @@ Point2_ project3(const Pose3_& x, const Point3_& p, const Cal3_S2_& K) { template Point2_ uncalibrate(const Expression& K, const Point2_& xy_hat) { - Point2(CAL::*uncal)(const Point2& p, - OptionalJacobian<2, 5> Dpose, - OptionalJacobian<2, 2> Dpoint) const = &CAL::uncalibrate; - - return Point2_(K, uncal, xy_hat); + return Point2_(K, &CAL::uncalibrate, xy_hat); } } // \namespace gtsam