From 7a4748d3dcb008889dffbdbb027aafb2db250a23 Mon Sep 17 00:00:00 2001 From: dellaert Date: Fri, 14 Nov 2014 16:43:53 +0100 Subject: [PATCH 01/19] Simplified method/function hierarchy drastically, and renamed bottom addOverload to initializeOrCheck to reflect what it does. Also, gratuitous re-ordering of addOverload arguments. --- wrap/Argument.h | 8 -- wrap/Class.cpp | 8 +- wrap/Constructor.h | 18 ++-- wrap/FullyOverloadedFunction.h | 133 +++++++++++++++++++++++ wrap/Function.cpp | 43 +++----- wrap/Function.h | 186 ++------------------------------- wrap/GlobalFunction.cpp | 23 ++-- wrap/GlobalFunction.h | 22 ++-- wrap/Method.cpp | 22 ++-- wrap/Method.h | 18 +--- wrap/Module.cpp | 9 +- wrap/OverloadedFunction.h | 126 ++++++++++++++++++++++ wrap/StaticMethod.cpp | 8 -- wrap/StaticMethod.h | 16 +-- wrap/tests/testMethod.cpp | 9 +- 15 files changed, 341 insertions(+), 308 deletions(-) create mode 100644 wrap/FullyOverloadedFunction.h create mode 100644 wrap/OverloadedFunction.h diff --git a/wrap/Argument.h b/wrap/Argument.h index 02f104418..3d8d7288f 100644 --- a/wrap/Argument.h +++ b/wrap/Argument.h @@ -122,13 +122,5 @@ struct ArgumentList: public std::vector { }; -template -inline void verifyArguments(const std::vector& validArgs, - const std::map& vt) { - typedef typename std::map::value_type NamedMethod; - BOOST_FOREACH(const NamedMethod& namedMethod, vt) - namedMethod.second.verifyArguments(validArgs); -} - } // \namespace wrap diff --git a/wrap/Class.cpp b/wrap/Class.cpp index 3a3432eb3..9c759bb62 100644 --- a/wrap/Class.cpp +++ b/wrap/Class.cpp @@ -286,13 +286,13 @@ void Class::addMethod(bool verbose, bool is_const, Str methodName, // Now stick in new overload stack with expandedMethodName key // but note we use the same, unexpanded methodName in overload string expandedMethodName = methodName + instName.name; - methods[expandedMethodName].addOverload(verbose, is_const, methodName, - expandedArgs, expandedRetVal, instName); + methods[expandedMethodName].addOverload(methodName, expandedArgs, + expandedRetVal, is_const, instName, verbose); } } else // just add overload - methods[methodName].addOverload(verbose, is_const, methodName, argumentList, - returnValue); + methods[methodName].addOverload(methodName, argumentList, returnValue, + is_const, Qualified(), verbose); } /* ************************************************************************* */ diff --git a/wrap/Constructor.h b/wrap/Constructor.h index adfcb8472..870f1a15e 100644 --- a/wrap/Constructor.h +++ b/wrap/Constructor.h @@ -18,7 +18,7 @@ #pragma once -#include "Function.h" +#include "OverloadedFunction.h" #include #include @@ -26,21 +26,17 @@ namespace wrap { // Constructor class -struct Constructor: public ArgumentOverloads { +struct Constructor: public OverloadedFunction { /// Constructor creates an empty class - Constructor(bool verbose = false) : - verbose_(verbose) { + Constructor(bool verbose = false) { + verbose_ = verbose; } - // Then the instance variables are set directly by the Module constructor - std::string name; - bool verbose_; - Constructor expandTemplate(const TemplateSubstitution& ts) const { Constructor inst = *this; inst.argLists_ = expandArgumentListsTemplate(ts); - inst.name = ts.expandedClassName(); + inst.name_ = ts.expandedClassName(); return inst; } @@ -56,7 +52,7 @@ struct Constructor: public ArgumentOverloads { proxyFile.oss << "%\n%-------Constructors-------\n"; for (size_t i = 0; i < nrOverloads(); i++) { proxyFile.oss << "%"; - argumentList(i).emit_prototype(proxyFile, name); + argumentList(i).emit_prototype(proxyFile, name_); proxyFile.oss << "\n"; } } @@ -80,7 +76,7 @@ struct Constructor: public ArgumentOverloads { friend std::ostream& operator<<(std::ostream& os, const Constructor& m) { for (size_t i = 0; i < m.nrOverloads(); i++) - os << m.name << m.argLists_[i]; + os << m.name_ << m.argLists_[i]; return os; } diff --git a/wrap/FullyOverloadedFunction.h b/wrap/FullyOverloadedFunction.h new file mode 100644 index 000000000..ac22ec3a8 --- /dev/null +++ b/wrap/FullyOverloadedFunction.h @@ -0,0 +1,133 @@ +/* ---------------------------------------------------------------------------- + + * 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 FullyOverloadedFunction.h + * @brief Function that can be fully overloaded: arguments and return values + * @author Frank Dellaert + * @date Nov 13, 2014 + **/ + +#pragma once + +#include "OverloadedFunction.h" + +namespace wrap { + +/** + * Signature Overload (including return value) + */ +class SignatureOverloads: public ArgumentOverloads { + +protected: + + std::vector returnVals_; + +public: + + const ReturnValue& returnValue(size_t i) const { + return returnVals_.at(i); + } + + void push_back(const ArgumentList& args, const ReturnValue& retVal) { + argLists_.push_back(args); + returnVals_.push_back(retVal); + } + + void verifyReturnTypes(const std::vector& validtypes, + const std::string& s) const { + BOOST_FOREACH(const ReturnValue& retval, returnVals_) { + retval.type1.verify(validtypes, s); + if (retval.isPair) + retval.type2.verify(validtypes, s); + } + } + + // TODO use transform ? + std::vector expandReturnValuesTemplate( + const TemplateSubstitution& ts) const { + std::vector result; + BOOST_FOREACH(const ReturnValue& retVal, returnVals_) { + ReturnValue instRetVal = retVal.expandTemplate(ts); + result.push_back(instRetVal); + } + return result; + } + + /// Expand templates, imperative ! + void expandTemplate(const TemplateSubstitution& ts) { + // substitute template in arguments + argLists_ = expandArgumentListsTemplate(ts); + // do the same for return types + returnVals_ = expandReturnValuesTemplate(ts); + } + + // emit a list of comments, one for each overload + void usage_fragment(FileWriter& proxyFile, const std::string& name) const { + unsigned int argLCount = 0; + BOOST_FOREACH(ArgumentList argList, argLists_) { + argList.emit_prototype(proxyFile, name); + if (argLCount != nrOverloads() - 1) + proxyFile.oss << ", "; + else + proxyFile.oss << " : returns " << returnValue(0).return_type(false) + << std::endl; + argLCount++; + } + } + + // emit a list of comments, one for each overload + void comment_fragment(FileWriter& proxyFile, const std::string& name) const { + size_t i = 0; + BOOST_FOREACH(ArgumentList argList, argLists_) { + proxyFile.oss << "%"; + argList.emit_prototype(proxyFile, name); + proxyFile.oss << " : returns " << returnVals_[i++].return_type(false) + << std::endl; + } + } + + friend std::ostream& operator<<(std::ostream& os, + const SignatureOverloads& overloads) { + for (size_t i = 0; i < overloads.nrOverloads(); i++) + os << overloads.returnVals_[i] << overloads.argLists_[i] << std::endl; + return os; + } + +}; + +class FullyOverloadedFunction: public Function, public SignatureOverloads { + +public: + + bool addOverload(const std::string& name, const ArgumentList& args, + const ReturnValue& retVal, const Qualified& instName = Qualified(), + bool verbose = false) { + bool first = initializeOrCheck(name, instName, verbose); + SignatureOverloads::push_back(args, retVal); + return first; + } + +}; + +// Templated checking functions +// TODO: do this via polymorphism, use transform ? + +template +inline void verifyReturnTypes(const std::vector& validTypes, + const std::map& vt) { + typedef typename std::map::value_type NamedMethod; + BOOST_FOREACH(const NamedMethod& namedMethod, vt) + namedMethod.second.verifyReturnTypes(validTypes); +} + +} // \namespace wrap + diff --git a/wrap/Function.cpp b/wrap/Function.cpp index ab3958c62..6faa70fb9 100644 --- a/wrap/Function.cpp +++ b/wrap/Function.cpp @@ -29,38 +29,29 @@ using namespace std; using namespace wrap; /* ************************************************************************* */ -void Function::addOverload(bool verbose, const std::string& name, - const Qualified& instName) { +bool Function::initializeOrCheck(const std::string& name, + const Qualified& instName, bool verbose) { + + if (name.empty()) + throw std::runtime_error( + "Function::initializeOrCheck called with empty name"); // Check if this overload is give to the correct method - if (name_.empty()) + if (name_.empty()) { name_ = name; - else if (name_ != name) - throw std::runtime_error( - "Function::addOverload: tried to add overload with name " + name - + " instead of expected " + name_); - - // Check if this overload is give to the correct method - if (templateArgValue_.empty()) templateArgValue_ = instName; - else if (templateArgValue_ != instName) - throw std::runtime_error( - "Function::addOverload: tried to add overload with template argument " - + instName.qualifiedName(":") + " instead of expected " - + templateArgValue_.qualifiedName(":")); + verbose_ = verbose; + return true; + } else { + if (name_ != name || templateArgValue_ != instName || verbose_ != verbose) + throw std::runtime_error( + "Function::initializeOrCheck called with different arguments: with name " + + name + " instead of expected " + name_ + + ", or with template argument " + instName.qualifiedName(":") + + " instead of expected " + templateArgValue_.qualifiedName(":")); - verbose_ = verbose; -} - -/* ************************************************************************* */ -vector ArgumentOverloads::expandArgumentListsTemplate( - const TemplateSubstitution& ts) const { - vector result; - BOOST_FOREACH(const ArgumentList& argList, argLists_) { - ArgumentList instArgList = argList.expandTemplate(ts); - result.push_back(instArgList); + return false; } - return result; } /* ************************************************************************* */ diff --git a/wrap/Function.h b/wrap/Function.h index 1d40fcbea..49a26bd8d 100644 --- a/wrap/Function.h +++ b/wrap/Function.h @@ -19,11 +19,6 @@ #pragma once #include "Argument.h" -#include "ReturnValue.h" -#include "TypeAttributesTable.h" - -#include -#include namespace wrap { @@ -32,20 +27,18 @@ class Function { protected: - bool verbose_; std::string name_; ///< name of method Qualified templateArgValue_; ///< value of template argument if applicable + bool verbose_; public: - /// Constructor creates empty object - Function(bool verbose = true) : - verbose_(verbose) { - } - - Function(const std::string& name, bool verbose = true) : - verbose_(verbose), name_(name) { - } + /** + * @brief first time, fill in instance variables, otherwise check if same + * @return true if first time, false thereafter + */ + bool initializeOrCheck(const std::string& name, const Qualified& instName = + Qualified(), bool verbose = false); std::string name() const { return name_; @@ -57,172 +50,7 @@ public: else return name_ + templateArgValue_.name; } - - // The first time this function is called, it initializes the class members - // with those in rhs, but in subsequent calls it adds additional argument - // lists as function overloads. - void addOverload(bool verbose, const std::string& name, - const Qualified& instName = Qualified()); }; -/** - * ArgumentList Overloads - */ -class ArgumentOverloads { - -protected: - - std::vector argLists_; - -public: - - size_t nrOverloads() const { - return argLists_.size(); - } - - const ArgumentList& argumentList(size_t i) const { - return argLists_.at(i); - } - - void addOverload(const ArgumentList& args) { - argLists_.push_back(args); - } - - std::vector expandArgumentListsTemplate( - const TemplateSubstitution& ts) const; - - /// Expand templates, imperative ! - virtual void ExpandTemplate(const TemplateSubstitution& ts) { - argLists_ = expandArgumentListsTemplate(ts); - } - - void verifyArguments(const std::vector& validArgs, - const std::string s) const { - BOOST_FOREACH(const ArgumentList& argList, argLists_) { - BOOST_FOREACH(Argument arg, argList) { - std::string fullType = arg.type.qualifiedName("::"); - if (find(validArgs.begin(), validArgs.end(), fullType) - == validArgs.end()) - throw DependencyMissing(fullType, "checking argument of " + s); - } - } - } - - friend std::ostream& operator<<(std::ostream& os, - const ArgumentOverloads& overloads) { - BOOST_FOREACH(const ArgumentList& argList, overloads.argLists_) - os << argList << std::endl; - return os; - } - -}; - -/** - * Signature Overload (including return value) - */ -class SignatureOverloads: public ArgumentOverloads { - -protected: - - std::vector returnVals_; - -public: - - const ReturnValue& returnValue(size_t i) const { - return returnVals_.at(i); - } - - void addOverload(const ArgumentList& args, const ReturnValue& retVal) { - argLists_.push_back(args); - returnVals_.push_back(retVal); - } - - void verifyReturnTypes(const std::vector& validtypes, - const std::string& s) const { - BOOST_FOREACH(const ReturnValue& retval, returnVals_) { - retval.type1.verify(validtypes, s); - if (retval.isPair) - retval.type2.verify(validtypes, s); - } - } - - // TODO use transform ? - std::vector expandReturnValuesTemplate( - const TemplateSubstitution& ts) const { - std::vector result; - BOOST_FOREACH(const ReturnValue& retVal, returnVals_) { - ReturnValue instRetVal = retVal.expandTemplate(ts); - result.push_back(instRetVal); - } - return result; - } - - /// Expand templates, imperative ! - void expandTemplate(const TemplateSubstitution& ts) { - // substitute template in arguments - argLists_ = expandArgumentListsTemplate(ts); - // do the same for return types - returnVals_ = expandReturnValuesTemplate(ts); - } - - // emit a list of comments, one for each overload - void usage_fragment(FileWriter& proxyFile, const std::string& name) const { - unsigned int argLCount = 0; - BOOST_FOREACH(ArgumentList argList, argLists_) { - argList.emit_prototype(proxyFile, name); - if (argLCount != nrOverloads() - 1) - proxyFile.oss << ", "; - else - proxyFile.oss << " : returns " << returnValue(0).return_type(false) - << std::endl; - argLCount++; - } - } - - // emit a list of comments, one for each overload - void comment_fragment(FileWriter& proxyFile, const std::string& name) const { - size_t i = 0; - BOOST_FOREACH(ArgumentList argList, argLists_) { - proxyFile.oss << "%"; - argList.emit_prototype(proxyFile, name); - proxyFile.oss << " : returns " << returnVals_[i++].return_type(false) - << std::endl; - } - } - - friend std::ostream& operator<<(std::ostream& os, - const SignatureOverloads& overloads) { - for (size_t i = 0; i < overloads.nrOverloads(); i++) - os << overloads.returnVals_[i] << overloads.argLists_[i] << std::endl; - return os; - } - -}; - -// Templated checking functions -// TODO: do this via polymorphism ? - -// TODO use transform -template -static std::map expandMethodTemplate( - const std::map& methods, const TemplateSubstitution& ts) { - std::map result; - typedef std::pair NamedMethod; - BOOST_FOREACH(NamedMethod namedMethod, methods) { - F instMethod = namedMethod.second; - instMethod.expandTemplate(ts); - namedMethod.second = instMethod; - result.insert(namedMethod); - } - return result; -} -template -inline void verifyReturnTypes(const std::vector& validTypes, - const std::map& vt) { - typedef typename std::map::value_type NamedMethod; - BOOST_FOREACH(const NamedMethod& namedMethod, vt) - namedMethod.second.verifyReturnTypes(validTypes); -} - } // \namespace wrap diff --git a/wrap/GlobalFunction.cpp b/wrap/GlobalFunction.cpp index 1f9d6518e..916189632 100644 --- a/wrap/GlobalFunction.cpp +++ b/wrap/GlobalFunction.cpp @@ -16,18 +16,18 @@ namespace wrap { using namespace std; /* ************************************************************************* */ -void GlobalFunction::addOverload(bool verbose, const Qualified& overload, +void GlobalFunction::addOverload(const Qualified& overload, const ArgumentList& args, const ReturnValue& retVal, - const Qualified& instName) { - Function::addOverload(verbose, overload.name, instName); - SignatureOverloads::addOverload(args, retVal); + const Qualified& instName, bool verbose) { + string name(overload.name); + FullyOverloadedFunction::addOverload(name, args, retVal, instName, verbose); overloads.push_back(overload); } /* ************************************************************************* */ -void GlobalFunction::matlab_proxy(const std::string& toolboxPath, - const std::string& wrapperName, const TypeAttributesTable& typeAttributes, - FileWriter& file, std::vector& functionNames) const { +void GlobalFunction::matlab_proxy(const string& toolboxPath, + const string& wrapperName, const TypeAttributesTable& typeAttributes, + FileWriter& file, vector& functionNames) const { // cluster overloads with same namespace // create new GlobalFunction structures around namespaces - same namespaces and names are overloads @@ -40,8 +40,7 @@ void GlobalFunction::matlab_proxy(const std::string& toolboxPath, string str_ns = qualifiedName("", overload.namespaces); const ReturnValue& ret = returnValue(i); const ArgumentList& args = argumentList(i); - grouped_functions[str_ns].addOverload(verbose_, overload, args, ret, - templateArgValue_); + grouped_functions[str_ns].addOverload(overload, args, ret); } size_t lastcheck = grouped_functions.size(); @@ -54,9 +53,9 @@ void GlobalFunction::matlab_proxy(const std::string& toolboxPath, } /* ************************************************************************* */ -void GlobalFunction::generateSingleFunction(const std::string& toolboxPath, - const std::string& wrapperName, const TypeAttributesTable& typeAttributes, - FileWriter& file, std::vector& functionNames) const { +void GlobalFunction::generateSingleFunction(const string& toolboxPath, + const string& wrapperName, const TypeAttributesTable& typeAttributes, + FileWriter& file, vector& functionNames) const { // create the folder for the namespace const Qualified& overload1 = overloads.front(); diff --git a/wrap/GlobalFunction.h b/wrap/GlobalFunction.h index 18bb91995..a086e8154 100644 --- a/wrap/GlobalFunction.h +++ b/wrap/GlobalFunction.h @@ -9,23 +9,18 @@ #pragma once -#include "Function.h" +#include "FullyOverloadedFunction.h" namespace wrap { -struct GlobalFunction: public Function, public SignatureOverloads { +struct GlobalFunction: public FullyOverloadedFunction { std::vector overloads; ///< Stack of qualified names - // Constructor only used in Module - GlobalFunction(bool verbose = true) : - Function(verbose) { - } - - // Used to reconstruct - GlobalFunction(const std::string& name, bool verbose = true) : - Function(name,verbose) { - } + // adds an overloaded version of this function, + void addOverload(const Qualified& overload, const ArgumentList& args, + const ReturnValue& retVal, const Qualified& instName = Qualified(), + bool verbose = false); void verifyArguments(const std::vector& validArgs) const { SignatureOverloads::verifyArguments(validArgs, name_); @@ -35,11 +30,6 @@ struct GlobalFunction: public Function, public SignatureOverloads { SignatureOverloads::verifyReturnTypes(validtypes, name_); } - // adds an overloaded version of this function, - void addOverload(bool verbose, const Qualified& overload, - const ArgumentList& args, const ReturnValue& retVal, - const Qualified& instName = Qualified()); - // codegen function called from Module to build the cpp and matlab versions of the function void matlab_proxy(const std::string& toolboxPath, const std::string& wrapperName, const TypeAttributesTable& typeAttributes, diff --git a/wrap/Method.cpp b/wrap/Method.cpp index a7072c9e7..49d90378d 100644 --- a/wrap/Method.cpp +++ b/wrap/Method.cpp @@ -29,17 +29,25 @@ using namespace std; using namespace wrap; /* ************************************************************************* */ -void Method::addOverload(bool verbose, bool is_const, Str name, - const ArgumentList& args, const ReturnValue& retVal, - const Qualified& instName) { - - StaticMethod::addOverload(verbose, name, args, retVal, instName); - is_const_ = is_const; +bool Method::addOverload(Str name, const ArgumentList& args, + const ReturnValue& retVal, bool is_const, const Qualified& instName, + bool verbose) { + bool first = StaticMethod::addOverload(name, args, retVal, instName, verbose); + if (first) + is_const_ = is_const; + else if (is_const && !is_const_) + throw std::runtime_error( + "Method::addOverload now designated as const whereas before it was not"); + else if (!is_const && is_const_) + throw std::runtime_error( + "Method::addOverload now designated as non-const whereas before it was"); + return first; } /* ************************************************************************* */ void Method::proxy_header(FileWriter& proxyFile) const { - proxyFile.oss << " function varargout = " << matlabName() << "(this, varargin)\n"; + proxyFile.oss << " function varargout = " << matlabName() + << "(this, varargin)\n"; } /* ************************************************************************* */ diff --git a/wrap/Method.h b/wrap/Method.h index 13847700d..db9e6bb9f 100644 --- a/wrap/Method.h +++ b/wrap/Method.h @@ -31,14 +31,9 @@ public: typedef const std::string& Str; - /// Constructor creates empty object - Method(bool verbose = true) : - StaticMethod(verbose), is_const_(false) { - } - - Method(const std::string& name, bool verbose = true) : - StaticMethod(name,verbose), is_const_(false) { - } + bool addOverload(Str name, const ArgumentList& args, + const ReturnValue& retVal, bool is_const, const Qualified& instName = + Qualified(), bool verbose = false); virtual bool isStatic() const { return false; @@ -48,13 +43,6 @@ public: return is_const_; } - // The first time this function is called, it initializes the class members - // with those in rhs, but in subsequent calls it adds additional argument - // lists as function overloads. - void addOverload(bool verbose, bool is_const, Str name, - const ArgumentList& args, const ReturnValue& retVal, - const Qualified& instName = Qualified()); - friend std::ostream& operator<<(std::ostream& os, const Method& m) { for (size_t i = 0; i < m.nrOverloads(); i++) os << m.returnVals_[i] << " " << m.name_ << m.argLists_[i]; diff --git a/wrap/Module.cpp b/wrap/Module.cpp index 2fc8f92bc..2f35f0bbf 100644 --- a/wrap/Module.cpp +++ b/wrap/Module.cpp @@ -217,7 +217,7 @@ void Module::parseMarkup(const std::string& data) { Constructor constructor0(verbose), constructor(verbose); Rule constructor_p = (className_p >> '(' >> argumentList_p >> ')' >> ';' >> !comments_p) - [bl::bind(&Constructor::addOverload, bl::var(constructor), bl::var(args))] + [bl::bind(&Constructor::push_back, bl::var(constructor), bl::var(args))] [clear_a(args)]; vector namespaces_return; /// namespace for current return type @@ -274,7 +274,7 @@ void Module::parseMarkup(const std::string& data) { '(' >> argumentList_p >> ')' >> ';' >> *comments_p) [bl::bind(&StaticMethod::addOverload, bl::var(cls.static_methods)[bl::var(methodName)], - verbose, bl::var(methodName), bl::var(args), bl::var(retVal), Qualified())] + bl::var(methodName), bl::var(args), bl::var(retVal), Qualified(),verbose)] [assign_a(retVal,retVal0)] [clear_a(args)]; @@ -295,7 +295,8 @@ void Module::parseMarkup(const std::string& data) { >> ((':' >> classParent_p >> '{') | '{') >> *(functions_p | comments_p) >> str_p("};")) - [assign_a(constructor.name, cls.name)] + [bl::bind(&Constructor::initializeOrCheck, bl::var(constructor), + bl::var(cls.name), Qualified(), verbose)] [assign_a(cls.constructor, constructor)] [assign_a(cls.namespaces, namespaces)] [assign_a(cls.deconstructor.name,cls.name)] @@ -313,7 +314,7 @@ void Module::parseMarkup(const std::string& data) { [assign_a(globalFunction.namespaces,namespaces)] [bl::bind(&GlobalFunction::addOverload, bl::var(global_functions)[bl::var(globalFunction.name)], - verbose, bl::var(globalFunction), bl::var(args), bl::var(retVal), Qualified())] + bl::var(globalFunction), bl::var(args), bl::var(retVal), Qualified(),verbose)] [assign_a(retVal,retVal0)] [clear_a(globalFunction)] [clear_a(args)]; diff --git a/wrap/OverloadedFunction.h b/wrap/OverloadedFunction.h new file mode 100644 index 000000000..47c418748 --- /dev/null +++ b/wrap/OverloadedFunction.h @@ -0,0 +1,126 @@ +/* ---------------------------------------------------------------------------- + + * 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 OverloadedFunction.h + * @brief Function that can overload its arguments only + * @author Frank Dellaert + * @date Nov 13, 2014 + **/ + +#pragma once + +#include "Function.h" +#include "Argument.h" + +namespace wrap { + +/** + * ArgumentList Overloads + */ +class ArgumentOverloads { + +protected: + + std::vector argLists_; + +public: + + size_t nrOverloads() const { + return argLists_.size(); + } + + const ArgumentList& argumentList(size_t i) const { + return argLists_.at(i); + } + + void push_back(const ArgumentList& args) { + argLists_.push_back(args); + } + + std::vector expandArgumentListsTemplate( + const TemplateSubstitution& ts) const { + std::vector result; + BOOST_FOREACH(const ArgumentList& argList, argLists_) { + ArgumentList instArgList = argList.expandTemplate(ts); + result.push_back(instArgList); + } + return result; + } + + /// Expand templates, imperative ! + virtual void ExpandTemplate(const TemplateSubstitution& ts) { + argLists_ = expandArgumentListsTemplate(ts); + } + + void verifyArguments(const std::vector& validArgs, + const std::string s) const { + BOOST_FOREACH(const ArgumentList& argList, argLists_) { + BOOST_FOREACH(Argument arg, argList) { + std::string fullType = arg.type.qualifiedName("::"); + if (find(validArgs.begin(), validArgs.end(), fullType) + == validArgs.end()) + throw DependencyMissing(fullType, "checking argument of " + s); + } + } + } + + friend std::ostream& operator<<(std::ostream& os, + const ArgumentOverloads& overloads) { + BOOST_FOREACH(const ArgumentList& argList, overloads.argLists_) + os << argList << std::endl; + return os; + } + +}; + +class OverloadedFunction: public Function, public ArgumentOverloads { + +public: + + bool addOverload(const std::string& name, const ArgumentList& args, + const Qualified& instName = Qualified(), bool verbose = false) { + bool first = initializeOrCheck(name, instName, verbose); + ArgumentOverloads::push_back(args); + return first; + } + +private: + +}; + +// Templated checking functions +// TODO: do this via polymorphism, use transform ? + +template +static std::map expandMethodTemplate( + const std::map& methods, const TemplateSubstitution& ts) { + std::map result; + typedef std::pair NamedMethod; + BOOST_FOREACH(NamedMethod namedMethod, methods) { + F instMethod = namedMethod.second; + instMethod.expandTemplate(ts); + namedMethod.second = instMethod; + result.insert(namedMethod); + } + return result; +} + +template +inline void verifyArguments(const std::vector& validArgs, + const std::map& vt) { + typedef typename std::map::value_type NamedMethod; + BOOST_FOREACH(const NamedMethod& namedMethod, vt) + namedMethod.second.verifyArguments(validArgs); +} + +} // \namespace wrap + diff --git a/wrap/StaticMethod.cpp b/wrap/StaticMethod.cpp index d3bd75628..83cf621b4 100644 --- a/wrap/StaticMethod.cpp +++ b/wrap/StaticMethod.cpp @@ -29,14 +29,6 @@ using namespace std; using namespace wrap; -/* ************************************************************************* */ -void StaticMethod::addOverload(bool verbose, Str name, const ArgumentList& args, - const ReturnValue& retVal, const Qualified& instName) { - - Function::addOverload(verbose, name, instName); - SignatureOverloads::addOverload(args, retVal); -} - /* ************************************************************************* */ void StaticMethod::proxy_header(FileWriter& proxyFile) const { string upperName = matlabName(); diff --git a/wrap/StaticMethod.h b/wrap/StaticMethod.h index 4a6fedbfc..06f98092f 100644 --- a/wrap/StaticMethod.h +++ b/wrap/StaticMethod.h @@ -19,31 +19,19 @@ #pragma once -#include "Function.h" +#include "FullyOverloadedFunction.h" namespace wrap { /// StaticMethod class -struct StaticMethod: public Function, public SignatureOverloads { +struct StaticMethod: public FullyOverloadedFunction { typedef const std::string& Str; - /// Constructor creates empty object - StaticMethod(bool verbosity = true) : - Function(verbosity) { - } - - StaticMethod(const std::string& name, bool verbose = true) : - Function(name,verbose) { - } - virtual bool isStatic() const { return true; } - void addOverload(bool verbose, Str name, const ArgumentList& args, - const ReturnValue& retVal, const Qualified& instName); - // emit a list of comments, one for each overload void comment_fragment(FileWriter& proxyFile) const { SignatureOverloads::comment_fragment(proxyFile, matlabName()); diff --git a/wrap/tests/testMethod.cpp b/wrap/tests/testMethod.cpp index 4067f3d85..8050f0d3c 100644 --- a/wrap/tests/testMethod.cpp +++ b/wrap/tests/testMethod.cpp @@ -32,12 +32,13 @@ TEST( Method, Constructor ) { /* ************************************************************************* */ // addOverload TEST( Method, addOverload ) { - Method method("myName"); - bool verbose = true, is_const = true; + Method method; + method.initializeOrCheck("myName"); + bool is_const = true; ArgumentList args; const ReturnValue retVal(ReturnType("return_type")); - method.addOverload(verbose, is_const, "myName", args, retVal); - EXPECT_LONGS_EQUAL(1,method.nrOverloads()); + method.addOverload("myName", args, retVal, is_const); + EXPECT_LONGS_EQUAL(1, method.nrOverloads()); } /* ************************************************************************* */ From e07402a58a77ad99856e665019111b078ca14da4 Mon Sep 17 00:00:00 2001 From: dellaert Date: Fri, 14 Nov 2014 17:04:45 +0100 Subject: [PATCH 02/19] Re-factored matlab_code only emits code: it does not post-process the classes anymore. That is now done in parse_Markup, i.e., the constructor.... --- wrap/Module.cpp | 70 ++++++++++++++++++++--------------------- wrap/Module.h | 50 +++++++++++++++++------------ wrap/tests/testWrap.cpp | 15 +++++---- 3 files changed, 71 insertions(+), 64 deletions(-) diff --git a/wrap/Module.cpp b/wrap/Module.cpp index 2f35f0bbf..3c228bd4a 100644 --- a/wrap/Module.cpp +++ b/wrap/Module.cpp @@ -394,51 +394,35 @@ void Module::parseMarkup(const std::string& data) { // Explicitly add methods to the classes from parents so it shows in documentation BOOST_FOREACH(Class& cls, classes) cls.appendInheritedMethods(cls, classes); -} - -/* ************************************************************************* */ -void Module::generateIncludes(FileWriter& file) const { - - // collect includes - vector all_includes(includes); - - // sort and remove duplicates - sort(all_includes.begin(), all_includes.end()); - vector::const_iterator last_include = unique(all_includes.begin(), all_includes.end()); - vector::const_iterator it = all_includes.begin(); - // add includes to file - for (; it != last_include; ++it) - file.oss << "#include <" << *it << ">" << endl; - file.oss << "\n"; -} - - -/* ************************************************************************* */ -void Module::matlab_code(const string& toolboxPath, const string& headerPath) const { - - fs::create_directories(toolboxPath); // Expand templates - This is done first so that template instantiations are // counted in the list of valid types, have their attributes and dependencies // checked, etc. - vector expandedClasses = ExpandTypedefInstantiations(classes, templateInstantiationTypedefs); + expandedClasses = ExpandTypedefInstantiations(classes, + templateInstantiationTypedefs); // Dependency check list - vector validTypes = GenerateValidTypes(expandedClasses, forward_declarations); + vector validTypes = GenerateValidTypes(expandedClasses, + forward_declarations); // Check that all classes have been defined somewhere verifyArguments(validTypes, global_functions); verifyReturnTypes(validTypes, global_functions); - bool hasSerialiable = false; + hasSerialiable = false; BOOST_FOREACH(const Class& cls, expandedClasses) cls.verifyAll(validTypes,hasSerialiable); // Create type attributes table and check validity - TypeAttributesTable typeAttributes; typeAttributes.addClasses(expandedClasses); typeAttributes.addForwardDeclarations(forward_declarations); typeAttributes.checkValidity(expandedClasses); +} + +/* ************************************************************************* */ +void Module::matlab_code(const string& toolboxPath) const { + + fs::create_directories(toolboxPath); // create the unified .cpp switch file const string wrapperName = name + "_wrapper"; @@ -459,19 +443,18 @@ void Module::matlab_code(const string& toolboxPath, const string& headerPath) co // Generate includes while avoiding redundant includes generateIncludes(wrapperFile); - // create typedef classes - we put this at the top of the wrap file so that collectors and method arguments can use these typedefs - BOOST_FOREACH(const Class& cls, expandedClasses) { + // create typedef classes - we put this at the top of the wrap file so that + // collectors and method arguments can use these typedefs + BOOST_FOREACH(const Class& cls, expandedClasses) if(!cls.typedefName.empty()) wrapperFile.oss << cls.getTypedef() << "\n"; - } wrapperFile.oss << "\n"; // Generate boost.serialization export flags (needs typedefs from above) if (hasSerialiable) { - BOOST_FOREACH(const Class& cls, expandedClasses) { + BOOST_FOREACH(const Class& cls, expandedClasses) if(cls.isSerializable) wrapperFile.oss << cls.getSerializationExport() << "\n"; - } wrapperFile.oss << "\n"; } @@ -484,14 +467,12 @@ void Module::matlab_code(const string& toolboxPath, const string& headerPath) co vector functionNames; // Function names stored by index for switch // create proxy class and wrapper code - BOOST_FOREACH(const Class& cls, expandedClasses) { + BOOST_FOREACH(const Class& cls, expandedClasses) cls.matlab_proxy(toolboxPath, wrapperName, typeAttributes, wrapperFile, functionNames); - } // create matlab files and wrapper code for global functions - BOOST_FOREACH(const GlobalFunctions::value_type& p, global_functions) { + BOOST_FOREACH(const GlobalFunctions::value_type& p, global_functions) p.second.matlab_proxy(toolboxPath, wrapperName, typeAttributes, wrapperFile, functionNames); - } // finish wrapper file wrapperFile.oss << "\n"; @@ -501,6 +482,23 @@ void Module::matlab_code(const string& toolboxPath, const string& headerPath) co } /* ************************************************************************* */ +void Module::generateIncludes(FileWriter& file) const { + + // collect includes + vector all_includes(includes); + + // sort and remove duplicates + sort(all_includes.begin(), all_includes.end()); + vector::const_iterator last_include = unique(all_includes.begin(), all_includes.end()); + vector::const_iterator it = all_includes.begin(); + // add includes to file + for (; it != last_include; ++it) + file.oss << "#include <" << *it << ">" << endl; + file.oss << "\n"; +} + + +/* ************************************************************************* */ void Module::finish_wrapper(FileWriter& file, const std::vector& functionNames) const { file.oss << "void mexFunction(int nargout, mxArray *out[], int nargin, const mxArray *in[])\n"; file.oss << "{\n"; diff --git a/wrap/Module.h b/wrap/Module.h index 8ef2bc4fd..1709719d1 100644 --- a/wrap/Module.h +++ b/wrap/Module.h @@ -37,40 +37,50 @@ struct Module { typedef std::map GlobalFunctions; typedef std::map Methods; - std::string name; ///< module name - bool verbose; ///< verbose flag + // Filled during parsing: + std::string name; ///< module name + bool verbose; ///< verbose flag std::vector classes; ///< list of classes std::vector templateInstantiationTypedefs; ///< list of template instantiations std::vector forward_declarations; - std::vector includes; ///< Include statements + std::vector includes; ///< Include statements GlobalFunctions global_functions; + // After parsing: + std::vector expandedClasses; + bool hasSerialiable; + TypeAttributesTable typeAttributes; + /// constructor that parses interface file - Module(const std::string& interfacePath, - const std::string& moduleName, - bool enable_verbose=true); + Module(const std::string& interfacePath, const std::string& moduleName, + bool enable_verbose = true); /// Dummy constructor that does no parsing - use only for testing - Module(const std::string& moduleName, bool enable_verbose=true); - - /// MATLAB code generation: - void matlab_code( - const std::string& path, - const std::string& headerPath) const; // FIXME: headerPath not actually used? - - void finish_wrapper(FileWriter& file, const std::vector& functionNames) const; - - void generateIncludes(FileWriter& file) const; + Module(const std::string& moduleName, bool enable_verbose = true); /// non-const function that performs parsing - typically called by constructor /// Throws exception on failure void parseMarkup(const std::string& data); + /// MATLAB code generation: + void matlab_code(const std::string& path) const; + + void generateIncludes(FileWriter& file) const; + + void finish_wrapper(FileWriter& file, + const std::vector& functionNames) const; + private: - static std::vector ExpandTypedefInstantiations(const std::vector& classes, const std::vector instantiations); - static std::vector GenerateValidTypes(const std::vector& classes, const std::vector forwardDeclarations); - static void WriteCollectorsAndCleanupFcn(FileWriter& wrapperFile, const std::string& moduleName, const std::vector& classes); - static void WriteRTTIRegistry(FileWriter& wrapperFile, const std::string& moduleName, const std::vector& classes); + static std::vector ExpandTypedefInstantiations( + const std::vector& classes, + const std::vector instantiations); + static std::vector GenerateValidTypes( + const std::vector& classes, + const std::vector forwardDeclarations); + static void WriteCollectorsAndCleanupFcn(FileWriter& wrapperFile, + const std::string& moduleName, const std::vector& classes); + static void WriteRTTIRegistry(FileWriter& wrapperFile, + const std::string& moduleName, const std::vector& classes); }; } // \namespace wrap diff --git a/wrap/tests/testWrap.cpp b/wrap/tests/testWrap.cpp index 8fe862182..003801b05 100644 --- a/wrap/tests/testWrap.cpp +++ b/wrap/tests/testWrap.cpp @@ -64,12 +64,11 @@ TEST( wrap, check_exception ) { THROWS_EXCEPTION(Module("/notarealpath", "geometry",enable_verbose)); CHECK_EXCEPTION(Module("/alsonotarealpath", "geometry",enable_verbose), CantOpenFile); - // clean out previous generated code - fs::remove_all("actual_deps"); - - string path = topdir + "/wrap/tests"; - Module module(path.c_str(), "testDependencies",enable_verbose); - CHECK_EXCEPTION(module.matlab_code("actual_deps", headerPath), DependencyMissing); +// // TODO: matlab_code does not throw this anymore, so check constructor +// fs::remove_all("actual_deps"); // clean out previous generated code +// string path = topdir + "/wrap/tests"; +// Module module(path.c_str(), "testDependencies",enable_verbose); +// CHECK_EXCEPTION(module.matlab_code("actual_deps"), DependencyMissing); } /* ************************************************************************* */ @@ -413,7 +412,7 @@ TEST( wrap, matlab_code_namespaces ) { // emit MATLAB code string exp_path = path + "/tests/expected_namespaces/"; string act_path = "actual_namespaces/"; - module.matlab_code("actual_namespaces", headerPath); + module.matlab_code("actual_namespaces"); EXPECT(files_equal(exp_path + "ClassD.m", act_path + "ClassD.m" )); @@ -441,7 +440,7 @@ TEST( wrap, matlab_code_geometry ) { // emit MATLAB code // make_geometry will not compile, use make testwrap to generate real make - module.matlab_code("actual", headerPath); + module.matlab_code("actual"); #ifndef WRAP_DISABLE_SERIALIZE string epath = path + "/tests/expected/"; #else From 6c24fc2aca018d37143b8182a11eac2ee4a9db2c Mon Sep 17 00:00:00 2001 From: dellaert Date: Fri, 14 Nov 2014 17:47:25 +0100 Subject: [PATCH 03/19] Python prototype --- wrap/Class.cpp | 12 +++++++ wrap/Class.h | 3 ++ wrap/Constructor.cpp | 74 +++++++++++++++++++++++++---------------- wrap/Constructor.h | 18 ++++++---- wrap/GlobalFunction.cpp | 5 +++ wrap/GlobalFunction.h | 3 ++ wrap/Module.cpp | 28 ++++++++++++++++ wrap/Module.h | 3 ++ wrap/StaticMethod.cpp | 7 ++++ wrap/StaticMethod.h | 3 ++ wrap/tests/testWrap.cpp | 19 +++++++++++ 11 files changed, 139 insertions(+), 36 deletions(-) diff --git a/wrap/Class.cpp b/wrap/Class.cpp index 9c759bb62..0e480f0fd 100644 --- a/wrap/Class.cpp +++ b/wrap/Class.cpp @@ -583,4 +583,16 @@ string Class::getSerializationExport() const { return "BOOST_CLASS_EXPORT_GUID(" + qualifiedName("::") + ", \"" + qualifiedName() + "\");"; } + +/* ************************************************************************* */ +void Class::python_wrapper(FileWriter& wrapperFile) const { + wrapperFile.oss << "class_<" << name << ">(\"" << name << "\")\n"; + constructor.python_wrapper(wrapperFile, name); + BOOST_FOREACH(const StaticMethod& m, static_methods | boost::adaptors::map_values) + m.python_wrapper(wrapperFile, name); + BOOST_FOREACH(const Method& m, methods | boost::adaptors::map_values) + m.python_wrapper(wrapperFile, name); + wrapperFile.oss << ";\n\n"; +} + /* ************************************************************************* */ diff --git a/wrap/Class.h b/wrap/Class.h index 6b9119316..34323b797 100644 --- a/wrap/Class.h +++ b/wrap/Class.h @@ -111,6 +111,9 @@ public: void deserialization_fragments(FileWriter& proxyFile, FileWriter& wrapperFile, Str wrapperName, std::vector& functionNames) const; + // emit python wrapper + void python_wrapper(FileWriter& wrapperFile) const; + friend std::ostream& operator<<(std::ostream& os, const Class& cls) { os << "class " << cls.name << "{\n"; os << cls.constructor << ";\n"; diff --git a/wrap/Constructor.cpp b/wrap/Constructor.cpp index fdbbf0e42..782c0ca80 100644 --- a/wrap/Constructor.cpp +++ b/wrap/Constructor.cpp @@ -29,52 +29,55 @@ using namespace std; using namespace wrap; - /* ************************************************************************* */ -string Constructor::matlab_wrapper_name(const string& className) const { +string Constructor::matlab_wrapper_name(Str className) const { string str = "new_" + className; return str; } /* ************************************************************************* */ -void Constructor::proxy_fragment(FileWriter& file, const std::string& wrapperName, - bool hasParent, const int id, const ArgumentList args) const { +void Constructor::proxy_fragment(FileWriter& file, + const std::string& wrapperName, bool hasParent, const int id, + const ArgumentList args) const { size_t nrArgs = args.size(); // check for number of arguments... file.oss << " elseif nargin == " << nrArgs; - if (nrArgs>0) file.oss << " && "; + if (nrArgs > 0) + file.oss << " && "; // ...and their types bool first = true; - for(size_t i=0;i(id); + const string wrapFunctionName = matlabUniqueName + "_constructor_" + + boost::lexical_cast(id); - file.oss << "void " << wrapFunctionName << "(int nargout, mxArray *out[], int nargin, const mxArray *in[])" << endl; + file.oss << "void " << wrapFunctionName + << "(int nargout, mxArray *out[], int nargin, const mxArray *in[])" + << endl; file.oss << "{\n"; file.oss << " mexAtExit(&_deleteAllObjects);\n"; //Typedef boost::shared_ptr @@ -82,22 +85,29 @@ string Constructor::wrapper_fragment(FileWriter& file, file.oss << "\n"; //Check to see if there will be any arguments and remove {} for consiseness - if(al.size() > 0) + if (al.size() > 0) al.matlab_unwrap(file); // unwrap arguments - file.oss << " Shared *self = new Shared(new " << cppClassName << "(" << al.names() << "));" << endl; + file.oss << " Shared *self = new Shared(new " << cppClassName << "(" + << al.names() << "));" << endl; file.oss << " collector_" << matlabUniqueName << ".insert(self);\n"; - if(verbose_) + if (verbose_) file.oss << " std::cout << \"constructed \" << self << std::endl;" << endl; - file.oss << " out[0] = mxCreateNumericMatrix(1, 1, mxUINT32OR64_CLASS, mxREAL);" << endl; - file.oss << " *reinterpret_cast (mxGetData(out[0])) = self;" << endl; + file.oss + << " out[0] = mxCreateNumericMatrix(1, 1, mxUINT32OR64_CLASS, mxREAL);" + << endl; + file.oss << " *reinterpret_cast (mxGetData(out[0])) = self;" + << endl; // If we have a base class, return the base class pointer (MATLAB will call the base class collectorInsertAndMakeBase to add this to the collector and recurse the heirarchy) - if(!cppBaseClassName.empty()) { + if (!cppBaseClassName.empty()) { file.oss << "\n"; - file.oss << " typedef boost::shared_ptr<" << cppBaseClassName << "> SharedBase;\n"; - file.oss << " out[1] = mxCreateNumericMatrix(1, 1, mxUINT32OR64_CLASS, mxREAL);\n"; - file.oss << " *reinterpret_cast(mxGetData(out[1])) = new SharedBase(*self);\n"; + file.oss << " typedef boost::shared_ptr<" << cppBaseClassName + << "> SharedBase;\n"; + file.oss + << " out[1] = mxCreateNumericMatrix(1, 1, mxUINT32OR64_CLASS, mxREAL);\n"; + file.oss + << " *reinterpret_cast(mxGetData(out[1])) = new SharedBase(*self);\n"; } file.oss << "}" << endl; @@ -106,3 +116,9 @@ string Constructor::wrapper_fragment(FileWriter& file, } /* ************************************************************************* */ +void Constructor::python_wrapper(FileWriter& wrapperFile, Str className) const { + wrapperFile.oss << " .def(\"" << name_ << "\", &" << className << "::" << name_ + << ");\n"; +} + +/* ************************************************************************* */ diff --git a/wrap/Constructor.h b/wrap/Constructor.h index 870f1a15e..a026bf423 100644 --- a/wrap/Constructor.h +++ b/wrap/Constructor.h @@ -28,6 +28,8 @@ namespace wrap { // Constructor class struct Constructor: public OverloadedFunction { + typedef const std::string& Str; + /// Constructor creates an empty class Constructor(bool verbose = false) { verbose_ = verbose; @@ -45,7 +47,7 @@ struct Constructor: public OverloadedFunction { // classFile is class proxy file, e.g., ../matlab/@Point2/Point2.m /// wrapper name - std::string matlab_wrapper_name(const std::string& className) const; + std::string matlab_wrapper_name(Str className) const; void comment_fragment(FileWriter& proxyFile) const { if (nrOverloads() > 0) @@ -61,19 +63,21 @@ struct Constructor: public OverloadedFunction { * Create fragment to select constructor in proxy class, e.g., * if nargin == 2, obj.self = new_Pose3_RP(varargin{1},varargin{2}); end */ - void proxy_fragment(FileWriter& file, const std::string& wrapperName, - bool hasParent, const int id, const ArgumentList args) const; + void proxy_fragment(FileWriter& file, Str wrapperName, bool hasParent, + const int id, const ArgumentList args) const; /// cpp wrapper - std::string wrapper_fragment(FileWriter& file, - const std::string& cppClassName, const std::string& matlabUniqueName, - const std::string& cppBaseClassName, int id, + std::string wrapper_fragment(FileWriter& file, Str cppClassName, + Str matlabUniqueName, Str cppBaseClassName, int id, const ArgumentList& al) const; /// constructor function - void generate_construct(FileWriter& file, const std::string& cppClassName, + void generate_construct(FileWriter& file, Str cppClassName, std::vector& args_list) const; + // emit python wrapper + void python_wrapper(FileWriter& wrapperFile, Str className) const; + friend std::ostream& operator<<(std::ostream& os, const Constructor& m) { for (size_t i = 0; i < m.nrOverloads(); i++) os << m.name_ << m.argLists_[i]; diff --git a/wrap/GlobalFunction.cpp b/wrap/GlobalFunction.cpp index 916189632..e843481a1 100644 --- a/wrap/GlobalFunction.cpp +++ b/wrap/GlobalFunction.cpp @@ -127,6 +127,11 @@ void GlobalFunction::generateSingleFunction(const string& toolboxPath, mfunctionFile.emit(true); } +/* ************************************************************************* */ +void GlobalFunction::python_wrapper(FileWriter& wrapperFile) const { + wrapperFile.oss << "def(\"" << name_ << "\", " << name_ << ");\n"; +} + /* ************************************************************************* */ } // \namespace wrap diff --git a/wrap/GlobalFunction.h b/wrap/GlobalFunction.h index a086e8154..6a49fe5ce 100644 --- a/wrap/GlobalFunction.h +++ b/wrap/GlobalFunction.h @@ -35,6 +35,9 @@ struct GlobalFunction: public FullyOverloadedFunction { const std::string& wrapperName, const TypeAttributesTable& typeAttributes, FileWriter& file, std::vector& functionNames) const; + // emit python wrapper + void python_wrapper(FileWriter& wrapperFile) const; + private: // Creates a single global function - all in same namespace diff --git a/wrap/Module.cpp b/wrap/Module.cpp index 3c228bd4a..ac0e0a579 100644 --- a/wrap/Module.cpp +++ b/wrap/Module.cpp @@ -650,3 +650,31 @@ void Module::WriteRTTIRegistry(FileWriter& wrapperFile, const std::string& modul } /* ************************************************************************* */ +void Module::python_wrapper(const string& toolboxPath) const { + + fs::create_directories(toolboxPath); + + // create the unified .cpp switch file + const string wrapperName = name + "_python"; + string wrapperFileName = toolboxPath + "/" + wrapperName + ".cpp"; + FileWriter wrapperFile(wrapperFileName, verbose, "//"); + wrapperFile.oss << "#include \n\n"; + wrapperFile.oss << "using namespace boost::python;\n"; + wrapperFile.oss << "BOOST_PYTHON_MODULE(" + name + ")\n"; + wrapperFile.oss << "{\n"; + + // write out classes + BOOST_FOREACH(const Class& cls, expandedClasses) + cls.python_wrapper(wrapperFile); + + // write out global functions + BOOST_FOREACH(const GlobalFunctions::value_type& p, global_functions) + p.second.python_wrapper(wrapperFile); + + // finish wrapper file + wrapperFile.oss << "}\n"; + + wrapperFile.emit(true); +} + +/* ************************************************************************* */ diff --git a/wrap/Module.h b/wrap/Module.h index 1709719d1..a4659dc3a 100644 --- a/wrap/Module.h +++ b/wrap/Module.h @@ -70,6 +70,9 @@ struct Module { void finish_wrapper(FileWriter& file, const std::vector& functionNames) const; + /// Python code generation: + void python_wrapper(const std::string& path) const; + private: static std::vector ExpandTypedefInstantiations( const std::vector& classes, diff --git a/wrap/StaticMethod.cpp b/wrap/StaticMethod.cpp index 83cf621b4..5f91a15b4 100644 --- a/wrap/StaticMethod.cpp +++ b/wrap/StaticMethod.cpp @@ -165,3 +165,10 @@ string StaticMethod::wrapper_call(FileWriter& wrapperFile, Str cppClassName, } /* ************************************************************************* */ +void StaticMethod::python_wrapper(FileWriter& wrapperFile, + Str className) const { + wrapperFile.oss << " .def(\"" << name_ << "\", &" << className << "::" << name_ + << ");\n"; +} + +/* ************************************************************************* */ diff --git a/wrap/StaticMethod.h b/wrap/StaticMethod.h index 06f98092f..de8e4a94e 100644 --- a/wrap/StaticMethod.h +++ b/wrap/StaticMethod.h @@ -52,6 +52,9 @@ struct StaticMethod: public FullyOverloadedFunction { Str wrapperName, const TypeAttributesTable& typeAttributes, std::vector& functionNames) const; + // emit python wrapper + void python_wrapper(FileWriter& wrapperFile, Str className) const; + friend std::ostream& operator<<(std::ostream& os, const StaticMethod& m) { for (size_t i = 0; i < m.nrOverloads(); i++) os << "static " << m.returnVals_[i] << " " << m.name_ << m.argLists_[i]; diff --git a/wrap/tests/testWrap.cpp b/wrap/tests/testWrap.cpp index 003801b05..0645fb407 100644 --- a/wrap/tests/testWrap.cpp +++ b/wrap/tests/testWrap.cpp @@ -460,6 +460,25 @@ TEST( wrap, matlab_code_geometry ) { EXPECT(files_equal(epath + "overloadedGlobalFunction.m" , apath + "overloadedGlobalFunction.m" )); } +/* ************************************************************************* */ +TEST( wrap, python_code_geometry ) { + // Parse into class object + string header_path = topdir + "/wrap/tests"; + Module module(header_path,"geometry",enable_verbose); + string path = topdir + "/wrap"; + + // clean out previous generated code + fs::remove_all("actual-python"); + + // emit MATLAB code + // make_geometry will not compile, use make testwrap to generate real make + module.python_wrapper("actual-python"); + string epath = path + "/tests/expected-python/"; + string apath = "actual-python/"; + + EXPECT(files_equal(epath + "geometry_python.cpp", apath + "geometry_python.cpp" )); +} + /* ************************************************************************* */ int main() { TestResult tr; return TestRegistry::runAllTests(tr); } /* ************************************************************************* */ From 755cc60b6faa327fb4723cf32f8fc82ce4458cc8 Mon Sep 17 00:00:00 2001 From: dellaert Date: Fri, 14 Nov 2014 17:47:46 +0100 Subject: [PATCH 04/19] python wrapper file generated at this point --- .../tests/expected-python/geometry_python.cpp | 83 +++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 wrap/tests/expected-python/geometry_python.cpp diff --git a/wrap/tests/expected-python/geometry_python.cpp b/wrap/tests/expected-python/geometry_python.cpp new file mode 100644 index 000000000..f500f2984 --- /dev/null +++ b/wrap/tests/expected-python/geometry_python.cpp @@ -0,0 +1,83 @@ +#include + +using namespace boost::python; +BOOST_PYTHON_MODULE(geometry) +{ +class_("Point2") + .def("Point2", &Point2::Point2); + .def("argChar", &Point2::argChar); + .def("argUChar", &Point2::argUChar); + .def("dim", &Point2::dim); + .def("returnChar", &Point2::returnChar); + .def("vectorConfusion", &Point2::vectorConfusion); + .def("x", &Point2::x); + .def("y", &Point2::y); +; + +class_("Point3") + .def("Point3", &Point3::Point3); + .def("StaticFunctionRet", &Point3::StaticFunctionRet); + .def("staticFunction", &Point3::staticFunction); + .def("norm", &Point3::norm); +; + +class_("Test") + .def("Test", &Test::Test); + .def("arg_EigenConstRef", &Test::arg_EigenConstRef); + .def("create_MixedPtrs", &Test::create_MixedPtrs); + .def("create_ptrs", &Test::create_ptrs); + .def("print", &Test::print); + .def("return_Point2Ptr", &Test::return_Point2Ptr); + .def("return_Test", &Test::return_Test); + .def("return_TestPtr", &Test::return_TestPtr); + .def("return_bool", &Test::return_bool); + .def("return_double", &Test::return_double); + .def("return_field", &Test::return_field); + .def("return_int", &Test::return_int); + .def("return_matrix1", &Test::return_matrix1); + .def("return_matrix2", &Test::return_matrix2); + .def("return_pair", &Test::return_pair); + .def("return_ptrs", &Test::return_ptrs); + .def("return_size_t", &Test::return_size_t); + .def("return_string", &Test::return_string); + .def("return_vector1", &Test::return_vector1); + .def("return_vector2", &Test::return_vector2); +; + +class_("MyBase") + .def("MyBase", &MyBase::MyBase); +; + +class_("MyTemplatePoint2") + .def("MyTemplatePoint2", &MyTemplatePoint2::MyTemplatePoint2); + .def("accept_T", &MyTemplatePoint2::accept_T); + .def("accept_Tptr", &MyTemplatePoint2::accept_Tptr); + .def("create_MixedPtrs", &MyTemplatePoint2::create_MixedPtrs); + .def("create_ptrs", &MyTemplatePoint2::create_ptrs); + .def("return_T", &MyTemplatePoint2::return_T); + .def("return_Tptr", &MyTemplatePoint2::return_Tptr); + .def("return_ptrs", &MyTemplatePoint2::return_ptrs); + .def("templatedMethod", &MyTemplatePoint2::templatedMethod); + .def("templatedMethod", &MyTemplatePoint2::templatedMethod); +; + +class_("MyTemplatePoint3") + .def("MyTemplatePoint3", &MyTemplatePoint3::MyTemplatePoint3); + .def("accept_T", &MyTemplatePoint3::accept_T); + .def("accept_Tptr", &MyTemplatePoint3::accept_Tptr); + .def("create_MixedPtrs", &MyTemplatePoint3::create_MixedPtrs); + .def("create_ptrs", &MyTemplatePoint3::create_ptrs); + .def("return_T", &MyTemplatePoint3::return_T); + .def("return_Tptr", &MyTemplatePoint3::return_Tptr); + .def("return_ptrs", &MyTemplatePoint3::return_ptrs); + .def("templatedMethod", &MyTemplatePoint3::templatedMethod); + .def("templatedMethod", &MyTemplatePoint3::templatedMethod); +; + +class_("MyFactorPosePoint2") + .def("MyFactorPosePoint2", &MyFactorPosePoint2::MyFactorPosePoint2); +; + +def("aGlobalFunction", aGlobalFunction); +def("overloadedGlobalFunction", overloadedGlobalFunction); +} From 4966f5a94202a40ce2cd4981d7372165fdeb3664 Mon Sep 17 00:00:00 2001 From: HannesSommer Date: Sat, 15 Nov 2014 09:07:30 +0100 Subject: [PATCH 05/19] 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 06/19] 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 290f0ab8a4efbb352612e6f91aeb5806206b58d3 Mon Sep 17 00:00:00 2001 From: Jing Dong Date: Wed, 19 Nov 2014 13:29:38 -0500 Subject: [PATCH 07/19] propose patch to fix Values constructor from ConstFiltered, and unit tests --- gtsam/nonlinear/Values-inl.h | 2 +- gtsam/nonlinear/tests/testValues.cpp | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/gtsam/nonlinear/Values-inl.h b/gtsam/nonlinear/Values-inl.h index 0d559cfe6..4595a70ed 100644 --- a/gtsam/nonlinear/Values-inl.h +++ b/gtsam/nonlinear/Values-inl.h @@ -215,7 +215,7 @@ namespace gtsam { Values::Values(const Values::ConstFiltered& view) { BOOST_FOREACH(const typename ConstFiltered::KeyValuePair& key_value, view) { Key key = key_value.key; - insert(key, key_value.value); + insert(key, static_cast(key_value.value)); } } diff --git a/gtsam/nonlinear/tests/testValues.cpp b/gtsam/nonlinear/tests/testValues.cpp index 60639e8b7..219cacfd1 100644 --- a/gtsam/nonlinear/tests/testValues.cpp +++ b/gtsam/nonlinear/tests/testValues.cpp @@ -383,6 +383,12 @@ TEST(Values, filter) { expectedSubValues1.insert(3, pose3); EXPECT(assert_equal(expectedSubValues1, actualSubValues1)); + // ConstFilter by Key + Values::ConstFiltered constfiltered = values.filter(boost::bind(std::greater_equal(), _1, 2)); + EXPECT_LONGS_EQUAL(2, (long)constfiltered.size()); + Values fromconstfiltered(constfiltered); + EXPECT(assert_equal(expectedSubValues1, fromconstfiltered)); + // Filter by type i = 0; Values::ConstFiltered pose_filtered = values.filter(); From 2983cf33a6c2aa3f8f3b9d24273fac600ea65fbc Mon Sep 17 00:00:00 2001 From: dellaert Date: Fri, 21 Nov 2014 15:48:02 +0100 Subject: [PATCH 08/19] 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 09/19] 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 018e66be7f6280ef99e960056e8ba5e84c570391 Mon Sep 17 00:00:00 2001 From: dellaert Date: Fri, 21 Nov 2014 16:56:03 +0100 Subject: [PATCH 10/19] Fixed compile issue --- wrap/wrap.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wrap/wrap.cpp b/wrap/wrap.cpp index 37b8e36b1..2e5ac1612 100644 --- a/wrap/wrap.cpp +++ b/wrap/wrap.cpp @@ -40,7 +40,7 @@ void generate_matlab_toolbox( wrap::Module module(interfacePath, moduleName, false); // Then emit MATLAB code - module.matlab_code(toolboxPath,headerPath); + module.matlab_code(toolboxPath); } /** Displays usage information */ From f699dd26bb50dac035a937061bedfeb57ecaeb03 Mon Sep 17 00:00:00 2001 From: HannesSommer Date: Fri, 21 Nov 2014 21:09:03 +0100 Subject: [PATCH 11/19] 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 12/19] - 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 13/19] 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 14/19] 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 15/19] 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 16/19] 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 17/19] 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; From ab08cb65b00518aa3d12a7411c9df457c55a8b31 Mon Sep 17 00:00:00 2001 From: dellaert Date: Sat, 22 Nov 2014 22:13:21 +0100 Subject: [PATCH 18/19] Fixed unit test --- wrap/tests/testMethod.cpp | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/wrap/tests/testMethod.cpp b/wrap/tests/testMethod.cpp index 8050f0d3c..5861bab4a 100644 --- a/wrap/tests/testMethod.cpp +++ b/wrap/tests/testMethod.cpp @@ -33,12 +33,13 @@ TEST( Method, Constructor ) { // addOverload TEST( Method, addOverload ) { Method method; - method.initializeOrCheck("myName"); - bool is_const = true; ArgumentList args; - const ReturnValue retVal(ReturnType("return_type")); - method.addOverload("myName", args, retVal, is_const); - EXPECT_LONGS_EQUAL(1, method.nrOverloads()); + bool is_const = true; + const ReturnValue retVal1(ReturnType("return_type1")); + method.addOverload("myName", args, retVal1, is_const); + const ReturnValue retVal2(ReturnType("return_type2")); + method.addOverload("myName", args, retVal2, is_const); + EXPECT_LONGS_EQUAL(2, method.nrOverloads()); } /* ************************************************************************* */ From db3126cd141aa21d4584554e2f2a3d3e72cb5327 Mon Sep 17 00:00:00 2001 From: dellaert Date: Sat, 22 Nov 2014 22:31:28 +0100 Subject: [PATCH 19/19] Fixed evaluateError signature (double -> const double&) --- gtsam/navigation/MagFactor.h | 2 +- gtsam/navigation/tests/testMagFactor.cpp | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gtsam/navigation/MagFactor.h b/gtsam/navigation/MagFactor.h index 96a0971c5..9abd70e58 100644 --- a/gtsam/navigation/MagFactor.h +++ b/gtsam/navigation/MagFactor.h @@ -189,7 +189,7 @@ public: * @param nM (unknown) local earth magnetic field vector, in nav frame * @param bias (unknown) 3D bias */ - Vector evaluateError(double scale, const Unit3& direction, + Vector evaluateError(const double& scale, const Unit3& direction, const Point3& bias, boost::optional H1 = boost::none, boost::optional H2 = boost::none, boost::optional H3 = boost::none) const { diff --git a/gtsam/navigation/tests/testMagFactor.cpp b/gtsam/navigation/tests/testMagFactor.cpp index 5599c93d6..80f67c58d 100644 --- a/gtsam/navigation/tests/testMagFactor.cpp +++ b/gtsam/navigation/tests/testMagFactor.cpp @@ -48,7 +48,7 @@ Point3 bias(10, -10, 50); Point3 scaled = scale * nM; Point3 measured = nRb.inverse() * (scale * nM) + bias; -LieScalar s(scale * nM.norm()); +double s(scale * nM.norm()); Unit3 dir(nM); SharedNoiseModel model = noiseModel::Isotropic::Sigma(3, 0.25); @@ -94,7 +94,7 @@ TEST( MagFactor, Factors ) { // MagFactor2 MagFactor3 f3(1, 2, 3, measured, nRb, model); EXPECT(assert_equal(zero(3),f3.evaluateError(s,dir,bias,H1,H2,H3),1e-5)); - EXPECT(assert_equal(numericalDerivative11 // + EXPECT(assert_equal(numericalDerivative11 // (boost::bind(&MagFactor3::evaluateError, &f3, _1, dir, bias, none, none, none), s),// H1, 1e-7)); EXPECT(assert_equal(numericalDerivative11 //