-
Notifications
You must be signed in to change notification settings - Fork 795
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Hybrid Factor Graph Implementation #1203
Merged
varunagrawal
merged 76 commits into
borglab:develop
from
varunagrawal:fan/prototype-hybrid-tr
Jun 3, 2022
Merged
Changes from all commits
Commits
Show all changes
76 commits
Select commit
Hold shift + click to select a range
ea4ebb6
Fix typo
ProfFan 0567303
Prototype a type-erased hybrid machinery
ProfFan efa37ff
Add better mocking for actual elimination
ProfFan 3aac52c
Fix compilation error
ProfFan 53551e0
Add shorthand for inserting raw JacobianFactor
ProfFan 095f6ad
Add full elimination
ProfFan 2bae286
More mock-ups added
ProfFan ee4f9d1
Added mixture factor functionality
ProfFan 5ea614a
Added more mockups and color output of the elimination process
ProfFan 0aeb596
add more comments
ProfFan fe5dde7
even better printing and comments
ProfFan f237438
Address comments
ProfFan 36ee4ce
Missing pragma onces
ProfFan 7ad2031
Now we have real multifrontal!
ProfFan d42fc21
Fully working Multifrontal
ProfFan 1e8aae3
Add a test for EliminateSequential
ProfFan b4f8eea
Address comments
ProfFan 4f8dfeb
Remove warning
ProfFan 293ef61
Address comments
ProfFan a36c86a
Display debug messages only when DEBUG = true
ProfFan 8b4283b
Add doxygen for GMM
ProfFan 5f03e0f
Address compilation error on GCC
ProfFan 8d88860
Fix GCC error
ProfFan d2dc620
Add Python bindings
ProfFan 7f2fa61
Added more Python examples
ProfFan 0f69b4c
Added plotting for nested dissection
ProfFan a9c1d43
Working iSAM for Hybrid
ProfFan 1fe7e74
Merge remote-tracking branch 'origin/develop' into fan/prototype-hybr…
ProfFan 2de3169
Fix compile
ProfFan 1bf9952
Merge remote-tracking branch 'origin/develop' into fan/prototype-hybr…
ProfFan 4e481ea
Fix the enable_if_t absence
ProfFan d012bcd
Remove most debug messages
ProfFan d5fd279
Merge remote-tracking branch 'origin/develop' into fan/prototype-hybr…
ProfFan 2c4990b
Address Varun's comments
ProfFan 04593cc
Fix compile error
ProfFan 2ae2cb6
Don't crash anymore
ProfFan b8299d7
Don't use Python dict method since it is not
ProfFan 74af969
Trying to make MSVC happy
ProfFan b215d3a
Address PR comments
ProfFan e36583e
include missing headers for msvc and fix warning
varunagrawal e325cd1
include GaussianJunctionTree
varunagrawal eb074e7
run formatting and rename wrappedFactors to asGaussianFactorGraphTree
varunagrawal 3f239c2
fix equality checks
varunagrawal c3a92a4
Hybrid and Gaussian Mixture conditional docs and some refactor
varunagrawal b3cab1b
GaussianMixtureFactor docs
varunagrawal 865d10d
Hybrid factor docs and minor refactor
varunagrawal 573448f
make functional
varunagrawal 6d26818
HybridDiscreteFactor docs and minor refactor
varunagrawal 7bfa011
update tests
varunagrawal d60c4ac
add demarkers
varunagrawal adcbb65
HybridFactorGraph docs and minor updates
varunagrawal 5d3ffb7
docs update
varunagrawal e2c7753
remove debug statements
varunagrawal 30c8e1d
fix doxygen section
varunagrawal 4ee4b37
HybridISAM docs
varunagrawal 9d26a3d
remove debug statements and add docs
varunagrawal 3bde044
add doc strings to python unit test and add assertions
varunagrawal f443cf3
add docs for HybridFactor
varunagrawal 6cd20fb
break up EliminateHybrid into smaller functions
varunagrawal c3a59cf
update the EliminateHybrid note to be more specific
varunagrawal 852a9b9
rename HybridFactorGraph to GaussianHybridFactorGraph
varunagrawal 841e6b0
improved equality checks
varunagrawal 1a8fa23
Merge pull request #46 from varunagrawal/feature/eliminate-hybrid
varunagrawal 7c7b5dd
Rename files so that everything is HybridX
varunagrawal 31ab1a3
update description of GaussianMixtureConditional
varunagrawal d2029f3
Add more information on conditionals requirement for GaussianMixture
varunagrawal c8bf9d3
rename GaussianMixtureConditional to GaussianMixture
varunagrawal 2cc0611
Merge branch 'fan/prototype-hybrid-tr' into feature/GaussianHybridFac…
varunagrawal 709bbb0
Merge pull request #47 from varunagrawal/feature/GaussianHybridFactor…
varunagrawal 932e65c
Add GTSAM_EXPORT and Testable traits
varunagrawal 2afa678
fix python test
varunagrawal 3e10514
if checks for dynamic_cast
varunagrawal dd87747
separate out summing of frontals into separate function
varunagrawal 92176db
add comments
varunagrawal b47cd9d
update GaussianMixture docstring
varunagrawal eeecb27
rename back to HybridJunctionTree
varunagrawal File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ set (gtsam_subdirs | |
inference | ||
symbolic | ||
discrete | ||
hybrid | ||
linear | ||
nonlinear | ||
sam | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
# Install headers | ||
set(subdir hybrid) | ||
file(GLOB hybrid_headers "*.h") | ||
# FIXME: exclude headers | ||
install(FILES ${hybrid_headers} DESTINATION include/gtsam/hybrid) | ||
|
||
# Add all tests | ||
add_subdirectory(tests) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,110 @@ | ||
/* ---------------------------------------------------------------------------- | ||
|
||
* 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 GaussianMixture.cpp | ||
* @brief A hybrid conditional in the Conditional Linear Gaussian scheme | ||
* @author Fan Jiang | ||
* @author Varun Agrawal | ||
* @author Frank Dellaert | ||
* @date Mar 12, 2022 | ||
*/ | ||
|
||
#include <gtsam/base/utilities.h> | ||
#include <gtsam/discrete/DecisionTree-inl.h> | ||
#include <gtsam/hybrid/GaussianMixture.h> | ||
#include <gtsam/inference/Conditional-inst.h> | ||
#include <gtsam/linear/GaussianFactorGraph.h> | ||
|
||
namespace gtsam { | ||
|
||
GaussianMixture::GaussianMixture( | ||
const KeyVector &continuousFrontals, const KeyVector &continuousParents, | ||
const DiscreteKeys &discreteParents, | ||
const GaussianMixture::Conditionals &conditionals) | ||
: BaseFactor(CollectKeys(continuousFrontals, continuousParents), | ||
discreteParents), | ||
BaseConditional(continuousFrontals.size()), | ||
conditionals_(conditionals) {} | ||
|
||
/* *******************************************************************************/ | ||
const GaussianMixture::Conditionals & | ||
GaussianMixture::conditionals() { | ||
return conditionals_; | ||
} | ||
|
||
/* *******************************************************************************/ | ||
GaussianMixture GaussianMixture::FromConditionals( | ||
const KeyVector &continuousFrontals, const KeyVector &continuousParents, | ||
const DiscreteKeys &discreteParents, | ||
const std::vector<GaussianConditional::shared_ptr> &conditionalsList) { | ||
Conditionals dt(discreteParents, conditionalsList); | ||
|
||
return GaussianMixture(continuousFrontals, continuousParents, | ||
discreteParents, dt); | ||
} | ||
|
||
/* *******************************************************************************/ | ||
GaussianMixture::Sum GaussianMixture::add( | ||
const GaussianMixture::Sum &sum) const { | ||
using Y = GaussianFactorGraph; | ||
auto add = [](const Y &graph1, const Y &graph2) { | ||
auto result = graph1; | ||
result.push_back(graph2); | ||
return result; | ||
}; | ||
const Sum tree = asGaussianFactorGraphTree(); | ||
return sum.empty() ? tree : sum.apply(tree, add); | ||
} | ||
|
||
/* *******************************************************************************/ | ||
GaussianMixture::Sum | ||
GaussianMixture::asGaussianFactorGraphTree() const { | ||
auto lambda = [](const GaussianFactor::shared_ptr &factor) { | ||
GaussianFactorGraph result; | ||
result.push_back(factor); | ||
return result; | ||
}; | ||
return {conditionals_, lambda}; | ||
} | ||
|
||
/* *******************************************************************************/ | ||
bool GaussianMixture::equals(const HybridFactor &lf, | ||
double tol) const { | ||
const This *e = dynamic_cast<const This *>(&lf); | ||
return e != nullptr && BaseFactor::equals(*e, tol); | ||
} | ||
|
||
/* *******************************************************************************/ | ||
void GaussianMixture::print(const std::string &s, | ||
const KeyFormatter &formatter) const { | ||
std::cout << s; | ||
if (isContinuous()) std::cout << "Continuous "; | ||
if (isDiscrete()) std::cout << "Discrete "; | ||
if (isHybrid()) std::cout << "Hybrid "; | ||
BaseConditional::print("", formatter); | ||
std::cout << "\nDiscrete Keys = "; | ||
for (auto &dk : discreteKeys()) { | ||
std::cout << "(" << formatter(dk.first) << ", " << dk.second << "), "; | ||
} | ||
std::cout << "\n"; | ||
conditionals_.print( | ||
"", [&](Key k) { return formatter(k); }, | ||
[&](const GaussianConditional::shared_ptr &gf) -> std::string { | ||
RedirectCout rd; | ||
if (!gf->empty()) | ||
gf->print("", formatter); | ||
else | ||
return {"nullptr"}; | ||
return rd.str(); | ||
}); | ||
} | ||
} // namespace gtsam |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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 GaussianMixture.h | ||
* @brief A hybrid conditional in the Conditional Linear Gaussian scheme | ||
* @author Fan Jiang | ||
* @author Varun Agrawal | ||
* @date Mar 12, 2022 | ||
*/ | ||
|
||
#pragma once | ||
|
||
#include <gtsam/discrete/DecisionTree.h> | ||
#include <gtsam/hybrid/HybridFactor.h> | ||
#include <gtsam/inference/Conditional.h> | ||
#include <gtsam/linear/GaussianConditional.h> | ||
|
||
namespace gtsam { | ||
|
||
/** | ||
* @brief A conditional of gaussian mixtures indexed by discrete variables, as | ||
* part of a Bayes Network. | ||
* | ||
* Represents the conditional density P(X | M, Z) where X is a continuous random | ||
* variable, M is the selection of discrete variables corresponding to a subset | ||
* of the Gaussian variables and Z is parent of this node | ||
* | ||
* The probability P(x|y,z,...) is proportional to | ||
* \f$ \sum_i k_i \exp - \frac{1}{2} |R_i x - (d_i - S_i y - T_i z - ...)|^2 \f$ | ||
* where i indexes the components and k_i is a component-wise normalization | ||
* constant. | ||
* | ||
*/ | ||
class GTSAM_EXPORT GaussianMixture | ||
: public HybridFactor, | ||
public Conditional<HybridFactor, GaussianMixture> { | ||
public: | ||
using This = GaussianMixture; | ||
using shared_ptr = boost::shared_ptr<GaussianMixture>; | ||
using BaseFactor = HybridFactor; | ||
using BaseConditional = Conditional<HybridFactor, GaussianMixture>; | ||
|
||
/// Alias for DecisionTree of GaussianFactorGraphs | ||
using Sum = DecisionTree<Key, GaussianFactorGraph>; | ||
|
||
/// typedef for Decision Tree of Gaussian Conditionals | ||
using Conditionals = DecisionTree<Key, GaussianConditional::shared_ptr>; | ||
|
||
private: | ||
Conditionals conditionals_; | ||
|
||
/** | ||
* @brief Convert a DecisionTree of factors into a DT of Gaussian FGs. | ||
*/ | ||
Sum asGaussianFactorGraphTree() const; | ||
|
||
public: | ||
/// @name Constructors | ||
/// @{ | ||
|
||
/// Defaut constructor, mainly for serialization. | ||
GaussianMixture() = default; | ||
|
||
/** | ||
* @brief Construct a new GaussianMixture object. | ||
* | ||
* @param continuousFrontals the continuous frontals. | ||
* @param continuousParents the continuous parents. | ||
* @param discreteParents the discrete parents. Will be placed last. | ||
* @param conditionals a decision tree of GaussianConditionals. The number of | ||
* conditionals should be C^(number of discrete parents), where C is the | ||
* cardinality of the DiscreteKeys in discreteParents, since the | ||
* discreteParents will be used as the labels in the decision tree. | ||
*/ | ||
GaussianMixture(const KeyVector &continuousFrontals, | ||
const KeyVector &continuousParents, | ||
const DiscreteKeys &discreteParents, | ||
const Conditionals &conditionals); | ||
|
||
/** | ||
* @brief Make a Gaussian Mixture from a list of Gaussian conditionals | ||
* | ||
* @param continuousFrontals The continuous frontal variables | ||
* @param continuousParents The continuous parent variables | ||
* @param discreteParents Discrete parents variables | ||
* @param conditionals List of conditionals | ||
*/ | ||
static This FromConditionals( | ||
const KeyVector &continuousFrontals, const KeyVector &continuousParents, | ||
const DiscreteKeys &discreteParents, | ||
const std::vector<GaussianConditional::shared_ptr> &conditionals); | ||
|
||
/// @} | ||
/// @name Testable | ||
/// @{ | ||
|
||
/// Test equality with base HybridFactor | ||
bool equals(const HybridFactor &lf, double tol = 1e-9) const override; | ||
|
||
/* print utility */ | ||
void print( | ||
const std::string &s = "GaussianMixture\n", | ||
const KeyFormatter &formatter = DefaultKeyFormatter) const override; | ||
|
||
/// @} | ||
|
||
/// Getter for the underlying Conditionals DecisionTree | ||
const Conditionals &conditionals(); | ||
|
||
/** | ||
* @brief Merge the Gaussian Factor Graphs in `this` and `sum` while | ||
* maintaining the decision tree structure. | ||
* | ||
* @param sum Decision Tree of Gaussian Factor Graphs | ||
* @return Sum | ||
*/ | ||
Sum add(const Sum &sum) const; | ||
}; | ||
|
||
// traits | ||
template <> | ||
struct traits<GaussianMixture> : public Testable<GaussianMixture> {}; | ||
|
||
} // namespace gtsam |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
/* ---------------------------------------------------------------------------- | ||
|
||
* 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 GaussianMixtureFactor.cpp | ||
* @brief A set of Gaussian factors indexed by a set of discrete keys. | ||
* @author Fan Jiang | ||
* @author Varun Agrawal | ||
* @author Frank Dellaert | ||
* @date Mar 12, 2022 | ||
*/ | ||
|
||
#include <gtsam/base/utilities.h> | ||
#include <gtsam/discrete/DecisionTree-inl.h> | ||
#include <gtsam/discrete/DecisionTree.h> | ||
#include <gtsam/hybrid/GaussianMixtureFactor.h> | ||
#include <gtsam/linear/GaussianFactorGraph.h> | ||
|
||
namespace gtsam { | ||
|
||
/* *******************************************************************************/ | ||
GaussianMixtureFactor::GaussianMixtureFactor(const KeyVector &continuousKeys, | ||
const DiscreteKeys &discreteKeys, | ||
const Factors &factors) | ||
: Base(continuousKeys, discreteKeys), factors_(factors) {} | ||
|
||
/* *******************************************************************************/ | ||
bool GaussianMixtureFactor::equals(const HybridFactor &lf, double tol) const { | ||
const This *e = dynamic_cast<const This *>(&lf); | ||
return e != nullptr && Base::equals(*e, tol); | ||
} | ||
|
||
/* *******************************************************************************/ | ||
GaussianMixtureFactor GaussianMixtureFactor::FromFactors( | ||
const KeyVector &continuousKeys, const DiscreteKeys &discreteKeys, | ||
const std::vector<GaussianFactor::shared_ptr> &factors) { | ||
Factors dt(discreteKeys, factors); | ||
|
||
return GaussianMixtureFactor(continuousKeys, discreteKeys, dt); | ||
} | ||
|
||
/* *******************************************************************************/ | ||
void GaussianMixtureFactor::print(const std::string &s, | ||
const KeyFormatter &formatter) const { | ||
HybridFactor::print(s, formatter); | ||
factors_.print( | ||
"mixture = ", [&](Key k) { return formatter(k); }, | ||
[&](const GaussianFactor::shared_ptr &gf) -> std::string { | ||
RedirectCout rd; | ||
if (!gf->empty()) | ||
gf->print("", formatter); | ||
else | ||
return {"nullptr"}; | ||
return rd.str(); | ||
}); | ||
} | ||
|
||
/* *******************************************************************************/ | ||
const GaussianMixtureFactor::Factors &GaussianMixtureFactor::factors() { | ||
return factors_; | ||
} | ||
|
||
/* *******************************************************************************/ | ||
varunagrawal marked this conversation as resolved.
Show resolved
Hide resolved
|
||
GaussianMixtureFactor::Sum GaussianMixtureFactor::add( | ||
const GaussianMixtureFactor::Sum &sum) const { | ||
using Y = GaussianFactorGraph; | ||
auto add = [](const Y &graph1, const Y &graph2) { | ||
auto result = graph1; | ||
result.push_back(graph2); | ||
return result; | ||
}; | ||
const Sum tree = asGaussianFactorGraphTree(); | ||
return sum.empty() ? tree : sum.apply(tree, add); | ||
} | ||
|
||
/* *******************************************************************************/ | ||
GaussianMixtureFactor::Sum GaussianMixtureFactor::asGaussianFactorGraphTree() | ||
const { | ||
auto wrap = [](const GaussianFactor::shared_ptr &factor) { | ||
GaussianFactorGraph result; | ||
result.push_back(factor); | ||
return result; | ||
}; | ||
return {factors_, wrap}; | ||
} | ||
} // namespace gtsam |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ProfFan @dellaert why is this inheriting from
HybridFactor
and notHybridGaussianMixtureFactor
like in the previous scheme? Do we not need to record the conditionals as factors in the base class anymore?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I lost track of this - @ProfFan might have the answer
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this can be done that way, just what was on my mind is that I want to keep the two things separate, as they don't necessarily need to have inheritance relationship. We can revisit this later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry but this is an early design decision that needs to be resolved ASAP.
My understanding was that eliminating a HybridMixtureFactor should result in a GaussianMixture conditional, so there already exists an intrinsic relationship between the two.
My main concern is what happens when we only eliminate a partial set of the keys? If the factor is F(X1, X2, M1) and we only eliminate X1, shouldn't the gaussian factors for X1 still be captured in the conditional until even X2 is eliminated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually agree with Varun: a conditional is a factor. No sense to have extra storage in this conditional for the Gaussian components, if that is going on.
However, I don't fully understand how the elimination example is relevant. We don't eliminate conditionals, we eliminate factors, so not sure how it bears on this discussion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GaussianMixtureFactor is a container for other factors via the decision tree with Gaussian FGs as the leaves. So if we eliminate one variable, I don't remember how that affects the other variables within the tree. What is the expected behavior?
My understanding is that it should be a conditional on the variable with the remaining variables as parents, so does this mean the conditional would be dependent on variables still a part of factors? In that case we should be recording the factors. However the final Bayes network will only have conditionals, so the GaussianMixtures will have to be retroactively updated to reflect the new conditionals for parents.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll go back and look at the behavior from the previous scheme.