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

Uncertainty #161

Merged
merged 17 commits into from
Nov 19, 2019
Merged

Uncertainty #161

merged 17 commits into from
Nov 19, 2019

Conversation

raoulcollenteur
Copy link
Member

Initial methods to add uncertainty calculations to Pastas Models.

For example, uncertainty calculations are called as follows after model optimisation:

ml.fit.prediction_interval()
ml.fit.ci_simulation()
  • An example is added (example_uncertainty.py)
  • Tests are added (test_solvers.py)

@raoulcollenteur raoulcollenteur added the development Indicates development of new features label Oct 25, 2019
@raoulcollenteur raoulcollenteur added this to the 0.13.0 milestone Oct 25, 2019
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.

Looking good! I added a few minor comments, mostly about code style that apply to several of the new methods (not just where i added the comment):

  • Do we hide internal methods in pastas by prepending _?
  • Should docstrings for internal methods be as complete as for public methods?

pastas/model.py Outdated Show resolved Hide resolved
@rubencalje
Copy link
Collaborator

Looks good. I do not have time to look into the code in detail though. Personally I would keep 'model' instead of 'ml', but if this helps to be consistent throughout Pastas it's better like this.

@dbrakenhoff
Copy link
Member

@mbakker7, could you review this PR (if you haven't already)? I'd like to start using/testing it from the dev branch.

Fit object is now stored when saving to a pas-file so uncertainty methods are available after loading a model from file. Also this commit solves the following two issues:

- Fix Issue #166
- Fix Issue #82
@raoulcollenteur raoulcollenteur merged commit b71d51d into dev Nov 19, 2019
@raoulcollenteur raoulcollenteur deleted the uncertainty branch November 19, 2019 07:24
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.

None yet

4 participants