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

SMC minor improvements #3625

Merged
merged 4 commits into from
Oct 3, 2019
Merged

Conversation

aloctavodia
Copy link
Member

  • This refactors the code using an object oriented approach, I have been trying different ideas and this approach allows easier iteration, and I hope it will make easier future improvements such as adding new kernels or diagnostic tests.
  • Improve docstrings, specially for SMC-ABC
  • Now each chain has it's own scaling, the initial scaling is computed based on the dimension of the model and the update rule is simpler than before. This seems to improve some models but honestly I was hoping for larger improvements. I will keep trying ideas around this.
  • add test for SMC-ABC

@junpenglao junpenglao self-requested a review September 25, 2019 19:37
smc.setup_kernel()
smc.initialize_logp()

while smc.beta < 1:
Copy link
Member

Choose a reason for hiding this comment

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

Risk of it running forever? Maybe we should have an additional termination rule like after N iterations.

Copy link
Member Author

Choose a reason for hiding this comment

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

Early stopping will return a non useful result as the samples will not be from the posterior. Thus I think is OK to let the user kill the process if it is running more than what the user can wait. One alternative will be to jump to beta=1 after N iterations, but most likely than not that will also give a poor sample


if self.kernel.lower() == "abc":
warnings.warn(EXPERIMENTAL_WARNING)
simulator = self.model.observed_RVs[0]
Copy link
Member

Choose a reason for hiding this comment

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

Could you add an assertion here to make sure the length of observed_RVs is equal 1, otherwise throw an error?

@aloctavodia aloctavodia merged commit c760a85 into pymc-devs:master Oct 3, 2019
@aloctavodia aloctavodia deleted the smc_refactor branch October 3, 2019 20:50
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