AlgebraicDecisionTree Helpers #1696
Conversation
gtsam/hybrid/HybridBayesNet.cpp
Outdated
| @@ -220,15 +220,103 @@ GaussianBayesNet HybridBayesNet::choose( | |||
| /* ************************************************************************* */ | |||
| HybridValues HybridBayesNet::optimize() const { | |||
| // Collect all the discrete factors to compute MPE | |||
| DiscreteBayesNet discrete_bn; | |||
| DiscreteFactorGraph discrete_fg; | |||
There was a problem hiding this comment.
This function is way too large to properly understand. Please break up?
| // the unnormalized probability q(μ;m) = exp(-error(μ;m)) at the mean. | ||
| // discrete_probability = exp(-error(μ;m)) * sqrt(det(2π Σ_m)) | ||
| auto probability = [&](const Result &pair) -> double { | ||
| // Compute the probability q(μ;m) = exp(-error(μ;m)) * sqrt(det(2π Σ_m)) |
There was a problem hiding this comment.
Oh, I guess this will conflict with my simplified PR...
There was a problem hiding this comment.
Yeah we should probably land all of my old PRs so we can reduce conflicts.
gtsam/linear/GaussianConditional.cpp
Outdated
| return - 0.5 * n * log2pi + logDeterminant(); | ||
| // Sigma = (R'R)^{-1}, det(Sigma) = det((R'R)^{-1}) = det(R'R)^{-1} | ||
| // log det(Sigma) = -log(det(R'R)) = -2*log(det(R)) | ||
| // Hence, log det(Sigma)) = - 2.0 * logDeterminant() |
There was a problem hiding this comment.
take this comment all the way :-)
gtsam/hybrid/HybridBayesNet.cpp
Outdated
| DiscreteFactorGraph discrete_fg; | ||
|
|
||
| // Compute model selection term | ||
| AlgebraicDecisionTree<Key> model_selection_term = model_selection(); |
There was a problem hiding this comment.
naming convention on variables. modelSelectionTerms? This is a global comment on this PR.
gtsam/hybrid/HybridBayesNet.h
Outdated
| This can be computed by multiplying all the exponentiated errors | ||
| of each of the conditionals. | ||
| */ | ||
| AlgebraicDecisionTree<Key> model_selection() const; |
There was a problem hiding this comment.
naming convention: modelSelection. Although, it's a bit of a weird name, and the comment does not help much. What is the return value?
There was a problem hiding this comment.
Done! Hopefully the improved docstring is better.
varunagrawal
force-pushed
the
model-selection-integration
branch
from
d552eef to
538871a
Compare
|
@dellaert I want to see what can I do to land this ASAP. My thesis document has the latest updates describing these changes (section 7.5.6). |
varunagrawal
changed the title
| @@ -196,6 +196,42 @@ namespace gtsam { | |||
| return this->apply(g, &Ring::div); | |||
| } | |||
|
|
|||
| /// Compute sum of all values | |||
This PR does two primary things:
min,max,sum, etc) inAlgebraicDecisionTree.(NOTE: I broke down a much larger PR into this smaller one. Will create additional PRs with the rest of the changes)