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

Implement GaussianMRF distribution #1981

Merged
merged 19 commits into from Jul 29, 2019

Conversation

@fritzo
Copy link
Collaborator

commented Jul 26, 2019

Addresses #1970

This implements a GaussianMRF that uses time-parallel filtering to compute a differentiable .log_prob().

Note this Markov Random Field model involves an underlying factor graph, whereas the more standard Hidden Markov Model involves conditional normal distributions. This MRF implementation is simpler to implement than an HMM and is closer in code to the DiscreteHMM. In practice provides similar modeling capabilities, but requires slightly different interpretation of factors. We may add an additional GaussianHMM distribution in the future.

The main parts to review are: new methods of the Gaussian class, the _sequential_gaussian_tensordot() function, and the GaussianMRF class.

Tested

  • unit tests for new Gaussian methods
  • math test for the recursive _sequential_gaussian_tensordot()
  • shape tests for GaussianMRF
  • density tests for GaussianMRF

@fritzo fritzo added the WIP label Jul 26, 2019

@fritzo fritzo requested a review from fehiepsi Jul 26, 2019

pyro/ops/gaussian.py Outdated Show resolved Hide resolved

fritzo added some commits Jul 26, 2019

pyro/ops/gaussian.py Outdated Show resolved Hide resolved
pyro/ops/gaussian.py Outdated Show resolved Hide resolved
pyro/ops/gaussian.py Outdated Show resolved Hide resolved
tests/ops/test_gaussian.py Outdated Show resolved Hide resolved
pyro/distributions/hmm.py Outdated Show resolved Hide resolved

@fritzo fritzo requested a review from fehiepsi Jul 27, 2019

@fritzo fritzo added awaiting review WIP and removed WIP labels Jul 27, 2019

@fritzo fritzo removed the WIP label Jul 27, 2019

@fritzo

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 27, 2019

@fehiepsi I'm pretty happy with this PR now. The last test_gaussian_mrf_log_prob did catch one transpose error. Let me know if you can think of any stronger tests.

In follow-up PRs I'd like to add an example, test jit support, test cuda support, etc.

@fehiepsi
Copy link
Collaborator

left a comment

The implementation looks beautiful to me! I just catch a few nits during reviewing. Regarding to MRF log_prob, I think we can also test for identity transition_dist and observation_dist but it is not important IMO.

pyro/ops/gaussian.py Outdated Show resolved Hide resolved
pyro/ops/gaussian.py Outdated Show resolved Hide resolved
pyro/ops/gaussian.py Show resolved Hide resolved
pyro/ops/gaussian.py Outdated Show resolved Hide resolved

@fritzo fritzo changed the title Implement GaussianMRF distribution Implement GaussianGaussianMRF distribution Jul 27, 2019

@fritzo fritzo added WIP and removed awaiting review labels Jul 28, 2019

pyro/ops/gaussian.py Outdated Show resolved Hide resolved
pyro/ops/gaussian.py Outdated Show resolved Hide resolved

fritzo added some commits Jul 28, 2019

@fritzo fritzo added awaiting review and removed WIP labels Jul 28, 2019

@fritzo fritzo changed the title Implement GaussianGaussianMRF distribution Implement GaussianMRF distribution Jul 28, 2019

@fehiepsi
Copy link
Collaborator

left a comment

LGTM when tests pass. I have learned a lot from your implementation.

@fritzo

This comment has been minimized.

Copy link
Collaborator Author

commented Jul 28, 2019

Thanks for the detailed review and all of your math help, @fehiepsi !

@fehiepsi fehiepsi merged commit 3619772 into dev Jul 29, 2019

2 checks passed

Travis CI - Pull Request Build Passed
Details
license/cla Contributor License Agreement is signed.
Details

c0nn3r added a commit to c0nn3r/pyro that referenced this pull request Aug 19, 2019

Implement GaussianMRF distribution (pyro-ppl#1981)
* Sketch GaussianMRF distribution

* Add unit tests for Gaussian shape ops

* Add more tests

* Fix assertion

* Add .pad() and .__add__() ops; fix GaussianMRF

* Fix logsumexp tests

* Attempt to fix .log_density()

* Incorporate math fixes by Du Phan :)

* Implement Gaussian.condition()

* Fix GaussianMRF shape errors

* Strengthen test

* Add a test for GaussianMRF.log_prob()

* Fix test; clarify docstring

* Address review comments

* WIP attempt to normalize GaussianGaussianMRF

* Fix most of marginalize (info_vec is still broken)

* Attempt to fix info_vec computation

* Fix marginalize() log_normalizer term

* Rename back to GaussianMRF
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.