-
-
Notifications
You must be signed in to change notification settings - Fork 984
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
Remove dependency on TracePosterior and deprecate utilities in AbstractInfer #1930
Comments
@neerajprad is any part of this issue blocking the 0.4 release? |
Deprecating In the meantime, I will try to see if we can support SVI by making some minor changes to mcmc's predictive utility. As this utility becomes mature, that will make it easier for us to deprecate the old interface. |
@fehiepsi - Since you volunteered helping with the predictive utility, I have added you to this task. Note that we already have a |
Sure, I am happy to work on this. I'll try to make a PR which covers usage cases which I know, then we can discuss there how to support more general cases, which I believe that you have some experience with. |
I think we need to allow for a guide argument and figure out predictions when data subsamples are differently sized between training and testing. @ahmadsalim has already handled this in |
@neerajprad @ahmadsalim I have thought about the I have been using
|
I think we should be able to just run the guide forward on the new data without worrying about differences in plate sizes. IIRC, we only had to do subsampling when we were storing posterior data in @fehiepsi - I think your simplified solution is better. Let us just go ahead with that instead. Could you add @ahmadsalim to the PR, who had a few models that were using TracePredictive and it will be nice if we can ensure that these continue to work well with the refactoring? |
@fehiepsi Please, see https://github.com/pyro-ppl/pyro/blob/dev/tests/infer/test_abstract_infer.py#L102-L103 and https://github.com/pyro-ppl/pyro/blob/dev/tests/infer/test_abstract_infer.py#L47-L48 . You can see that the batch size changes, because we use |
Yeah, I just noticed it. I'll make a PR soon then we can discuss those situations there. :) |
Removing the 1.0 milestone since all tasks are done. We should prioritize removing deprecated code in a future release. |
While attempting the MCMC class refactoring (as originally suggested in #1725), I realized that it will be best to not subclass off of
TracePosterior
.TODOs:
Next Release:
Next Release:
.run
method in SVI module to plan for removal(Note: raise
UserWarning
rather thanDeprecationWarning
)Following Release:
The text was updated successfully, but these errors were encountered: