Skip to content

Disallow instance override of .reparameterized property#640

Merged
neerajprad merged 2 commits intodevfrom
static-reparameterized
Dec 20, 2017
Merged

Disallow instance override of .reparameterized property#640
neerajprad merged 2 commits intodevfrom
static-reparameterized

Conversation

@fritzo
Copy link
Copy Markdown
Member

@fritzo fritzo commented Dec 20, 2017

Fixes #631

This removes plumbing that allows Distribution instances to override the class-level .reparameterized property. Removing this plumbing simplifies our migration to torch.distributions. This plumbing was only used in tests; to provide the non-reparameterized Normal distribution provided by that override, this PR introduces a fakes.NonreparameterizedNormal distribution that is exposed only in tests/.

Comment thread tests/fakes.py
reparameterized = False


nonreparameterized_normal = RandomPrimitive(NonreparameterizedNormal)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why do you need this? cant you set the class attribute directly in the tests in your conditional?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Mock.patching would not work in this case. Some tests.require both reparam and nonreparam versions of normal, and the attribute is examined out-of-order deep inside TraceGraph_ELBO after tracing the model and guide.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Once all our distributions are migrated, are we planning to use this as an alias for has_rsample?

Copy link
Copy Markdown
Member

@neerajprad neerajprad left a comment

Choose a reason for hiding this comment

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

LGTM.

@neerajprad neerajprad merged commit b473516 into dev Dec 20, 2017
@fritzo fritzo deleted the static-reparameterized branch December 21, 2017 01:55
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.

3 participants