From 4966f5a94202a40ce2cd4981d7372165fdeb3664 Mon Sep 17 00:00:00 2001 From: HannesSommer Date: Sat, 15 Nov 2014 09:07:30 +0100 Subject: [PATCH 01/11] mini cleanup and improve comment TODO --- gtsam_unstable/nonlinear/Expression-inl.h | 4 ++-- gtsam_unstable/nonlinear/Expression.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gtsam_unstable/nonlinear/Expression-inl.h b/gtsam_unstable/nonlinear/Expression-inl.h index 40fc54751..d7db4f30c 100644 --- a/gtsam_unstable/nonlinear/Expression-inl.h +++ b/gtsam_unstable/nonlinear/Expression-inl.h @@ -733,8 +733,8 @@ public: virtual T value(const Values& values) const { using boost::none; return function_(this->template expression()->value(values), - this->template expression()->value(values), - none, none); + this->template expression()->value(values), + none, none); } /// Construct an execution trace for reverse AD diff --git a/gtsam_unstable/nonlinear/Expression.h b/gtsam_unstable/nonlinear/Expression.h index fc1efeb87..e8bd8bbe7 100644 --- a/gtsam_unstable/nonlinear/Expression.h +++ b/gtsam_unstable/nonlinear/Expression.h @@ -116,7 +116,7 @@ public: return root_->traceSize(); } - /// trace execution, very unsafe, for testing purposes only + /// trace execution, very unsafe, for testing purposes only //TODO this is not only used for testing, but in value() below! T traceExecution(const Values& values, ExecutionTrace& trace, char* raw) const { return root_->traceExecution(values, trace, raw); From fb24ab586e7dd4c9d83070b626132903b5428711 Mon Sep 17 00:00:00 2001 From: HannesSommer Date: Mon, 17 Nov 2014 10:06:00 +0100 Subject: [PATCH 02/11] introduced a MaxVirtualStaticRows compile time constant and realized as many static rows specific virtual reverseAD methods in the CallRecord interface to speedup the Jacobian evaluatio. --- gtsam_unstable/nonlinear/Expression-inl.h | 195 +++++++++++++++------- 1 file changed, 133 insertions(+), 62 deletions(-) diff --git a/gtsam_unstable/nonlinear/Expression-inl.h b/gtsam_unstable/nonlinear/Expression-inl.h index d7db4f30c..63b159e05 100644 --- a/gtsam_unstable/nonlinear/Expression-inl.h +++ b/gtsam_unstable/nonlinear/Expression-inl.h @@ -72,27 +72,119 @@ public: }; //----------------------------------------------------------------------------- +/** + * MaxVirtualStaticRows defines how many separate virtual reverseAD with specific + * static rows (1..MaxVirtualStaticRows) methods will be part of the CallRecord interface. + */ +const int MaxVirtualStaticRows = 4; + +namespace internal { +/** + * ConvertToDynamicIf converts to a dense matrix with dynamic rows iff ConvertToDynamicRows (colums stay as they are) otherwise + * it just passes dense Eigen matrices through. + */ +template +struct ConvertToDynamicRowsIf { + template + static Eigen::Matrix convert(const Eigen::MatrixBase & x){ + return x; + } +}; +template <> +struct ConvertToDynamicRowsIf { + template + static const Eigen::Matrix & convert(const Eigen::Matrix & x){ + return x; + } +}; + +/** + * Recursive definition of an interface having several purely + * virtual _reverseAD(const Eigen::Matrix &, JacobianMap&) + * with Rows in 1..MaxSupportedStaticRows + */ +template +struct ReverseADInterface : public ReverseADInterface < MaxSupportedStaticRows - 1, Cols> { + protected: + using ReverseADInterface < MaxSupportedStaticRows - 1, Cols>::_reverseAD; + virtual void _reverseAD(const Eigen::Matrix & dFdT, JacobianMap& jacobians) const = 0; +}; + +template +struct ReverseADInterface<0, Cols> { + protected: + void _reverseAD(){} //dummy to allow the using directive in the template without failing for MaxSupportedStaticRows == 1. +}; +} + /** * The CallRecord class stores the Jacobians of applying a function * with respect to each of its arguments. It also stores an executation trace * (defined below) for each of its arguments. * - * It is sub-classed in the function-style ExpressionNode sub-classes below. + * It is implemented in the function-style ExpressionNode's nested Record class below. */ -template -struct CallRecord { - static size_t const N = 0; - virtual void print(const std::string& indent) const { +template +struct CallRecord : private internal::ReverseADInterface { + inline void print(const std::string& indent) const { + _print(indent); } - virtual void startReverseAD(JacobianMap& jacobians) const { + inline void startReverseAD(JacobianMap& jacobians) const { + _startReverseAD(jacobians); } - virtual void reverseAD(const Matrix& dFdT, JacobianMap& jacobians) const { + template + inline void reverseAD(const Eigen::Matrix & dFdT, JacobianMap& jacobians) const{ + _reverseAD(internal::ConvertToDynamicRowsIf<(Rows > MaxVirtualStaticRows)>::convert(dFdT), jacobians); } - typedef Eigen::Matrix Jacobian2T; - virtual void reverseAD2(const Jacobian2T& dFdT, - JacobianMap& jacobians) const { + virtual ~CallRecord() { + } + private: + using internal::ReverseADInterface::_reverseAD; + virtual void _print(const std::string& indent) const = 0; + virtual void _startReverseAD(JacobianMap& jacobians) const = 0; + virtual void _reverseAD(const Eigen::Matrix & dFdT, JacobianMap& jacobians) const = 0; + virtual void _reverseAD(const Matrix & dFdT, JacobianMap& jacobians) const = 0; +}; + +namespace internal { +/** + * ReverseADImplementor is a utility class used by CallRecordImplementor to implementing the recursive CallRecord interface. + */ +template +struct ReverseADImplementor : ReverseADImplementor { + protected: + virtual void _reverseAD(const Eigen::Matrix & dFdT, JacobianMap& jacobians) const { + static_cast(this)->reverseAD(dFdT, jacobians); } }; +template +struct ReverseADImplementor : CallRecord { +}; + +/** + * The CallRecordImplementor implements the CallRecord interface for a Derived class by + * delegating to its corresponding (templated) non-virtual methods. + */ +template +struct CallRecordImplementor : public ReverseADImplementor { + private: + const Derived & derived() const { + return static_cast(*this); + } + virtual void _print(const std::string& indent) const { + derived().print(indent); + } + virtual void _startReverseAD(JacobianMap& jacobians) const { + derived().startReverseAD(jacobians); + } + virtual void _reverseAD(const Eigen::Matrix & dFdT, JacobianMap& jacobians) const { + derived().reverseAD(dFdT, jacobians); + } + virtual void _reverseAD(const Matrix & dFdT, JacobianMap& jacobians) const { + derived().reverseAD(dFdT, jacobians); + } +}; +} //----------------------------------------------------------------------------- /// Handle Leaf Case: reverseAD ends here, by writing a matrix into Jacobians @@ -101,10 +193,10 @@ void handleLeafCase(const Eigen::Matrix& dTdA, JacobianMap& jacobians, Key key) { jacobians(key).block(0, 0) += dTdA; // block makes HUGE difference } -/// Handle Leaf Case for Dynamic Matrix type (slower) -template<> +/// Handle Leaf Case for Dynamic ROWS Matrix type (slower) +template inline void handleLeafCase( - const Eigen::Matrix& dTdA, + const Eigen::Matrix& dTdA, JacobianMap& jacobians, Key key) { jacobians(key) += dTdA; } @@ -193,45 +285,18 @@ public: content.ptr->startReverseAD(jacobians); } // Either add to Jacobians (Leaf) or propagate (Function) - void reverseAD(const Matrix& dTdA, JacobianMap& jacobians) const { + template + void reverseAD(const Eigen::MatrixBase & dTdA, JacobianMap& jacobians) const { if (kind == Leaf) - handleLeafCase(dTdA, jacobians, content.key); + handleLeafCase(dTdA.eval(), jacobians, content.key); else if (kind == Function) - content.ptr->reverseAD(dTdA, jacobians); - } - // Either add to Jacobians (Leaf) or propagate (Function) - typedef Eigen::Matrix Jacobian2T; - void reverseAD2(const Jacobian2T& dTdA, JacobianMap& jacobians) const { - if (kind == Leaf) - handleLeafCase(dTdA, jacobians, content.key); - else if (kind == Function) - content.ptr->reverseAD2(dTdA, jacobians); + content.ptr->reverseAD(dTdA.eval(), jacobians); } /// Define type so we can apply it as a meta-function typedef ExecutionTrace type; }; -/// Primary template calls the generic Matrix reverseAD pipeline -template -struct Select { - typedef Eigen::Matrix::value> Jacobian; - static void reverseAD(const ExecutionTrace& trace, const Jacobian& dTdA, - JacobianMap& jacobians) { - trace.reverseAD(dTdA, jacobians); - } -}; - -/// Partially specialized template calls the 2-dimensional output version -template -struct Select<2, A> { - typedef Eigen::Matrix::value> Jacobian; - static void reverseAD(const ExecutionTrace& trace, const Jacobian& dTdA, - JacobianMap& jacobians) { - trace.reverseAD2(dTdA, jacobians); - } -}; - //----------------------------------------------------------------------------- /** * Expression node. The superclass for objects that do the heavy lifting @@ -479,8 +544,15 @@ template struct FunctionalBase: ExpressionNode { static size_t const N = 0; // number of arguments - typedef CallRecord::value> Record; - + struct Record { + void print(const std::string& indent) const { + } + void startReverseAD(JacobianMap& jacobians) const { + } + template + void reverseAD(const SomeMatrix & dFdT, JacobianMap& jacobians) const { + } + }; /// Construct an execution trace for reverse AD void trace(const Values& values, Record* record, char*& raw) const { // base case: does not do anything @@ -539,7 +611,7 @@ struct GenerateFunctionalNode: Argument, Base { typedef JacobianTrace This; /// Print to std::cout - virtual void print(const std::string& indent) const { + void print(const std::string& indent) const { Base::Record::print(indent); static const Eigen::IOFormat matlab(0, 1, " ", "; ", "", "", "[", "]"); std::cout << This::dTdA.format(matlab) << std::endl; @@ -547,25 +619,17 @@ struct GenerateFunctionalNode: Argument, Base { } /// Start the reverse AD process - virtual void startReverseAD(JacobianMap& jacobians) const { + void startReverseAD(JacobianMap& jacobians) const { Base::Record::startReverseAD(jacobians); - Select::value, A>::reverseAD(This::trace, This::dTdA, - jacobians); + This::trace.reverseAD(This::dTdA, jacobians); } /// Given df/dT, multiply in dT/dA and continue reverse AD process - virtual void reverseAD(const Matrix& dFdT, JacobianMap& jacobians) const { + template + void reverseAD(const Eigen::Matrix & dFdT, JacobianMap& jacobians) const { Base::Record::reverseAD(dFdT, jacobians); This::trace.reverseAD(dFdT * This::dTdA, jacobians); } - - /// Version specialized to 2-dimensional output - typedef Eigen::Matrix::value> Jacobian2T; - virtual void reverseAD2(const Jacobian2T& dFdT, - JacobianMap& jacobians) const { - Base::Record::reverseAD2(dFdT, jacobians); - This::trace.reverseAD2(dFdT * This::dTdA, jacobians); - } }; /// Construct an execution trace for reverse AD @@ -619,8 +683,16 @@ struct FunctionalNode { return static_cast const &>(*this).expression; } - /// Provide convenience access to Record storage - struct Record: public Base::Record { + /// Provide convenience access to Record storage and implement + /// the virtual function based interface of CallRecord using the CallRecordImplementor + struct Record: + public internal::CallRecordImplementor::value>, + public Base::Record { + using Base::Record::print; + using Base::Record::startReverseAD; + using Base::Record::reverseAD; + + virtual ~Record(){} /// Access Value template @@ -633,7 +705,6 @@ struct FunctionalNode { typename Jacobian::type& jacobian() { return static_cast&>(*this).dTdA; } - }; /// Construct an execution trace for reverse AD From 2983cf33a6c2aa3f8f3b9d24273fac600ea65fbc Mon Sep 17 00:00:00 2001 From: dellaert Date: Fri, 21 Nov 2014 15:48:02 +0100 Subject: [PATCH 03/11] Created CallRecord header --- .cproject | 106 +++++----- gtsam_unstable/nonlinear/CallRecord.h | 181 ++++++++++++++++++ .../nonlinear/tests/testCallRecord.cpp | 55 ++++++ 3 files changed, 298 insertions(+), 44 deletions(-) create mode 100644 gtsam_unstable/nonlinear/CallRecord.h create mode 100644 gtsam_unstable/nonlinear/tests/testCallRecord.cpp diff --git a/.cproject b/.cproject index 412359938..52c627f5c 100644 --- a/.cproject +++ b/.cproject @@ -600,6 +600,7 @@ make + tests/testBayesTree.run true false @@ -607,6 +608,7 @@ make + testBinaryBayesNet.run true false @@ -654,6 +656,7 @@ make + testSymbolicBayesNet.run true false @@ -661,6 +664,7 @@ make + tests/testSymbolicFactor.run true false @@ -668,6 +672,7 @@ make + testSymbolicFactorGraph.run true false @@ -683,6 +688,7 @@ make + tests/testBayesTree true false @@ -1114,6 +1120,7 @@ make + testErrors.run true false @@ -1343,6 +1350,46 @@ true true + + make + -j5 + testBTree.run + true + true + true + + + make + -j5 + testDSF.run + true + true + true + + + make + -j5 + testDSFMap.run + true + true + true + + + make + -j5 + testDSFVector.run + true + true + true + + + make + -j5 + testFixedVector.run + true + true + true + make -j2 @@ -1425,7 +1472,6 @@ make - testSimulated2DOriented.run true false @@ -1465,7 +1511,6 @@ make - testSimulated2D.run true false @@ -1473,7 +1518,6 @@ make - testSimulated3D.run true false @@ -1487,46 +1531,6 @@ true true - - make - -j5 - testBTree.run - true - true - true - - - make - -j5 - testDSF.run - true - true - true - - - make - -j5 - testDSFMap.run - true - true - true - - - make - -j5 - testDSFVector.run - true - true - true - - - make - -j5 - testFixedVector.run - true - true - true - make -j5 @@ -1784,6 +1788,7 @@ cpack + -G DEB true false @@ -1791,6 +1796,7 @@ cpack + -G RPM true false @@ -1798,6 +1804,7 @@ cpack + -G TGZ true false @@ -1805,6 +1812,7 @@ cpack + --config CPackSourceConfig.cmake true false @@ -2619,6 +2627,7 @@ make + testGraph.run true false @@ -2626,6 +2635,7 @@ make + testJunctionTree.run true false @@ -2633,6 +2643,7 @@ make + testSymbolicBayesNetB.run true false @@ -2742,6 +2753,14 @@ true true + + make + -j5 + testCallRecord.run + true + true + true + make -j2 @@ -3152,7 +3171,6 @@ make - tests/testGaussianISAM2 true false diff --git a/gtsam_unstable/nonlinear/CallRecord.h b/gtsam_unstable/nonlinear/CallRecord.h new file mode 100644 index 000000000..2161d9cb8 --- /dev/null +++ b/gtsam_unstable/nonlinear/CallRecord.h @@ -0,0 +1,181 @@ +/* ---------------------------------------------------------------------------- + + * GTSAM Copyright 2010, Georgia Tech Research Corporation, + * Atlanta, Georgia 30332-0415 + * All Rights Reserved + * Authors: Frank Dellaert, et al. (see THANKS for the full author list) + + * See LICENSE for the license information + + * -------------------------------------------------------------------------- */ + +/** + * @file CallRecord.h + * @date November 21, 2014 + * @author Frank Dellaert + * @author Paul Furgale + * @author Hannes Sommer + * @brief Internals for Expression.h, not for general consumption + */ + +#pragma once + +#include +#include +#include + +namespace gtsam { + +class JacobianMap; +// forward declaration + +//----------------------------------------------------------------------------- +/** + * MaxVirtualStaticRows defines how many separate virtual reverseAD with specific + * static rows (1..MaxVirtualStaticRows) methods will be part of the CallRecord interface. + */ +const int MaxVirtualStaticRows = 4; + +namespace internal { + +/** + * ConvertToDynamicIf converts to a dense matrix with dynamic rows iff + * ConvertToDynamicRows (colums stay as they are) otherwise + * it just passes dense Eigen matrices through. + */ +template +struct ConvertToDynamicRowsIf { + template + static Eigen::Matrix convert( + const Eigen::MatrixBase & x) { + return x; + } +}; + +template<> +struct ConvertToDynamicRowsIf { + template + static const Eigen::Matrix & convert( + const Eigen::Matrix & x) { + return x; + } +}; + +/** + * Recursive definition of an interface having several purely + * virtual _reverseAD(const Eigen::Matrix &, JacobianMap&) + * with Rows in 1..MaxSupportedStaticRows + */ +template +struct ReverseADInterface: public ReverseADInterface { +protected: + using ReverseADInterface::_reverseAD; + virtual void _reverseAD( + const Eigen::Matrix & dFdT, + JacobianMap& jacobians) const = 0; +}; + +template +struct ReverseADInterface<0, Cols> { +protected: + void _reverseAD() { + } //dummy to allow the using directive in the template without failing for MaxSupportedStaticRows == 1. +}; +} + +/** + * The CallRecord class stores the Jacobians of applying a function + * with respect to each of its arguments. It also stores an executation trace + * (defined below) for each of its arguments. + * + * It is implemented in the function-style ExpressionNode's nested Record class below. + */ +template +struct CallRecord: private internal::ReverseADInterface { + + inline void print(const std::string& indent) const { + _print(indent); + } + + inline void startReverseAD(JacobianMap& jacobians) const { + _startReverseAD(jacobians); + } + + template + inline void reverseAD(const Eigen::Matrix & dFdT, + JacobianMap& jacobians) const { + _reverseAD( + internal::ConvertToDynamicRowsIf<(Rows > MaxVirtualStaticRows)>::convert( + dFdT), jacobians); + } + + virtual ~CallRecord() { + } + +private: + + using internal::ReverseADInterface::_reverseAD; + virtual void _print(const std::string& indent) const = 0; + virtual void _startReverseAD(JacobianMap& jacobians) const = 0; + virtual void _reverseAD( + const Eigen::Matrix & dFdT, + JacobianMap& jacobians) const = 0; + virtual void _reverseAD(const Matrix & dFdT, + JacobianMap& jacobians) const = 0; +}; + +namespace internal { + +/** + * ReverseADImplementor is a utility class used by CallRecordImplementor to + * implementing the recursive CallRecord interface. + */ +template +struct ReverseADImplementor: ReverseADImplementor { + +protected: + + virtual void _reverseAD( + const Eigen::Matrix & dFdT, + JacobianMap& jacobians) const { + static_cast(this)->reverseAD(dFdT, jacobians); + } +}; + +template +struct ReverseADImplementor : CallRecord { +}; + +/** + * The CallRecordImplementor implements the CallRecord interface for a Derived class by + * delegating to its corresponding (templated) non-virtual methods. + */ +template +struct CallRecordImplementor: public ReverseADImplementor { +private: + const Derived & derived() const { + return static_cast(*this); + } + virtual void _print(const std::string& indent) const { + derived().print(indent); + } + virtual void _startReverseAD(JacobianMap& jacobians) const { + derived().startReverseAD(jacobians); + } + virtual void _reverseAD( + const Eigen::Matrix & dFdT, + JacobianMap& jacobians) const { + derived().reverseAD(dFdT, jacobians); + } + virtual void _reverseAD(const Matrix & dFdT, JacobianMap& jacobians) const { + derived().reverseAD(dFdT, jacobians); + } +}; + +} // internal + +} // gtsam diff --git a/gtsam_unstable/nonlinear/tests/testCallRecord.cpp b/gtsam_unstable/nonlinear/tests/testCallRecord.cpp new file mode 100644 index 000000000..a8abf295a --- /dev/null +++ b/gtsam_unstable/nonlinear/tests/testCallRecord.cpp @@ -0,0 +1,55 @@ +/* ---------------------------------------------------------------------------- + + * GTSAM Copyright 2010, Georgia Tech Research Corporation, + * Atlanta, Georgia 30332-0415 + * All Rights Reserved + * Authors: Frank Dellaert, et al. (see THANKS for the full author list) + + * See LICENSE for the license information + + * -------------------------------1------------------------------------------- */ + +/** + * @file testCallRecord.cpp + * @date November 21, 2014 + * @author Frank Dellaert + * @author Paul Furgale + * @author Hannes Sommer + * @brief unit tests for Callrecord class + */ + +#include + +#include + +using namespace std; +using namespace gtsam; + +/* ************************************************************************* */ +static const int Cols = 3; + +struct Record: public internal::CallRecordImplementor { + virtual ~Record() { + } + void print(const std::string& indent) const { + } + void startReverseAD(JacobianMap& jacobians) const { + } + template + void reverseAD(const SomeMatrix & dFdT, JacobianMap& jacobians) const { + } +}; + +/* ************************************************************************* */ +// Construct +TEST(CallRecord, constant) { + Record record; +} + +/* ************************************************************************* */ +int main() { + TestResult tr; + return TestRegistry::runAllTests(tr); +} +/* ************************************************************************* */ + From c238e5852cbaadbfdc1c043a718e07f08f8b9117 Mon Sep 17 00:00:00 2001 From: dellaert Date: Fri, 21 Nov 2014 15:48:29 +0100 Subject: [PATCH 04/11] Now uses CallRecord.h --- gtsam_unstable/nonlinear/Expression-inl.h | 124 +--------------------- 1 file changed, 5 insertions(+), 119 deletions(-) diff --git a/gtsam_unstable/nonlinear/Expression-inl.h b/gtsam_unstable/nonlinear/Expression-inl.h index 63b159e05..9f5a3ab90 100644 --- a/gtsam_unstable/nonlinear/Expression-inl.h +++ b/gtsam_unstable/nonlinear/Expression-inl.h @@ -19,17 +19,16 @@ #pragma once +#include #include -#include #include #include -#include #include #include #include -// template meta-programming headers +// template meta-programming headers, TODO not all needed? #include #include #include @@ -40,8 +39,10 @@ #include #include namespace MPL = boost::mpl::placeholders; +// +//#include // for placement new +#include -#include // for placement new class ExpressionFactorBinaryTest; // Forward declare for testing @@ -71,121 +72,6 @@ public: } }; -//----------------------------------------------------------------------------- -/** - * MaxVirtualStaticRows defines how many separate virtual reverseAD with specific - * static rows (1..MaxVirtualStaticRows) methods will be part of the CallRecord interface. - */ -const int MaxVirtualStaticRows = 4; - -namespace internal { -/** - * ConvertToDynamicIf converts to a dense matrix with dynamic rows iff ConvertToDynamicRows (colums stay as they are) otherwise - * it just passes dense Eigen matrices through. - */ -template -struct ConvertToDynamicRowsIf { - template - static Eigen::Matrix convert(const Eigen::MatrixBase & x){ - return x; - } -}; -template <> -struct ConvertToDynamicRowsIf { - template - static const Eigen::Matrix & convert(const Eigen::Matrix & x){ - return x; - } -}; - -/** - * Recursive definition of an interface having several purely - * virtual _reverseAD(const Eigen::Matrix &, JacobianMap&) - * with Rows in 1..MaxSupportedStaticRows - */ -template -struct ReverseADInterface : public ReverseADInterface < MaxSupportedStaticRows - 1, Cols> { - protected: - using ReverseADInterface < MaxSupportedStaticRows - 1, Cols>::_reverseAD; - virtual void _reverseAD(const Eigen::Matrix & dFdT, JacobianMap& jacobians) const = 0; -}; - -template -struct ReverseADInterface<0, Cols> { - protected: - void _reverseAD(){} //dummy to allow the using directive in the template without failing for MaxSupportedStaticRows == 1. -}; -} - -/** - * The CallRecord class stores the Jacobians of applying a function - * with respect to each of its arguments. It also stores an executation trace - * (defined below) for each of its arguments. - * - * It is implemented in the function-style ExpressionNode's nested Record class below. - */ -template -struct CallRecord : private internal::ReverseADInterface { - inline void print(const std::string& indent) const { - _print(indent); - } - inline void startReverseAD(JacobianMap& jacobians) const { - _startReverseAD(jacobians); - } - template - inline void reverseAD(const Eigen::Matrix & dFdT, JacobianMap& jacobians) const{ - _reverseAD(internal::ConvertToDynamicRowsIf<(Rows > MaxVirtualStaticRows)>::convert(dFdT), jacobians); - } - virtual ~CallRecord() { - } - private: - using internal::ReverseADInterface::_reverseAD; - virtual void _print(const std::string& indent) const = 0; - virtual void _startReverseAD(JacobianMap& jacobians) const = 0; - virtual void _reverseAD(const Eigen::Matrix & dFdT, JacobianMap& jacobians) const = 0; - virtual void _reverseAD(const Matrix & dFdT, JacobianMap& jacobians) const = 0; -}; - -namespace internal { -/** - * ReverseADImplementor is a utility class used by CallRecordImplementor to implementing the recursive CallRecord interface. - */ -template -struct ReverseADImplementor : ReverseADImplementor { - protected: - virtual void _reverseAD(const Eigen::Matrix & dFdT, JacobianMap& jacobians) const { - static_cast(this)->reverseAD(dFdT, jacobians); - } -}; -template -struct ReverseADImplementor : CallRecord { -}; - -/** - * The CallRecordImplementor implements the CallRecord interface for a Derived class by - * delegating to its corresponding (templated) non-virtual methods. - */ -template -struct CallRecordImplementor : public ReverseADImplementor { - private: - const Derived & derived() const { - return static_cast(*this); - } - virtual void _print(const std::string& indent) const { - derived().print(indent); - } - virtual void _startReverseAD(JacobianMap& jacobians) const { - derived().startReverseAD(jacobians); - } - virtual void _reverseAD(const Eigen::Matrix & dFdT, JacobianMap& jacobians) const { - derived().reverseAD(dFdT, jacobians); - } - virtual void _reverseAD(const Matrix & dFdT, JacobianMap& jacobians) const { - derived().reverseAD(dFdT, jacobians); - } -}; -} - //----------------------------------------------------------------------------- /// Handle Leaf Case: reverseAD ends here, by writing a matrix into Jacobians template From f699dd26bb50dac035a937061bedfeb57ecaeb03 Mon Sep 17 00:00:00 2001 From: HannesSommer Date: Fri, 21 Nov 2014 21:09:03 +0100 Subject: [PATCH 05/11] correct case in import --- gtsam_unstable/nonlinear/Expression-inl.h | 2 +- gtsam_unstable/nonlinear/tests/testCallRecord.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gtsam_unstable/nonlinear/Expression-inl.h b/gtsam_unstable/nonlinear/Expression-inl.h index 9f5a3ab90..40eb49def 100644 --- a/gtsam_unstable/nonlinear/Expression-inl.h +++ b/gtsam_unstable/nonlinear/Expression-inl.h @@ -19,7 +19,7 @@ #pragma once -#include +#include #include #include #include diff --git a/gtsam_unstable/nonlinear/tests/testCallRecord.cpp b/gtsam_unstable/nonlinear/tests/testCallRecord.cpp index a8abf295a..ab7e62419 100644 --- a/gtsam_unstable/nonlinear/tests/testCallRecord.cpp +++ b/gtsam_unstable/nonlinear/tests/testCallRecord.cpp @@ -15,10 +15,10 @@ * @author Frank Dellaert * @author Paul Furgale * @author Hannes Sommer - * @brief unit tests for Callrecord class + * @brief unit tests for CallRecord class */ -#include +#include #include From 6d0c1a44e1b8fb51842b554dc006ca79d8baf682 Mon Sep 17 00:00:00 2001 From: HannesSommer Date: Fri, 21 Nov 2014 21:13:24 +0100 Subject: [PATCH 06/11] - some small cleanup and improved readability. - virtual overload warnings should not be issued anymore --- gtsam_unstable/nonlinear/CallRecord.h | 55 ++++++++++++++++----------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/gtsam_unstable/nonlinear/CallRecord.h b/gtsam_unstable/nonlinear/CallRecord.h index 2161d9cb8..3206ba9b1 100644 --- a/gtsam_unstable/nonlinear/CallRecord.h +++ b/gtsam_unstable/nonlinear/CallRecord.h @@ -67,9 +67,8 @@ struct ConvertToDynamicRowsIf { * with Rows in 1..MaxSupportedStaticRows */ template -struct ReverseADInterface: public ReverseADInterface { -protected: using ReverseADInterface::_reverseAD; virtual void _reverseAD( const Eigen::Matrix & dFdT, @@ -78,10 +77,15 @@ protected: template struct ReverseADInterface<0, Cols> { -protected: - void _reverseAD() { - } //dummy to allow the using directive in the template without failing for MaxSupportedStaticRows == 1. + virtual void _reverseAD( + const Eigen::Matrix & dFdT, + JacobianMap& jacobians) const = 0; + virtual void _reverseAD(const Matrix & dFdT, + JacobianMap& jacobians) const = 0; }; + +template +struct ReverseADImplementor; // forward for CallRecord's friend declaration } /** @@ -115,15 +119,12 @@ struct CallRecord: private internal::ReverseADInterface::_reverseAD; virtual void _print(const std::string& indent) const = 0; virtual void _startReverseAD(JacobianMap& jacobians) const = 0; - virtual void _reverseAD( - const Eigen::Matrix & dFdT, - JacobianMap& jacobians) const = 0; - virtual void _reverseAD(const Matrix & dFdT, - JacobianMap& jacobians) const = 0; + using internal::ReverseADInterface::_reverseAD; + template + friend struct internal::ReverseADImplementor; }; namespace internal { @@ -136,17 +137,33 @@ template struct ReverseADImplementor: ReverseADImplementor { -protected: - +private: + using ReverseADImplementor::_reverseAD; virtual void _reverseAD( const Eigen::Matrix & dFdT, JacobianMap& jacobians) const { static_cast(this)->reverseAD(dFdT, jacobians); } + friend struct internal::ReverseADImplementor; }; template struct ReverseADImplementor : CallRecord { +private: + using CallRecord::_reverseAD; + const Derived & derived() const { + return static_cast(*this); + } + virtual void _reverseAD( + const Eigen::Matrix & dFdT, + JacobianMap& jacobians) const { + derived().reverseAD(dFdT, jacobians); + } + virtual void _reverseAD(const Matrix & dFdT, JacobianMap& jacobians) const { + derived().reverseAD(dFdT, jacobians); + } + friend struct internal::ReverseADImplementor; }; /** @@ -154,7 +171,7 @@ struct ReverseADImplementor : CallRecord { * delegating to its corresponding (templated) non-virtual methods. */ template -struct CallRecordImplementor: public ReverseADImplementor { private: const Derived & derived() const { @@ -166,14 +183,6 @@ private: virtual void _startReverseAD(JacobianMap& jacobians) const { derived().startReverseAD(jacobians); } - virtual void _reverseAD( - const Eigen::Matrix & dFdT, - JacobianMap& jacobians) const { - derived().reverseAD(dFdT, jacobians); - } - virtual void _reverseAD(const Matrix & dFdT, JacobianMap& jacobians) const { - derived().reverseAD(dFdT, jacobians); - } }; } // internal From 32992cf05e78181c8ff6ad1e609a50a24846ccea Mon Sep 17 00:00:00 2001 From: HannesSommer Date: Fri, 21 Nov 2014 22:36:32 +0100 Subject: [PATCH 07/11] added missing overload for full dynamic matrix. --- gtsam_unstable/nonlinear/CallRecord.h | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/gtsam_unstable/nonlinear/CallRecord.h b/gtsam_unstable/nonlinear/CallRecord.h index 3206ba9b1..9d64384f2 100644 --- a/gtsam_unstable/nonlinear/CallRecord.h +++ b/gtsam_unstable/nonlinear/CallRecord.h @@ -115,6 +115,11 @@ struct CallRecord: private internal::ReverseADInterface Date: Fri, 21 Nov 2014 22:36:45 +0100 Subject: [PATCH 08/11] added CallRecord unit test --- .../nonlinear/tests/testCallRecord.cpp | 114 +++++++++++++++++- 1 file changed, 112 insertions(+), 2 deletions(-) diff --git a/gtsam_unstable/nonlinear/tests/testCallRecord.cpp b/gtsam_unstable/nonlinear/tests/testCallRecord.cpp index ab7e62419..a4561b349 100644 --- a/gtsam_unstable/nonlinear/tests/testCallRecord.cpp +++ b/gtsam_unstable/nonlinear/tests/testCallRecord.cpp @@ -22,12 +22,55 @@ #include +#include +#include + using namespace std; using namespace gtsam; /* ************************************************************************* */ static const int Cols = 3; + +int dynamicIfAboveMax(int i){ + if(i > MaxVirtualStaticRows){ + return Eigen::Dynamic; + } + else return i; +} +struct CallConfig { + int compTimeRows; + int compTimeCols; + int runTimeRows; + int runTimeCols; + CallConfig() {} + CallConfig(int rows, int cols): + compTimeRows(dynamicIfAboveMax(rows)), + compTimeCols(cols), + runTimeRows(rows), + runTimeCols(cols) + { + } + CallConfig(int compTimeRows, int compTimeCols, int runTimeRows, int runTimeCols): + compTimeRows(compTimeRows), + compTimeCols(compTimeCols), + runTimeRows(runTimeRows), + runTimeCols(runTimeCols) + { + } + + bool equals(const CallConfig & c, double /*tol*/) const { + return + this->compTimeRows == c.compTimeRows && + this->compTimeCols == c.compTimeCols && + this->runTimeRows == c.runTimeRows && + this->runTimeCols == c.runTimeCols; + } + void print(const std::string & prefix) const { + std::cout << prefix << "{" << compTimeRows << ", " << compTimeCols << ", " << runTimeRows << ", " << runTimeCols << "}\n" ; + } +}; + struct Record: public internal::CallRecordImplementor { virtual ~Record() { } @@ -35,15 +78,82 @@ struct Record: public internal::CallRecordImplementor { } void startReverseAD(JacobianMap& jacobians) const { } + + mutable CallConfig cc; + private: template void reverseAD(const SomeMatrix & dFdT, JacobianMap& jacobians) const { + cc.compTimeRows = SomeMatrix::RowsAtCompileTime; + cc.compTimeCols = SomeMatrix::ColsAtCompileTime; + cc.runTimeRows = dFdT.rows(); + cc.runTimeCols = dFdT.cols(); } + + template + friend struct internal::ReverseADImplementor; }; +JacobianMap & NJM= *static_cast(NULL); + /* ************************************************************************* */ -// Construct -TEST(CallRecord, constant) { +typedef Eigen::Matrix DynRowMat; + +TEST(CallRecord, virtualReverseAdDispatching) { Record record; + { + const int Rows = 1; + record.CallRecord::reverseAD(Eigen::Matrix(), NJM); + EXPECT((assert_equal(record.cc, CallConfig(Rows, Cols)))); + record.CallRecord::reverseAD(DynRowMat(Rows, Cols), NJM); + EXPECT((assert_equal(record.cc, CallConfig(Eigen::Dynamic, Cols, Rows, Cols)))); + record.CallRecord::reverseAD(Eigen::MatrixXd(Rows, Cols), NJM); + EXPECT((assert_equal(record.cc, CallConfig(Eigen::Dynamic, Eigen::Dynamic, Rows, Cols)))); + } + { + const int Rows = 2; + record.CallRecord::reverseAD(Eigen::Matrix(), NJM); + EXPECT((assert_equal(record.cc, CallConfig(Rows, Cols)))); + record.CallRecord::reverseAD(DynRowMat(Rows, Cols), NJM); + EXPECT((assert_equal(record.cc, CallConfig(Eigen::Dynamic, Cols, Rows, Cols)))); + record.CallRecord::reverseAD(Eigen::MatrixXd(Rows, Cols), NJM); + EXPECT((assert_equal(record.cc, CallConfig(Eigen::Dynamic, Eigen::Dynamic, Rows, Cols)))); + } + { + const int Rows = 3; + record.CallRecord::reverseAD(Eigen::Matrix(), NJM); + EXPECT((assert_equal(record.cc, CallConfig(Rows, Cols)))); + record.CallRecord::reverseAD(DynRowMat(Rows, Cols), NJM); + EXPECT((assert_equal(record.cc, CallConfig(Eigen::Dynamic, Cols, Rows, Cols)))); + record.CallRecord::reverseAD(Eigen::MatrixXd(Rows, Cols), NJM); + EXPECT((assert_equal(record.cc, CallConfig(Eigen::Dynamic, Eigen::Dynamic, Rows, Cols)))); + } + { + const int Rows = MaxVirtualStaticRows; + record.CallRecord::reverseAD(Eigen::Matrix(), NJM); + EXPECT((assert_equal(record.cc, CallConfig(Rows, Cols)))); + record.CallRecord::reverseAD(DynRowMat(Rows, Cols), NJM); + EXPECT((assert_equal(record.cc, CallConfig(Eigen::Dynamic, Cols, Rows, Cols)))); + record.CallRecord::reverseAD(Eigen::MatrixXd(Rows, Cols), NJM); + EXPECT((assert_equal(record.cc, CallConfig(Eigen::Dynamic, Eigen::Dynamic, Rows, Cols)))); + } + { + const int Rows = MaxVirtualStaticRows + 1; + record.CallRecord::reverseAD(Eigen::Matrix(), NJM); + EXPECT((assert_equal(record.cc, CallConfig(Rows, Cols)))); + record.CallRecord::reverseAD(DynRowMat(Rows, Cols), NJM); + EXPECT((assert_equal(record.cc, CallConfig(Eigen::Dynamic, Cols, Rows, Cols)))); + record.CallRecord::reverseAD(Eigen::MatrixXd(Rows, Cols), NJM); + EXPECT((assert_equal(record.cc, CallConfig(Eigen::Dynamic, Eigen::Dynamic, Rows, Cols)))); + } + { + const int Rows = MaxVirtualStaticRows + 2; + record.CallRecord::reverseAD(Eigen::Matrix(), NJM); + EXPECT((assert_equal(record.cc, CallConfig(Rows, Cols)))); + record.CallRecord::reverseAD(DynRowMat(Rows, Cols), NJM); + EXPECT((assert_equal(record.cc, CallConfig(Eigen::Dynamic, Cols, Rows, Cols)))); + record.CallRecord::reverseAD(Eigen::MatrixXd(Rows, Cols), NJM); + EXPECT((assert_equal(record.cc, CallConfig(Eigen::Dynamic, Eigen::Dynamic, Rows, Cols)))); + } } /* ************************************************************************* */ From 87ea6341f2edaf65a43e5ef531a9eac8adf22e44 Mon Sep 17 00:00:00 2001 From: HannesSommer Date: Fri, 21 Nov 2014 22:18:50 +0100 Subject: [PATCH 09/11] virtual inheritance for better readability and decoupling --- gtsam_unstable/nonlinear/CallRecord.h | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/gtsam_unstable/nonlinear/CallRecord.h b/gtsam_unstable/nonlinear/CallRecord.h index 9d64384f2..6927a79be 100644 --- a/gtsam_unstable/nonlinear/CallRecord.h +++ b/gtsam_unstable/nonlinear/CallRecord.h @@ -96,7 +96,7 @@ struct ReverseADImplementor; // forward for CallRecord's friend declaration * It is implemented in the function-style ExpressionNode's nested Record class below. */ template -struct CallRecord: private internal::ReverseADInterface { inline void print(const std::string& indent) const { @@ -154,9 +154,10 @@ private: }; template -struct ReverseADImplementor : CallRecord { +struct ReverseADImplementor + : virtual internal::ReverseADInterface { private: - using CallRecord::_reverseAD; + using internal::ReverseADInterface::_reverseAD; const Derived & derived() const { return static_cast(*this); } @@ -177,7 +178,7 @@ private: */ template struct CallRecordImplementor: ReverseADImplementor { + MaxVirtualStaticRows, Cols>, CallRecord{ private: const Derived & derived() const { return static_cast(*this); From cc997dd7746799850aa10e86e09b7cea2fc3882a Mon Sep 17 00:00:00 2001 From: HannesSommer Date: Sat, 22 Nov 2014 19:19:17 +0100 Subject: [PATCH 10/11] adapted a view comments and friendships to the new virtual inheritance sceme visibility fine tuning --- gtsam_unstable/nonlinear/CallRecord.h | 89 +++++++++++++-------------- 1 file changed, 42 insertions(+), 47 deletions(-) diff --git a/gtsam_unstable/nonlinear/CallRecord.h b/gtsam_unstable/nonlinear/CallRecord.h index 6927a79be..97fe74093 100644 --- a/gtsam_unstable/nonlinear/CallRecord.h +++ b/gtsam_unstable/nonlinear/CallRecord.h @@ -84,13 +84,48 @@ struct ReverseADInterface<0, Cols> { JacobianMap& jacobians) const = 0; }; +/** + * ReverseADImplementor is a utility class used by CallRecordImplementor to + * implementing the recursive ReverseADInterface interface. + */ template -struct ReverseADImplementor; // forward for CallRecord's friend declaration -} +struct ReverseADImplementor: ReverseADImplementor { +private: + using ReverseADImplementor::_reverseAD; + virtual void _reverseAD( + const Eigen::Matrix & dFdT, + JacobianMap& jacobians) const { + static_cast(this)->reverseAD(dFdT, jacobians); + } + friend struct internal::ReverseADImplementor; +}; + +template +struct ReverseADImplementor + : virtual internal::ReverseADInterface { +private: + using internal::ReverseADInterface::_reverseAD; + const Derived & derived() const { + return static_cast(*this); + } + virtual void _reverseAD( + const Eigen::Matrix & dFdT, + JacobianMap& jacobians) const { + derived().reverseAD(dFdT, jacobians); + } + virtual void _reverseAD(const Matrix & dFdT, JacobianMap& jacobians) const { + derived().reverseAD(dFdT, jacobians); + } + friend struct internal::ReverseADImplementor; +}; + +} // namespace internal /** * The CallRecord class stores the Jacobians of applying a function - * with respect to each of its arguments. It also stores an executation trace + * with respect to each of its arguments. It also stores an execution trace * (defined below) for each of its arguments. * * It is implemented in the function-style ExpressionNode's nested Record class below. @@ -128,57 +163,16 @@ private: virtual void _startReverseAD(JacobianMap& jacobians) const = 0; using internal::ReverseADInterface::_reverseAD; - template - friend struct internal::ReverseADImplementor; }; namespace internal { - -/** - * ReverseADImplementor is a utility class used by CallRecordImplementor to - * implementing the recursive CallRecord interface. - */ -template -struct ReverseADImplementor: ReverseADImplementor { - -private: - using ReverseADImplementor::_reverseAD; - virtual void _reverseAD( - const Eigen::Matrix & dFdT, - JacobianMap& jacobians) const { - static_cast(this)->reverseAD(dFdT, jacobians); - } - friend struct internal::ReverseADImplementor; -}; - -template -struct ReverseADImplementor - : virtual internal::ReverseADInterface { -private: - using internal::ReverseADInterface::_reverseAD; - const Derived & derived() const { - return static_cast(*this); - } - virtual void _reverseAD( - const Eigen::Matrix & dFdT, - JacobianMap& jacobians) const { - derived().reverseAD(dFdT, jacobians); - } - virtual void _reverseAD(const Matrix & dFdT, JacobianMap& jacobians) const { - derived().reverseAD(dFdT, jacobians); - } - friend struct internal::ReverseADImplementor; -}; - /** * The CallRecordImplementor implements the CallRecord interface for a Derived class by * delegating to its corresponding (templated) non-virtual methods. */ template -struct CallRecordImplementor: ReverseADImplementor, CallRecord{ +struct CallRecordImplementor: public CallRecord, + private ReverseADImplementor { private: const Derived & derived() const { return static_cast(*this); @@ -189,8 +183,9 @@ private: virtual void _startReverseAD(JacobianMap& jacobians) const { derived().startReverseAD(jacobians); } + template friend class ReverseADImplementor; }; -} // internal +} // namespace internal } // gtsam From d00aeb7e70d6ce2b3df11c9e800d543015896455 Mon Sep 17 00:00:00 2001 From: dellaert Date: Sat, 22 Nov 2014 21:48:36 +0100 Subject: [PATCH 11/11] Formatting and some small problems --- .../examples/Pose2SLAMExampleExpressions.cpp | 2 +- gtsam_unstable/nonlinear/CallRecord.h | 29 +++++++-------- gtsam_unstable/nonlinear/Expression-inl.h | 37 ++++++++----------- .../nonlinear/tests/testExpressionMeta.cpp | 1 - 4 files changed, 30 insertions(+), 39 deletions(-) diff --git a/gtsam_unstable/examples/Pose2SLAMExampleExpressions.cpp b/gtsam_unstable/examples/Pose2SLAMExampleExpressions.cpp index 936c9957b..ac43fa428 100644 --- a/gtsam_unstable/examples/Pose2SLAMExampleExpressions.cpp +++ b/gtsam_unstable/examples/Pose2SLAMExampleExpressions.cpp @@ -10,7 +10,7 @@ * -------------------------------------------------------------------------- */ /** - * @file Pose2SLAMExample.cpp + * @file Pose2SLAMExampleExpressions.cpp * @brief Expressions version of Pose2SLAMExample.cpp * @date Oct 2, 2014 * @author Frank Dellaert diff --git a/gtsam_unstable/nonlinear/CallRecord.h b/gtsam_unstable/nonlinear/CallRecord.h index 97fe74093..3806f1803 100644 --- a/gtsam_unstable/nonlinear/CallRecord.h +++ b/gtsam_unstable/nonlinear/CallRecord.h @@ -24,6 +24,8 @@ #include #include +#include + namespace gtsam { class JacobianMap; @@ -67,8 +69,7 @@ struct ConvertToDynamicRowsIf { * with Rows in 1..MaxSupportedStaticRows */ template -struct ReverseADInterface: ReverseADInterface { +struct ReverseADInterface: ReverseADInterface { using ReverseADInterface::_reverseAD; virtual void _reverseAD( const Eigen::Matrix & dFdT, @@ -92,19 +93,19 @@ template struct ReverseADImplementor: ReverseADImplementor { private: - using ReverseADImplementor::_reverseAD; + using ReverseADImplementor::_reverseAD; virtual void _reverseAD( const Eigen::Matrix & dFdT, JacobianMap& jacobians) const { static_cast(this)->reverseAD(dFdT, jacobians); } - friend struct internal::ReverseADImplementor; + friend struct internal::ReverseADImplementor; }; template -struct ReverseADImplementor - : virtual internal::ReverseADInterface { +struct ReverseADImplementor : virtual internal::ReverseADInterface< + MaxVirtualStaticRows, Cols> { private: using internal::ReverseADInterface::_reverseAD; const Derived & derived() const { @@ -131,8 +132,8 @@ private: * It is implemented in the function-style ExpressionNode's nested Record class below. */ template -struct CallRecord: virtual private internal::ReverseADInterface { +struct CallRecord: virtual private internal::ReverseADInterface< + MaxVirtualStaticRows, Cols> { inline void print(const std::string& indent) const { _print(indent); @@ -150,8 +151,7 @@ struct CallRecord: virtual private internal::ReverseADInterface::_reverseAD; + using internal::ReverseADInterface::_reverseAD; }; namespace internal { @@ -172,7 +171,7 @@ namespace internal { */ template struct CallRecordImplementor: public CallRecord, - private ReverseADImplementor { + private ReverseADImplementor { private: const Derived & derived() const { return static_cast(*this); @@ -183,7 +182,7 @@ private: virtual void _startReverseAD(JacobianMap& jacobians) const { derived().startReverseAD(jacobians); } - template friend class ReverseADImplementor; + template friend struct ReverseADImplementor; }; } // namespace internal diff --git a/gtsam_unstable/nonlinear/Expression-inl.h b/gtsam_unstable/nonlinear/Expression-inl.h index 40eb49def..a98ab349f 100644 --- a/gtsam_unstable/nonlinear/Expression-inl.h +++ b/gtsam_unstable/nonlinear/Expression-inl.h @@ -28,19 +28,10 @@ #include #include -// template meta-programming headers, TODO not all needed? -#include -#include -#include -#include +// template meta-programming headers #include -#include -#include -#include -#include namespace MPL = boost::mpl::placeholders; -// -//#include // for placement new + #include class ExpressionFactorBinaryTest; @@ -171,8 +162,9 @@ public: content.ptr->startReverseAD(jacobians); } // Either add to Jacobians (Leaf) or propagate (Function) - template - void reverseAD(const Eigen::MatrixBase & dTdA, JacobianMap& jacobians) const { + template + void reverseAD(const Eigen::MatrixBase & dTdA, + JacobianMap& jacobians) const { if (kind == Leaf) handleLeafCase(dTdA.eval(), jacobians, content.key); else if (kind == Function) @@ -435,7 +427,7 @@ struct FunctionalBase: ExpressionNode { } void startReverseAD(JacobianMap& jacobians) const { } - template + template void reverseAD(const SomeMatrix & dFdT, JacobianMap& jacobians) const { } }; @@ -511,8 +503,9 @@ struct GenerateFunctionalNode: Argument, Base { } /// Given df/dT, multiply in dT/dA and continue reverse AD process - template - void reverseAD(const Eigen::Matrix & dFdT, JacobianMap& jacobians) const { + template + void reverseAD(const Eigen::Matrix & dFdT, + JacobianMap& jacobians) const { Base::Record::reverseAD(dFdT, jacobians); This::trace.reverseAD(dFdT * This::dTdA, jacobians); } @@ -571,14 +564,14 @@ struct FunctionalNode { /// Provide convenience access to Record storage and implement /// the virtual function based interface of CallRecord using the CallRecordImplementor - struct Record: - public internal::CallRecordImplementor::value>, - public Base::Record { + struct Record: public internal::CallRecordImplementor::value>, public Base::Record { using Base::Record::print; using Base::Record::startReverseAD; using Base::Record::reverseAD; - virtual ~Record(){} + virtual ~Record() { + } /// Access Value template @@ -690,8 +683,8 @@ public: virtual T value(const Values& values) const { using boost::none; return function_(this->template expression()->value(values), - this->template expression()->value(values), - none, none); + this->template expression()->value(values), + none, none); } /// Construct an execution trace for reverse AD diff --git a/gtsam_unstable/nonlinear/tests/testExpressionMeta.cpp b/gtsam_unstable/nonlinear/tests/testExpressionMeta.cpp index b2cdcdf34..d10e31002 100644 --- a/gtsam_unstable/nonlinear/tests/testExpressionMeta.cpp +++ b/gtsam_unstable/nonlinear/tests/testExpressionMeta.cpp @@ -38,7 +38,6 @@ template struct Incomplete; typedef mpl::vector MyTypes; typedef FunctionalNode::type Generated; //Incomplete incomplete; -BOOST_MPL_ASSERT((boost::is_same< Matrix2, Generated::Record::Jacobian2T >)); // Try generating vectors of ExecutionTrace typedef mpl::vector, ExecutionTrace > ExpectedTraces;