Skip to content
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

Sampling test for Hybrid Posterior #1346

Merged
merged 7 commits into from
Dec 25, 2022
Merged

Conversation

varunagrawal
Copy link
Collaborator

This test checks for correctness of the hybrid posterior using a sampling based approach.

Specifically, given a simple factor graph as below

 M0\
X0 -*- X1 

we compute the posterior P(x0, m0, x1| z0, z1) given measurements z0, z1.

The test, at a high level performs the following steps:

  1. Start with the factor graph and linearize FG .
  2. Eliminate into a bayes net BN.
  3. Sample from the bayes net.
  4. Check BN(x)/FG(x) equals the same value for all samples.

Since we don't have a sample method for the hybrid bayes net, we perform a sampling scheme similar to a mixture model where,

  1. We sample from a discrete distribution weighted by the mixing weights $\pi$.
  2. Given the discrete sample value, pick the corresponding bayes net and factor graph.
  3. Sample as described above.

Sample Method Update

Since the linearized factor graph and the eliminated bayes net do not record the noise model, sampling from the Bayes Net throws an error. To overcome this, I've added an optional parameter to the sample method SharedDiagonal model, and if the conditional to sample from doesn't have a noise model associated with it, then it will sample from the provided model. This model should be the same one that is provided to the original factor in the Nonlinear/Gaussian Factor Graph, making it a convenient addition.

@varunagrawal varunagrawal self-assigned this Dec 23, 2022
@varunagrawal varunagrawal changed the base branch from develop to hybrid/tests December 23, 2022 14:59
@dellaert
Copy link
Member

Can I ask to create a separate PR (to develop) with sampling for hybrid BNs? Seeing that in isolation will be easier to review.

@dellaert
Copy link
Member

Also, convert to draft PR? Don’t think we should run CI as long as we don’t target develop

@varunagrawal varunagrawal marked this pull request as draft December 23, 2022 16:29
@varunagrawal
Copy link
Collaborator Author

Can I ask to create a separate PR (to develop) with sampling for hybrid BNs? Seeing that in isolation will be easier to review.

So a HybridBayesNet::sample method? Can do!

Copy link
Member

@dellaert dellaert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many comments…

gtsam/hybrid/tests/testHybridEstimation.cpp Outdated Show resolved Hide resolved
gtsam/hybrid/tests/testHybridEstimation.cpp Outdated Show resolved Hide resolved
gtsam/hybrid/tests/testHybridEstimation.cpp Outdated Show resolved Hide resolved
gtsam/hybrid/tests/testHybridEstimation.cpp Outdated Show resolved Hide resolved
gtsam/hybrid/tests/testHybridEstimation.cpp Outdated Show resolved Hide resolved
gtsam/hybrid/tests/testHybridEstimation.cpp Outdated Show resolved Hide resolved
gtsam/hybrid/tests/testHybridEstimation.cpp Outdated Show resolved Hide resolved
gtsam/hybrid/tests/testHybridEstimation.cpp Outdated Show resolved Hide resolved
gtsam/hybrid/tests/testHybridEstimation.cpp Outdated Show resolved Hide resolved
gtsam/hybrid/tests/testHybridEstimation.cpp Outdated Show resolved Hide resolved
@varunagrawal varunagrawal marked this pull request as ready for review December 25, 2022 04:44
Copy link
Member

@dellaert dellaert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let’s merge

@varunagrawal varunagrawal merged commit f3c85ae into hybrid/tests Dec 25, 2022
@varunagrawal varunagrawal deleted the hybrid/verification branch December 25, 2022 14:50
@varunagrawal varunagrawal restored the hybrid/verification branch December 25, 2022 17:27
@varunagrawal varunagrawal deleted the hybrid/verification branch December 25, 2022 17:29
@dellaert dellaert added this to the Hybrid Inference milestone Feb 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants