From d0ff3ab97ecdfb8466eae676a096baf55693aacd Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Sat, 22 Jan 2022 12:40:29 -0500 Subject: [PATCH] Fix most lint errors --- gtsam/discrete/DecisionTree-inl.h | 166 +++++++++++++++--------------- gtsam/discrete/DecisionTree.h | 70 +++++++------ 2 files changed, 121 insertions(+), 115 deletions(-) diff --git a/gtsam/discrete/DecisionTree-inl.h b/gtsam/discrete/DecisionTree-inl.h index 84116ccd5..01c7b689c 100644 --- a/gtsam/discrete/DecisionTree-inl.h +++ b/gtsam/discrete/DecisionTree-inl.h @@ -21,42 +21,44 @@ #include +#include #include #include +#include #include #include #include #include #include -#include #include #include #include +#include +#include #include +#include +#include using boost::assign::operator+=; namespace gtsam { - /*********************************************************************************/ + /****************************************************************************/ // Node - /*********************************************************************************/ + /****************************************************************************/ #ifdef DT_DEBUG_MEMORY template int DecisionTree::Node::nrNodes = 0; #endif - /*********************************************************************************/ + /****************************************************************************/ // Leaf - /*********************************************************************************/ - template - class DecisionTree::Leaf: public DecisionTree::Node { - + /****************************************************************************/ + template + struct DecisionTree::Leaf : public DecisionTree::Node { /** constant stored in this leaf */ Y constant_; - public: - /** Constructor from constant */ Leaf(const Y& constant) : constant_(constant) {} @@ -96,7 +98,7 @@ namespace gtsam { std::string value = valueFormatter(constant_); if (showZero || value.compare("0")) os << "\"" << this->id() << "\" [label=\"" << value - << "\", shape=box, rank=sink, height=0.35, fixedsize=true]\n"; // width=0.55, + << "\", shape=box, rank=sink, height=0.35, fixedsize=true]\n"; } /** evaluate */ @@ -121,13 +123,13 @@ namespace gtsam { // Applying binary operator to two leaves results in a leaf NodePtr apply_g_op_fL(const Leaf& fL, const Binary& op) const override { - NodePtr h(new Leaf(op(fL.constant_, constant_))); // fL op gL + NodePtr h(new Leaf(op(fL.constant_, constant_))); // fL op gL return h; } // If second argument is a Choice node, call it's apply with leaf as second NodePtr apply_g_op_fC(const Choice& fC, const Binary& op) const override { - return fC.apply_fC_op_gL(*this, op); // operand order back to normal + return fC.apply_fC_op_gL(*this, op); // operand order back to normal } /** choose a branch, create new memory ! */ @@ -136,32 +138,30 @@ namespace gtsam { } bool isLeaf() const override { return true; } + }; // Leaf - }; // Leaf - - /*********************************************************************************/ + /****************************************************************************/ // Choice - /*********************************************************************************/ + /****************************************************************************/ template - class DecisionTree::Choice: public DecisionTree::Node { - + struct DecisionTree::Choice: public DecisionTree::Node { /** the label of the variable on which we split */ L label_; /** The children of this Choice node. */ std::vector branches_; - private: + private: /** incremental allSame */ size_t allSame_; using ChoicePtr = boost::shared_ptr; - public: - + public: ~Choice() override { #ifdef DT_DEBUG_MEMORY - std::std::cout << Node::nrNodes << " destructing (Choice) " << this->id() << std::std::endl; + std::std::cout << Node::nrNodes << " destructing (Choice) " << this->id() + << std::std::endl; #endif } @@ -172,7 +172,8 @@ namespace gtsam { assert(f->branches().size() > 0); NodePtr f0 = f->branches_[0]; assert(f0->isLeaf()); - NodePtr newLeaf(new Leaf(boost::dynamic_pointer_cast(f0)->constant())); + NodePtr newLeaf( + new Leaf(boost::dynamic_pointer_cast(f0)->constant())); return newLeaf; } else #endif @@ -192,7 +193,6 @@ namespace gtsam { */ Choice(const Choice& f, const Choice& g, const Binary& op) : allSame_(true) { - // Choose what to do based on label if (f.label() > g.label()) { // f higher than g @@ -318,10 +318,8 @@ namespace gtsam { */ Choice(const L& label, const Choice& f, const Unary& op) : label_(label), allSame_(true) { - - branches_.reserve(f.branches_.size()); // reserve space - for (const NodePtr& branch: f.branches_) - push_back(branch->apply(op)); + branches_.reserve(f.branches_.size()); // reserve space + for (const NodePtr& branch : f.branches_) push_back(branch->apply(op)); } /** apply unary operator */ @@ -364,8 +362,7 @@ namespace gtsam { /** choose a branch, recursively */ NodePtr choose(const L& label, size_t index) const override { - if (label_ == label) - return branches_[index]; // choose branch + if (label_ == label) return branches_[index]; // choose branch // second case, not label of interest, just recurse auto r = boost::make_shared(label_, branches_.size()); @@ -373,12 +370,11 @@ namespace gtsam { r->push_back(branch->choose(label, index)); return Unique(r); } + }; // Choice - }; // Choice - - /*********************************************************************************/ + /****************************************************************************/ // DecisionTree - /*********************************************************************************/ + /****************************************************************************/ template DecisionTree::DecisionTree() { } @@ -388,13 +384,13 @@ namespace gtsam { root_(root) { } - /*********************************************************************************/ + /****************************************************************************/ template DecisionTree::DecisionTree(const Y& y) { root_ = NodePtr(new Leaf(y)); } - /*********************************************************************************/ + /****************************************************************************/ template DecisionTree::DecisionTree(const L& label, const Y& y1, const Y& y2) { auto a = boost::make_shared(label, 2); @@ -404,7 +400,7 @@ namespace gtsam { root_ = Choice::Unique(a); } - /*********************************************************************************/ + /****************************************************************************/ template DecisionTree::DecisionTree(const LabelC& labelC, const Y& y1, const Y& y2) { @@ -417,7 +413,7 @@ namespace gtsam { root_ = Choice::Unique(a); } - /*********************************************************************************/ + /****************************************************************************/ template DecisionTree::DecisionTree(const std::vector& labelCs, const std::vector& ys) { @@ -425,29 +421,28 @@ namespace gtsam { root_ = create(labelCs.begin(), labelCs.end(), ys.begin(), ys.end()); } - /*********************************************************************************/ + /****************************************************************************/ template DecisionTree::DecisionTree(const std::vector& labelCs, const std::string& table) { - // Convert std::string to values of type Y std::vector ys; std::istringstream iss(table); copy(std::istream_iterator(iss), std::istream_iterator(), - back_inserter(ys)); + back_inserter(ys)); // now call recursive Create root_ = create(labelCs.begin(), labelCs.end(), ys.begin(), ys.end()); } - /*********************************************************************************/ + /****************************************************************************/ template template DecisionTree::DecisionTree( Iterator begin, Iterator end, const L& label) { root_ = compose(begin, end, label); } - /*********************************************************************************/ + /****************************************************************************/ template DecisionTree::DecisionTree(const L& label, const DecisionTree& f0, const DecisionTree& f1) { @@ -456,17 +451,17 @@ namespace gtsam { root_ = compose(functions.begin(), functions.end(), label); } - /*********************************************************************************/ + /****************************************************************************/ template template DecisionTree::DecisionTree(const DecisionTree& other, Func Y_of_X) { // Define functor for identity mapping of node label. - auto L_of_L = [](const L& label) { return label; }; + auto L_of_L = [](const L& label) { return label; }; root_ = convertFrom(other.root_, L_of_L, Y_of_X); } - /*********************************************************************************/ + /****************************************************************************/ template template DecisionTree::DecisionTree(const DecisionTree& other, @@ -475,16 +470,16 @@ namespace gtsam { root_ = convertFrom(other.root_, L_of_M, Y_of_X); } - /*********************************************************************************/ + /****************************************************************************/ // Called by two constructors above. - // Takes a label and a corresponding range of decision trees, and creates a new - // decision tree. However, the order of the labels needs to be respected, so we - // cannot just create a root Choice node on the label: if the label is not the - // highest label, we need to do a complicated and expensive recursive call. - template template - typename DecisionTree::NodePtr DecisionTree::compose(Iterator begin, - Iterator end, const L& label) const { - + // Takes a label and a corresponding range of decision trees, and creates a + // new decision tree. However, the order of the labels needs to be respected, + // so we cannot just create a root Choice node on the label: if the label is + // not the highest label, we need a complicated/ expensive recursive call. + template + template + typename DecisionTree::NodePtr DecisionTree::compose( + Iterator begin, Iterator end, const L& label) const { // find highest label among branches boost::optional highestLabel; size_t nrChoices = 0; @@ -527,7 +522,7 @@ namespace gtsam { } } - /*********************************************************************************/ + /****************************************************************************/ // "create" is a bit of a complicated thing, but very useful. // It takes a range of labels and a corresponding range of values, // and creates a decision tree, as follows: @@ -552,7 +547,6 @@ namespace gtsam { template typename DecisionTree::NodePtr DecisionTree::create( It begin, It end, ValueIt beginY, ValueIt endY) const { - // get crucial counts size_t nrChoices = begin->second; size_t size = endY - beginY; @@ -564,7 +558,11 @@ namespace gtsam { // Create a simple choice node with values as leaves. if (size != nrChoices) { std::cout << "Trying to create DD on " << begin->first << std::endl; - std::cout << boost::format("DecisionTree::create: expected %d values but got %d instead") % nrChoices % size << std::endl; + std::cout << boost::format( + "DecisionTree::create: expected %d values but got %d " + "instead") % + nrChoices % size + << std::endl; throw std::invalid_argument("DecisionTree::create invalid argument"); } auto choice = boost::make_shared(begin->first, endY - beginY); @@ -585,7 +583,7 @@ namespace gtsam { return compose(functions.begin(), functions.end(), begin->first); } - /*********************************************************************************/ + /****************************************************************************/ template template typename DecisionTree::NodePtr DecisionTree::convertFrom( @@ -594,11 +592,11 @@ namespace gtsam { std::function Y_of_X) const { using LY = DecisionTree; - // ugliness below because apparently we can't have templated virtual functions - // If leaf, apply unary conversion "op" and create a unique leaf + // ugliness below because apparently we can't have templated virtual + // functions If leaf, apply unary conversion "op" and create a unique leaf using MXLeaf = typename DecisionTree::Leaf; if (auto leaf = boost::dynamic_pointer_cast(f)) - return NodePtr(new Leaf(Y_of_X(leaf->constant()))); + return NodePtr(new Leaf(Y_of_X(leaf->constant()))); // Check if Choice using MXChoice = typename DecisionTree::Choice; @@ -612,19 +610,19 @@ namespace gtsam { // put together via Shannon expansion otherwise not sorted. std::vector functions; - for(auto && branch: choice->branches()) { + for (auto&& branch : choice->branches()) { functions.emplace_back(convertFrom(branch, L_of_M, Y_of_X)); } return LY::compose(functions.begin(), functions.end(), newLabel); } - /*********************************************************************************/ + /****************************************************************************/ // Functor performing depth-first visit without Assignment argument. template struct Visit { using F = std::function; - Visit(F f) : f(f) {} ///< Construct from folding function. - F f; ///< folding function object. + explicit Visit(F f) : f(f) {} ///< Construct from folding function. + F f; ///< folding function object. /// Do a depth-first visit on the tree rooted at node. void operator()(const typename DecisionTree::NodePtr& node) const { @@ -647,15 +645,15 @@ namespace gtsam { visit(root_); } - /*********************************************************************************/ + /****************************************************************************/ // Functor performing depth-first visit with Assignment argument. template struct VisitWith { using Choices = Assignment; using F = std::function; - VisitWith(F f) : f(f) {} ///< Construct from folding function. - Choices choices; ///< Assignment, mutating through recursion. - F f; ///< folding function object. + explicit VisitWith(F f) : f(f) {} ///< Construct from folding function. + Choices choices; ///< Assignment, mutating through recursion. + F f; ///< folding function object. /// Do a depth-first visit on the tree rooted at node. void operator()(const typename DecisionTree::NodePtr& node) { @@ -681,7 +679,7 @@ namespace gtsam { visit(root_); } - /*********************************************************************************/ + /****************************************************************************/ // fold is just done with a visit template template @@ -690,7 +688,7 @@ namespace gtsam { return x0; } - /*********************************************************************************/ + /****************************************************************************/ // labels is just done with a visit template std::set DecisionTree::labels() const { @@ -702,7 +700,7 @@ namespace gtsam { return unique; } -/*********************************************************************************/ +/****************************************************************************/ template bool DecisionTree::equals(const DecisionTree& other, const CompareFunc& compare) const { @@ -736,7 +734,7 @@ namespace gtsam { return DecisionTree(root_->apply(op)); } - /*********************************************************************************/ + /****************************************************************************/ template DecisionTree DecisionTree::apply(const DecisionTree& g, const Binary& op) const { @@ -752,7 +750,7 @@ namespace gtsam { return result; } - /*********************************************************************************/ + /****************************************************************************/ // The way this works: // We have an ADT, picture it as a tree. // At a certain depth, we have a branch on "label". @@ -772,7 +770,7 @@ namespace gtsam { return result; } - /*********************************************************************************/ + /****************************************************************************/ template void DecisionTree::dot(std::ostream& os, const LabelFormatter& labelFormatter, @@ -790,9 +788,11 @@ namespace gtsam { bool showZero) const { std::ofstream os((name + ".dot").c_str()); dot(os, labelFormatter, valueFormatter, showZero); - int result = system( - ("dot -Tpdf " + name + ".dot -o " + name + ".pdf >& /dev/null").c_str()); - if (result==-1) throw std::runtime_error("DecisionTree::dot system call failed"); + int result = + system(("dot -Tpdf " + name + ".dot -o " + name + ".pdf >& /dev/null") + .c_str()); + if (result == -1) + throw std::runtime_error("DecisionTree::dot system call failed"); } template @@ -804,8 +804,6 @@ namespace gtsam { return ss.str(); } -/*********************************************************************************/ - -} // namespace gtsam - +/******************************************************************************/ + } // namespace gtsam diff --git a/gtsam/discrete/DecisionTree.h b/gtsam/discrete/DecisionTree.h index 78f3a75b7..53782ef5e 100644 --- a/gtsam/discrete/DecisionTree.h +++ b/gtsam/discrete/DecisionTree.h @@ -26,9 +26,11 @@ #include #include #include -#include -#include #include +#include +#include +#include +#include namespace gtsam { @@ -39,15 +41,13 @@ namespace gtsam { */ template class DecisionTree { - protected: /// Default method for comparison of two objects of type Y. static bool DefaultCompare(const Y& a, const Y& b) { return a == b; } - public: - + public: using LabelFormatter = std::function; using ValueFormatter = std::function; using CompareFunc = std::function; @@ -57,15 +57,14 @@ namespace gtsam { using Binary = std::function; /** A label annotated with cardinality */ - using LabelC = std::pair; + using LabelC = std::pair; /** DTs consist of Leaf and Choice nodes, both subclasses of Node */ - class Leaf; - class Choice; + struct Leaf; + struct Choice; /** ------------------------ Node base class --------------------------- */ - class Node { - public: + struct Node { using Ptr = boost::shared_ptr; #ifdef DT_DEBUG_MEMORY @@ -75,14 +74,16 @@ namespace gtsam { // Constructor Node() { #ifdef DT_DEBUG_MEMORY - std::cout << ++nrNodes << " constructed " << id() << std::endl; std::cout.flush(); + std::cout << ++nrNodes << " constructed " << id() << std::endl; + std::cout.flush(); #endif } // Destructor virtual ~Node() { #ifdef DT_DEBUG_MEMORY - std::cout << --nrNodes << " destructed " << id() << std::endl; std::cout.flush(); + std::cout << --nrNodes << " destructed " << id() << std::endl; + std::cout.flush(); #endif } @@ -110,17 +111,17 @@ namespace gtsam { }; /** ------------------------ Node base class --------------------------- */ - public: - + public: /** A function is a shared pointer to the root of a DT */ using NodePtr = typename Node::Ptr; /// A DecisionTree just contains the root. TODO(dellaert): make protected. NodePtr root_; - protected: - - /** Internal recursive function to create from keys, cardinalities, and Y values */ + protected: + /** Internal recursive function to create from keys, cardinalities, + * and Y values + */ template NodePtr create(It begin, It end, ValueIt beginY, ValueIt endY) const; @@ -140,7 +141,6 @@ namespace gtsam { std::function Y_of_X) const; public: - /// @name Standard Constructors /// @{ @@ -148,7 +148,7 @@ namespace gtsam { DecisionTree(); /** Create a constant */ - DecisionTree(const Y& y); + explicit DecisionTree(const Y& y); /** Create a new leaf function splitting on a variable */ DecisionTree(const L& label, const Y& y1, const Y& y2); @@ -167,8 +167,8 @@ namespace gtsam { DecisionTree(Iterator begin, Iterator end, const L& label); /** Create DecisionTree from two others */ - DecisionTree(const L& label, // - const DecisionTree& f0, const DecisionTree& f1); + DecisionTree(const L& label, const DecisionTree& f0, + const DecisionTree& f1); /** * @brief Convert from a different value type. @@ -289,7 +289,8 @@ namespace gtsam { } /** combine subtrees on key with binary operation "op" */ - DecisionTree combine(const L& label, size_t cardinality, const Binary& op) const; + DecisionTree combine(const L& label, size_t cardinality, + const Binary& op) const; /** combine with LabelC for convenience */ DecisionTree combine(const LabelC& labelC, const Binary& op) const { @@ -313,15 +314,14 @@ namespace gtsam { /// @{ // internal use only - DecisionTree(const NodePtr& root); + explicit DecisionTree(const NodePtr& root); // internal use only template NodePtr compose(Iterator begin, Iterator end, const L& label) const; /// @} - - }; // DecisionTree + }; // DecisionTree /** free versions of apply */ @@ -340,11 +340,19 @@ namespace gtsam { return f.apply(g, op); } - /// unzip a DecisionTree if its leaves are `std::pair` - template - std::pair, DecisionTree > unzip(const DecisionTree > &input) { - return std::make_pair(DecisionTree(input, [](std::pair i) { return i.first; }), - DecisionTree(input, [](std::pair i) { return i.second; })); + /** + * @brief unzip a DecisionTree with `std::pair` values. + * + * @param input the DecisionTree with `(T1,T2)` values. + * @return a pair of DecisionTree on T1 and T2, respectively. + */ + template + std::pair, DecisionTree > unzip( + const DecisionTree >& input) { + return std::make_pair( + DecisionTree(input, [](std::pair i) { return i.first; }), + DecisionTree(input, + [](std::pair i) { return i.second; })); } -} // namespace gtsam +} // namespace gtsam