From 6bd2dcff7eb8ad8155d7084267eba50bc86df65e Mon Sep 17 00:00:00 2001 From: Frank Dellaert Date: Sun, 5 Feb 2023 18:31:12 -0800 Subject: [PATCH] Better comments --- gtsam/inference/EliminateableFactorGraph.h | 24 +++++++++---------- .../tests/testSymbolicFactorGraph.cpp | 16 ++++++------- 2 files changed, 20 insertions(+), 20 deletions(-) diff --git a/gtsam/inference/EliminateableFactorGraph.h b/gtsam/inference/EliminateableFactorGraph.h index da4bfbc7b..ac4b96d28 100644 --- a/gtsam/inference/EliminateableFactorGraph.h +++ b/gtsam/inference/EliminateableFactorGraph.h @@ -216,7 +216,7 @@ namespace gtsam { /** Compute the marginal of the requested variables and return the result as a Bayes net. Uses * COLAMD marginalization ordering by default * @param variables Determines the *ordered* variables whose marginal to compute, - * will be ordered in the returned BayesNet. + * will be ordered in the returned BayesNet as specified. * @param function Optional dense elimination function. * @param variableIndex Optional pre-computed VariableIndex for the factor graph, if not * provided one will be computed. @@ -228,8 +228,8 @@ namespace gtsam { /** Compute the marginal of the requested variables and return the result as a Bayes net. Uses * COLAMD marginalization ordering by default - * @param variables Determines the variables whose marginal to compute, - * will be ordered using constrained COLAMD. + * @param variables Determines the variables whose marginal to compute, will be ordered + * using COLAMD; use `Ordering(variables)` to specify the variable ordering. * @param function Optional dense elimination function. * @param variableIndex Optional pre-computed VariableIndex for the factor graph, if not * provided one will be computed. @@ -241,7 +241,7 @@ namespace gtsam { /** Compute the marginal of the requested variables and return the result as a Bayes net. * @param variables Determines the *ordered* variables whose marginal to compute, - * will be ordered in the returned BayesNet. + * will be ordered in the returned BayesNet as specified. * @param marginalizedVariableOrdering Ordering for the variables being marginalized out, * i.e. all variables not in \c variables. * @param function Optional dense elimination function. @@ -255,8 +255,8 @@ namespace gtsam { OptionalVariableIndex variableIndex = {}) const; /** Compute the marginal of the requested variables and return the result as a Bayes net. - * @param variables Determines the variables whose marginal to compute, - * will be ordered using constrained COLAMD. + * @param variables Determines the variables whose marginal to compute, will be ordered + * using COLAMD; use `Ordering(variables)` to specify the variable ordering. * @param marginalizedVariableOrdering Ordering for the variables being marginalized out, * i.e. all variables not in \c variables. * @param function Optional dense elimination function. @@ -272,7 +272,7 @@ namespace gtsam { /** Compute the marginal of the requested variables and return the result as a Bayes tree. Uses * COLAMD marginalization order by default * @param variables Determines the *ordered* variables whose marginal to compute, - * will be ordered in the returned BayesNet. + * will be ordered in the returned BayesNet as specified. * @param function Optional dense elimination function.. * @param variableIndex Optional pre-computed VariableIndex for the factor graph, if not * provided one will be computed. */ @@ -283,8 +283,8 @@ namespace gtsam { /** Compute the marginal of the requested variables and return the result as a Bayes tree. Uses * COLAMD marginalization order by default - * @param variables Determines the variables whose marginal to compute, - * will be ordered using constrained COLAMD. + * @param variables Determines the variables whose marginal to compute, will be ordered + * using COLAMD; use `Ordering(variables)` to specify the variable ordering. * @param function Optional dense elimination function.. * @param variableIndex Optional pre-computed VariableIndex for the factor graph, if not * provided one will be computed. */ @@ -295,7 +295,7 @@ namespace gtsam { /** Compute the marginal of the requested variables and return the result as a Bayes tree. * @param variables Determines the *ordered* variables whose marginal to compute, - * will be ordered in the returned BayesNet. + * will be ordered in the returned BayesNet as specified. * @param marginalizedVariableOrdering Ordering for the variables being marginalized out, * i.e. all variables not in \c variables. * @param function Optional dense elimination function.. @@ -308,8 +308,8 @@ namespace gtsam { OptionalVariableIndex variableIndex = {}) const; /** Compute the marginal of the requested variables and return the result as a Bayes tree. - * @param variables Determines the variables whose marginal to compute, - * will be ordered using constrained COLAMD. + * @param variables Determines the variables whose marginal to compute, will be ordered + * using COLAMD; use `Ordering(variables)` to specify the variable ordering. * @param marginalizedVariableOrdering Ordering for the variables being marginalized out, * i.e. all variables not in \c variables. * @param function Optional dense elimination function.. diff --git a/gtsam/symbolic/tests/testSymbolicFactorGraph.cpp b/gtsam/symbolic/tests/testSymbolicFactorGraph.cpp index f114bd250..2363a0fad 100644 --- a/gtsam/symbolic/tests/testSymbolicFactorGraph.cpp +++ b/gtsam/symbolic/tests/testSymbolicFactorGraph.cpp @@ -125,38 +125,36 @@ TEST(SymbolicFactorGraph, eliminatePartialMultifrontal) { /* ************************************************************************* */ TEST(SymbolicFactorGraph, MarginalMultifrontalBayesNetOrdering) { - auto expectedBayesNet = SymbolicBayesNet({0, 1, 2})({1, 2, 3})({2, 3})({3}); - SymbolicBayesNet actual = *simpleTestGraph2.marginalMultifrontalBayesNet(Ordering{0, 1, 2, 3}); + auto expectedBayesNet = SymbolicBayesNet({0, 1, 2})({1, 2, 3})({2, 3})({3}); EXPECT(assert_equal(expectedBayesNet, actual)); } TEST(SymbolicFactorGraph, MarginalMultifrontalBayesNetKeyVector) { - auto expectedBayesNet = SymbolicBayesNet({0, 1, 2})({2, 1, 3})({1, 3})({3}); - SymbolicBayesNet actual = *simpleTestGraph2.marginalMultifrontalBayesNet(KeyVector{0, 1, 2, 3}); + // Since we use KeyVector, the variable ordering will be determined by COLAMD: + auto expectedBayesNet = SymbolicBayesNet({0, 1, 2})({2, 1, 3})({1, 3})({3}); EXPECT(assert_equal(expectedBayesNet, actual)); } TEST(SymbolicFactorGraph, MarginalMultifrontalBayesNetOrderingPlus) { - auto expectedBayesNet = SymbolicBayesNet(SymbolicConditional{0, 3})({3}); - const Ordering orderedVariables{0, 3}, marginalizedVariableOrdering{1, 2, 4, 5}; SymbolicBayesNet actual = *simpleTestGraph2.marginalMultifrontalBayesNet( orderedVariables, marginalizedVariableOrdering); + auto expectedBayesNet = SymbolicBayesNet(SymbolicConditional{0, 3})({3}); EXPECT(assert_equal(expectedBayesNet, actual)); } TEST(SymbolicFactorGraph, MarginalMultifrontalBayesNetKeyVectorPlus) { - auto expectedBayesNet = SymbolicBayesNet({0, 1, 3})({3, 1})({1}); - const KeyVector variables{0, 1, 3}; const Ordering marginalizedVariableOrdering{2, 4, 5}; SymbolicBayesNet actual = *simpleTestGraph2.marginalMultifrontalBayesNet( variables, marginalizedVariableOrdering); + // Since we use KeyVector, the variable ordering will be determined by COLAMD: + auto expectedBayesNet = SymbolicBayesNet({0, 1, 3})({3, 1})({1}); EXPECT(assert_equal(expectedBayesNet, actual)); } @@ -172,6 +170,7 @@ TEST(SymbolicFactorGraph, MarginalMultifrontalBayesTreeOrdering) { } TEST(SymbolicFactorGraph, MarginalMultifrontalBayesTreeKeyVector) { + // Same: KeyVector variant will use COLAMD: auto expectedBayesTree = *simpleTestGraph2.eliminatePartialMultifrontal(Ordering{4, 5}) .second->eliminateMultifrontal(Ordering::OrderingType::COLAMD); @@ -195,6 +194,7 @@ TEST(SymbolicFactorGraph, MarginalMultifrontalBayesTreeOrderingPlus) { } TEST(SymbolicFactorGraph, MarginalMultifrontalBayesTreeKeyVectorPlus) { + // Again: KeyVector variant will use COLAMD: const Ordering marginalizedVariableOrdering{2, 4, 5}; auto expectedBayesTree = *simpleTestGraph2