Skip to content

Fix DMM example with --num-iafs > 0#734

Merged
fritzo merged 2 commits intopyro-ppl:devfrom
neerajprad:fix-dmm
Feb 1, 2018
Merged

Fix DMM example with --num-iafs > 0#734
fritzo merged 2 commits intopyro-ppl:devfrom
neerajprad:fix-dmm

Conversation

@neerajprad
Copy link
Copy Markdown
Member

We needed some shape fixes to TransformedDistribution to fix the DMM example with IAF. There is still the issue with NAN loss - #711 that seems to have been introduced much earlier (I checked until November 2017).

Fixes #721.

fritzo
fritzo previously approved these changes Jan 31, 2018
Copy link
Copy Markdown
Member

@fritzo fritzo left a comment

Choose a reason for hiding this comment

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

I agree the automated example discovery logic is getting kludgey.

@neerajprad
Copy link
Copy Markdown
Member Author

neerajprad commented Jan 31, 2018

@fritzo - Can I just remove the pyro TransformedDistribution and Bijector classes and use the corresponding PyTorch implementations? I will modify the InverseAutoregressiveFlow class to inherit off of torch Bijector. I will probably need that for HMC too.

EDIT - I'll let it be for now, and have you and @martinjankowiak take care of that.

@neerajprad
Copy link
Copy Markdown
Member Author

I agree the automated example discovery logic is getting kludgey.

Its not that bad yet. :) Just added a TODO to keep in mind that if we are messing around with that part too much, it might be worth revisiting.

@fritzo
Copy link
Copy Markdown
Member

fritzo commented Jan 31, 2018

Can I just remove the pyro TransformedDistribution

I am working on that in this branch https://github.com/uber/pyro/compare/torch-transformed-and-iaf
Feel free to take over the work if you like. I can open a PR if you want to discuss.

@neerajprad
Copy link
Copy Markdown
Member Author

I am working on that in this branch https://github.com/uber/pyro/compare/torch-transformed-and-iaf Feel free to take over the work if you like. I can open a PR if you want to discuss.

Yeah, I will need this for HMC. Unfortunately, we can't mix RandomPrimitive and other distributions, since RandomPrimitive instances can't support properties (we need to pass in distribution parameters to those methods). As such they are not interchangeable with torch distributions.

@fritzo fritzo merged commit 8db8972 into pyro-ppl:dev Feb 1, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DMM Example Model and guide dims disagree when IAF is applied

2 participants