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

Add MCMC solver to Pastas #572

Merged
merged 19 commits into from
Aug 15, 2023
Merged

Add MCMC solver to Pastas #572

merged 19 commits into from
Aug 15, 2023

Conversation

raoulcollenteur
Copy link
Member

@raoulcollenteur raoulcollenteur commented Mar 21, 2023

Short Description

In this PR a MCMC solver based on Emcee is added to Pastas. Here are the most important changes:

  • Add new solver: EmceeSolve. I have extensively tested this and I think it is working fine. However, since this is new, I will add a warning message if it is used: "EmceeSolve is still experimental and arguments may change in next version of Pastas".
  • Add prior distributions to parameters. All parameter DataFrames throughout Pastas now have a new column "dist", where a distribution for the prior can be provided. This does to affect any of the functionality, but is required for MCMC.
  • Add objective functions that can be chosen (e.g., which likelihood function to use). For now two are implemented for Gaussian (autocorrelated) residuals. I think this is a cool feature, because it allows someone to write a new objective functions / likelihood function!
  • A notebook is added which gives more information on how to use the new solver and how to apply different settings.

This does not add dependencies, but if one wants to use it emcee and corner should be installed.

Checklist before PR can be merged:

I think the current implementation works well, so curious to hear what you all think about it!

Cheers,
Raoul

@raoulcollenteur raoulcollenteur self-assigned this Mar 21, 2023
@raoulcollenteur raoulcollenteur added the development Indicates development of new features label Mar 21, 2023
@raoulcollenteur raoulcollenteur added this to the 1.1 milestone Mar 21, 2023
@raoulcollenteur raoulcollenteur marked this pull request as draft March 21, 2023 16:52
@raoulcollenteur raoulcollenteur linked an issue Jun 22, 2023 that may be closed by this pull request
5 tasks
@raoulcollenteur raoulcollenteur marked this pull request as ready for review June 22, 2023 13:42
@codacy-production
Copy link

codacy-production bot commented Jun 22, 2023

Coverage summary from Codacy

Merging #572 (9945c38) into dev (cb06df7) - See PR on Codacy

Coverage variation Diff coverage
-0.21% (target: +0.00%) 77.93%
Coverage variation details
Coverable lines Covered lines Coverage
Common ancestor commit (cb06df7) 4955 3599 72.63%
Head commit (9945c38) 5113 (+158) 3703 (+104) 72.42% (-0.21%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details
Coverable lines Covered lines Diff coverage
Pull request (#572) 213 166 77.93%

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

pastas/solver.py Outdated Show resolved Hide resolved
Copy link
Member

@dbrakenhoff dbrakenhoff left a comment

Choose a reason for hiding this comment

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

Nice, this looks cool!

Besides the comments I made in the code a couple of comments here:

  • Perhaps it would be interesting to compare the priors and posteriors for one or more parameters in the example notebook?
  • Can we help the user a bit more selecting some default emcee parameters, if that is possible?
  • Add emcee group to pyproject.toml with optional packages: [emcee, corner, tqdm]?

pastas/solver.py Show resolved Hide resolved
pastas/solver.py Outdated Show resolved Hide resolved
pastas/solver.py Outdated Show resolved Hide resolved
doc/examples/uncertainty_emcee.ipynb Outdated Show resolved Hide resolved
doc/examples/uncertainty_emcee.ipynb Outdated Show resolved Hide resolved
doc/examples/uncertainty_emcee.ipynb Outdated Show resolved Hide resolved
doc/examples/uncertainty_emcee.ipynb Outdated Show resolved Hide resolved
doc/examples/uncertainty_emcee.ipynb Show resolved Hide resolved
doc/examples/uncertainty_emcee.ipynb Outdated Show resolved Hide resolved
@martinvonk martinvonk modified the milestones: 1.1, 1.2 Aug 3, 2023
@raoulcollenteur
Copy link
Member Author

Nice, this looks cool!

Thanks for the review! I made all the your suggested in the notebook and code. I think

Besides the comments I made in the code a couple of comments here:

  • Perhaps it would be interesting to compare the priors and posteriors for one or more parameters in the example notebook?

I will note this one down for a future development :)

  • Can we help the user a bit more selecting some default emcee parameters, if that is possible?

Not really to be honest, if people want to use this solver they should really read the emcee docs before. I removed some of the defaults in the docstrings, and in the notebook I just want to show how to use them. Then users can figure this out themselves for now. In later stage after we got some more experience with the solver ourselves we may be able to give more guidance..

  • Add emcee group to pyproject.toml with optional packages: [emcee, corner, tqdm]?

Yes, Done!

Let me know if all is good and if I may merge :)

Cheers,
Raoul

Copy link
Member

@dbrakenhoff dbrakenhoff left a comment

Choose a reason for hiding this comment

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

Looks good!

@raoulcollenteur raoulcollenteur merged commit 9cf6126 into dev Aug 15, 2023
7 of 9 checks passed
@raoulcollenteur raoulcollenteur deleted the 559-emceesolve branch August 15, 2023 05:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Indicates development of new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DEVELOPMENT] Add EmceeSolve for MCMC
3 participants