From c27cd7a2e8862afc8c159af02ed545760c90ee37 Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Sat, 25 Dec 2021 13:51:13 -0500 Subject: [PATCH 1/3] switch formatter and writer arguments --- gtsam/discrete/discrete.i | 18 +++++++------ gtsam/inference/DotWriter.h | 15 ++++++----- gtsam/inference/FactorGraph-inst.h | 17 ++++++------ gtsam/inference/FactorGraph.h | 13 ++++----- gtsam/nonlinear/NonlinearFactorGraph.cpp | 23 ++++++++-------- gtsam/nonlinear/NonlinearFactorGraph.h | 34 ++++++++++++++---------- gtsam/nonlinear/nonlinear.i | 13 ++++++++- 7 files changed, 78 insertions(+), 55 deletions(-) diff --git a/gtsam/discrete/discrete.i b/gtsam/discrete/discrete.i index 40569ebbf9..f0dc72a24d 100644 --- a/gtsam/discrete/discrete.i +++ b/gtsam/discrete/discrete.i @@ -133,7 +133,9 @@ class DiscreteBayesTree { #include class DotWriter { - DotWriter(); + DotWriter(double figureWidthInches = 5, double figureHeightInches = 5, + bool plotFactorPoints = true, bool connectKeysToFactor = true, + bool binaryEdges = true); }; #include @@ -153,13 +155,13 @@ class DiscreteFactorGraph { void print(string s = "") const; bool equals(const gtsam::DiscreteFactorGraph& fg, double tol = 1e-9) const; - string dot(const gtsam::DotWriter& dotWriter = gtsam::DotWriter(), - const gtsam::KeyFormatter& keyFormatter = - gtsam::DefaultKeyFormatter) const; - void saveGraph(string s, - const gtsam::DotWriter& dotWriter = gtsam::DotWriter(), - const gtsam::KeyFormatter& keyFormatter = - gtsam::DefaultKeyFormatter) const; + string dot( + const gtsam::KeyFormatter& keyFormatter = gtsam::DefaultKeyFormatter, + const gtsam::DotWriter& dotWriter = gtsam::DotWriter()) const; + void saveGraph( + string s, + const gtsam::KeyFormatter& keyFormatter = gtsam::DefaultKeyFormatter, + const gtsam::DotWriter& dotWriter = gtsam::DotWriter()) const; gtsam::DecisionTreeFactor product() const; double operator()(const gtsam::DiscreteValues& values) const; diff --git a/gtsam/inference/DotWriter.h b/gtsam/inference/DotWriter.h index 8f36f3ce64..bd36da496c 100644 --- a/gtsam/inference/DotWriter.h +++ b/gtsam/inference/DotWriter.h @@ -35,12 +35,15 @@ struct GTSAM_EXPORT DotWriter { ///< the dot of the factor bool binaryEdges; ///< just use non-dotted edges for binary factors - DotWriter() - : figureWidthInches(5), - figureHeightInches(5), - plotFactorPoints(true), - connectKeysToFactor(true), - binaryEdges(true) {} + explicit DotWriter(double figureWidthInches = 5, + double figureHeightInches = 5, + bool plotFactorPoints = true, + bool connectKeysToFactor = true, bool binaryEdges = true) + : figureWidthInches(figureWidthInches), + figureHeightInches(figureHeightInches), + plotFactorPoints(plotFactorPoints), + connectKeysToFactor(connectKeysToFactor), + binaryEdges(binaryEdges) {} /// Write out preamble, including size. void writePreamble(std::ostream* os) const; diff --git a/gtsam/inference/FactorGraph-inst.h b/gtsam/inference/FactorGraph-inst.h index 06b71036c7..058075f2d5 100644 --- a/gtsam/inference/FactorGraph-inst.h +++ b/gtsam/inference/FactorGraph-inst.h @@ -128,8 +128,9 @@ FactorIndices FactorGraph::add_factors(const CONTAINER& factors, /* ************************************************************************* */ template -void FactorGraph::dot(std::ostream& os, const DotWriter& writer, - const KeyFormatter& keyFormatter) const { +void FactorGraph::dot(std::ostream& os, + const KeyFormatter& keyFormatter, + const DotWriter& writer) const { writer.writePreamble(&os); // Create nodes for each variable in the graph @@ -153,20 +154,20 @@ void FactorGraph::dot(std::ostream& os, const DotWriter& writer, /* ************************************************************************* */ template -std::string FactorGraph::dot(const DotWriter& writer, - const KeyFormatter& keyFormatter) const { +std::string FactorGraph::dot(const KeyFormatter& keyFormatter, + const DotWriter& writer) const { std::stringstream ss; - dot(ss, writer, keyFormatter); + dot(ss, keyFormatter, writer); return ss.str(); } /* ************************************************************************* */ template void FactorGraph::saveGraph(const std::string& filename, - const DotWriter& writer, - const KeyFormatter& keyFormatter) const { + const KeyFormatter& keyFormatter, + const DotWriter& writer) const { std::ofstream of(filename.c_str()); - dot(of, writer, keyFormatter); + dot(of, keyFormatter, writer); of.close(); } diff --git a/gtsam/inference/FactorGraph.h b/gtsam/inference/FactorGraph.h index a4c293b648..9c0f10f9a5 100644 --- a/gtsam/inference/FactorGraph.h +++ b/gtsam/inference/FactorGraph.h @@ -378,17 +378,18 @@ class FactorGraph { /// @{ /// Output to graphviz format, stream version. - void dot(std::ostream& os, const DotWriter& writer = DotWriter(), - const KeyFormatter& keyFormatter = DefaultKeyFormatter) const; + void dot(std::ostream& os, + const KeyFormatter& keyFormatter = DefaultKeyFormatter, + const DotWriter& writer = DotWriter()) const; /// Output to graphviz format string. - std::string dot(const DotWriter& writer = DotWriter(), - const KeyFormatter& keyFormatter = DefaultKeyFormatter) const; + std::string dot(const KeyFormatter& keyFormatter = DefaultKeyFormatter, + const DotWriter& writer = DotWriter()) const; /// output to file with graphviz format. void saveGraph(const std::string& filename, - const DotWriter& writer = DotWriter(), - const KeyFormatter& keyFormatter = DefaultKeyFormatter) const; + const KeyFormatter& keyFormatter = DefaultKeyFormatter, + const DotWriter& writer = DotWriter()) const; /// @} /// @name Advanced Interface diff --git a/gtsam/nonlinear/NonlinearFactorGraph.cpp b/gtsam/nonlinear/NonlinearFactorGraph.cpp index 7c6d2e6cf0..0d1ed31487 100644 --- a/gtsam/nonlinear/NonlinearFactorGraph.cpp +++ b/gtsam/nonlinear/NonlinearFactorGraph.cpp @@ -26,7 +26,6 @@ #include #include #include -#include #include #include // for GTSAM_USE_TBB @@ -92,8 +91,8 @@ bool NonlinearFactorGraph::equals(const NonlinearFactorGraph& other, double tol) /* ************************************************************************* */ void NonlinearFactorGraph::dot(std::ostream& os, const Values& values, - const GraphvizFormatting& writer, - const KeyFormatter& keyFormatter) const { + const KeyFormatter& keyFormatter, + const GraphvizFormatting& writer) const { writer.writePreamble(&os); // Find bounds (imperative) @@ -139,21 +138,21 @@ void NonlinearFactorGraph::dot(std::ostream& os, const Values& values, } /* ************************************************************************* */ -std::string NonlinearFactorGraph::dot( - const Values& values, const GraphvizFormatting& writer, - const KeyFormatter& keyFormatter) const { +std::string NonlinearFactorGraph::dot(const Values& values, + const KeyFormatter& keyFormatter, + const GraphvizFormatting& writer) const { std::stringstream ss; - dot(ss, values, writer, keyFormatter); + dot(ss, values, keyFormatter, writer); return ss.str(); } /* ************************************************************************* */ -void NonlinearFactorGraph::saveGraph( - const std::string& filename, const Values& values, - const GraphvizFormatting& writer, - const KeyFormatter& keyFormatter) const { +void NonlinearFactorGraph::saveGraph(const std::string& filename, + const Values& values, + const KeyFormatter& keyFormatter, + const GraphvizFormatting& writer) const { std::ofstream of(filename); - dot(of, values, writer, keyFormatter); + dot(of, values, keyFormatter, writer); of.close(); } diff --git a/gtsam/nonlinear/NonlinearFactorGraph.h b/gtsam/nonlinear/NonlinearFactorGraph.h index c958d63023..2fad561be0 100644 --- a/gtsam/nonlinear/NonlinearFactorGraph.h +++ b/gtsam/nonlinear/NonlinearFactorGraph.h @@ -213,23 +213,22 @@ namespace gtsam { using FactorGraph::saveGraph; /// Output to graphviz format, stream version, with Values/extra options. - void dot( - std::ostream& os, const Values& values, - const GraphvizFormatting& graphvizFormatting = GraphvizFormatting(), - const KeyFormatter& keyFormatter = DefaultKeyFormatter) const; + void dot(std::ostream& os, const Values& values, + const KeyFormatter& keyFormatter = DefaultKeyFormatter, + const GraphvizFormatting& graphvizFormatting = + GraphvizFormatting()) const; /// Output to graphviz format string, with Values/extra options. - std::string dot( - const Values& values, - const GraphvizFormatting& graphvizFormatting = GraphvizFormatting(), - const KeyFormatter& keyFormatter = DefaultKeyFormatter) const; + std::string dot(const Values& values, + const KeyFormatter& keyFormatter = DefaultKeyFormatter, + const GraphvizFormatting& graphvizFormatting = + GraphvizFormatting()) const; /// output to file with graphviz format, with Values/extra options. - void saveGraph( - const std::string& filename, const Values& values, - const GraphvizFormatting& graphvizFormatting = GraphvizFormatting(), - const KeyFormatter& keyFormatter = DefaultKeyFormatter) const; - + void saveGraph(const std::string& filename, const Values& values, + const KeyFormatter& keyFormatter = DefaultKeyFormatter, + const GraphvizFormatting& graphvizFormatting = + GraphvizFormatting()) const; /// @} private: @@ -267,7 +266,14 @@ namespace gtsam { std::ostream& os, const Values& values = Values(), const GraphvizFormatting& graphvizFormatting = GraphvizFormatting(), const KeyFormatter& keyFormatter = DefaultKeyFormatter) const { - dot(os, values, graphvizFormatting, keyFormatter); + dot(os, values, keyFormatter, graphvizFormatting); + } + /** \deprecated */ + void GTSAM_DEPRECATED saveGraph( + const std::string& filename, const Values& values, + const GraphvizFormatting& graphvizFormatting = GraphvizFormatting(), + const KeyFormatter& keyFormatter = DefaultKeyFormatter) const { + saveGraph(filename, values, keyFormatter, graphvizFormatting); } #endif diff --git a/gtsam/nonlinear/nonlinear.i b/gtsam/nonlinear/nonlinear.i index 960b3ff273..84c4939f49 100644 --- a/gtsam/nonlinear/nonlinear.i +++ b/gtsam/nonlinear/nonlinear.i @@ -193,7 +193,12 @@ class NonlinearFactorGraph { // enabling serialization functionality void serialize() const; - void saveGraph(const string& s) const; + string dot( + const gtsam::Values& values, + const gtsam::KeyFormatter& keyFormatter = gtsam::DefaultKeyFormatter); + void saveGraph(const string& s, const gtsam::Values& values, + const gtsam::KeyFormatter& keyFormatter = + gtsam::DefaultKeyFormatter) const; }; #include @@ -782,6 +787,12 @@ class ISAM2 { void printStats() const; gtsam::VectorValues gradientAtZero() const; + + string dot(const gtsam::KeyFormatter& keyFormatter = + gtsam::DefaultKeyFormatter) const; + void saveGraph(string s, + const gtsam::KeyFormatter& keyFormatter = + gtsam::DefaultKeyFormatter) const; }; #include From 6225700bb7461e8506bf020c36cbfd0e13228567 Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Sat, 25 Dec 2021 14:12:43 -0500 Subject: [PATCH 2/3] Fix missing argument --- gtsam/discrete/DiscreteConditional.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gtsam/discrete/DiscreteConditional.cpp b/gtsam/discrete/DiscreteConditional.cpp index 49a6154523..080dbba9bc 100644 --- a/gtsam/discrete/DiscreteConditional.cpp +++ b/gtsam/discrete/DiscreteConditional.cpp @@ -238,7 +238,7 @@ std::string DiscreteConditional::markdown( if (nrParents() == 0) { // We have no parents, call factor method. ss << ")$:" << std::endl; - ss << DecisionTreeFactor::markdown(); + ss << DecisionTreeFactor::markdown(keyFormatter); return ss.str(); } From 44d66ddbe1e19e092d87a9ed22dc89e726bed0a3 Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Sat, 25 Dec 2021 16:46:31 -0500 Subject: [PATCH 3/3] Fix test --- gtsam/discrete/tests/testDiscreteBayesNet.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gtsam/discrete/tests/testDiscreteBayesNet.cpp b/gtsam/discrete/tests/testDiscreteBayesNet.cpp index 33c7b7011f..ea58165669 100644 --- a/gtsam/discrete/tests/testDiscreteBayesNet.cpp +++ b/gtsam/discrete/tests/testDiscreteBayesNet.cpp @@ -181,7 +181,7 @@ TEST(DiscreteBayesNet, markdown) { "`DiscreteBayesNet` of size 2\n" "\n" " $P(Asia)$:\n" - "|0|value|\n" + "|Asia|value|\n" "|:-:|:-:|\n" "|0|0.99|\n" "|1|0.01|\n"