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

(WIP) Design of a BayesianEstimator Class for sktime and skpro #35

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

meraldoantonio
Copy link

Bayesian estimators uniquely integrate prior knowledge through prior distributions and update this knowledge based on observed data to form posterior distributions. To accommodate the distinctive workflow of Bayesian estimators, we propose designing a new BayesianEstimator class.

I'm opening this PR to discuss various ideas for implementing this template. Any contributions or ideas are welcome!

@meraldoantonio meraldoantonio marked this pull request as draft June 12, 2024 16:13
@meraldoantonio meraldoantonio changed the title (WIP) Design of a BayesianEstimator Class for sktime and skpro (WIP) Design of a BayesianEstimator Class for sktime and skpro #358 Jun 13, 2024
@meraldoantonio meraldoantonio changed the title (WIP) Design of a BayesianEstimator Class for sktime and skpro #358 (WIP) Design of a BayesianEstimator Class for sktime and skpro Jun 13, 2024
@meraldoantonio
Copy link
Author

Linking sktime/skpro#358 (skpro's WIP BayesianLinearRegressor class) as an application of this template

@fkiraly
Copy link
Contributor

fkiraly commented Jun 13, 2024

Related: intermediate base class from prophetverse by @felipeangelimvieira:
https://github.com/felipeangelimvieira/prophetverse/blob/main/src/prophetverse/sktime/base.py
(imo: this is at a lower degree of generality than what we may want, but is pertinent design-wise)

@fkiraly
Copy link
Contributor

fkiraly commented Jun 15, 2024

First high-level comments: I think we ought to write up what the key workflow elements are that every Bayesian model needs to support.

I think:

  • passing prior distributions at construction
  • inspecting or retrieving posterior distributions
  • Bayesian update of an already fitted model

Thoughts about the state transitions and related interfaces:

  • priors must be passed before fit. So there are only two option: in __init__ in a dedicated argument (or multiple), or in a separate method. So, three options: init, multiple args; init, single arg; separate method
  • posteriors can be retrieved anytime after fit. There already exists an interface point for this, get_fitted_params. Again a second option is adding a specialized method. Again, three options: get_fitted_params, multiple params; same, one param; separate method
  • Bayesian update is similar to general update, such as the update method in forecaster. This does not exist in skpro tabular regressors, but could be added, similar to the river interface.

Further thoughts: for priors and posteriors, it might be worth using a structured object, similar to pymc handles them. Another question is whether we want to use skpro distributoins for inputs or outputs, or another standard.

Some preliminary desiderata:

  • proliferation of methods should be avoided, all other things being equal
  • unified interfaces that are easy to comply with are better than those that are difficult to follow, and that´s better than non-unified

From this, some slight preferences:

  • mapping priors on __init__, and posteriors on get_fitted_params
  • unification: single prior param argument and posterior_ fitted param
  • that means, we need a BaseObject child class for priors/posteriors.
    • this could be a BaseDistribution representing multiple, potentially associated random variables?

Regarding plotting, as mentioned in the document: if we use BaseDistibution objects as representation, then we get "for free" the plotting functions attached, i.e., plot. Though, question, what are common diagnostic plots?

@fkiraly
Copy link
Contributor

fkiraly commented Jun 15, 2024

FYI @felipeangelimvieira

@meraldoantonio
Copy link
Author

I've updated the design document to include these ideas, and I believe the desiderata make sense. Mapping priors in __init__ and posteriors in get_fitted_params is also logical.

However, I have two areas where I'm unsure of:

Representing Multiple Associated Random Variables:
Can we use a single BaseDistribution class to represent multiple, potentially associated random variables? This approach would need careful consideration to handle dependencies and interactions between the variables effectively.

Bayesian Workflow Integration:
Besides passing a list of priors in __init__, we need to include information on how these priors build on each other to form a Bayesian workflow. Should this be part of __init__, or do we "freeze" the workflow within a class, making each class specific to a workflow (e.g., Bayesian regression as one class)?

@felipeangelimvieira
Copy link

Hi @meraldoantonio!
Recently started reading more about PyMC, and they have just added a custom Prior class in their "PyMC-Marketing" lib. Maybe this can help with ideas: code here

@meraldoantonio
Copy link
Author

Re-opening this thread and this work stream.

Following @felipeangelimvieira's suggestion - I looked at pymc-marketing's Prior class and it looks like a suitable way to specify the prior distributions. It is flexible and is native to pymc ecosystem. Also, the fact that the library already comes with the Bayesian template ("ModelBuilder") means that we can repurpose a lot of functionalities for our purpose.
An example of the application of the ModelBuilder is here.

@meraldoantonio
Copy link
Author

Copying @fkiraly 's points from our meeting:

  • reflections:
    • is there option to "mix"? E.g., have a "simple" option with prescribed priors and configurable?
    • are there any dependencies that are specific to the specification? E.g., if we use Prior from pymc-marketing, would that imply more deps than the models have?
      • FK: if estimator also uses pymc-marketing internally, that's probably ok
      • but not all Bayesian estimators will use pymc-marketing. I think taking a dependency for pure speciication reasons is a bad idea
        • story/experience: originally we wanted to rely on tensorflow-probability for the proba distributions, as return objects; but this turned out a bad choice since tensorflow got less popular and the proba distirbutions got less maintained, yet implied a heavy dependency footprint for the sole reason of specification
  • minor points:
    • what is the reliability of pymc-marketing - usage and maintenance footprint, this is not pymc itself

@meraldoantonio
Copy link
Author

Summary of updates:

  • Enhancement Proposals Update:
    Provided workflow ideas for the Bayesian class and included an example implementation (BayesianLinearRegression). This class utilizes PyMC for inference, ArViz for data handling, and pymc-marketing's Prior for prior specification. The updated Template.md justifies these design choices.

  • Implementation Update:
    Enhanced the BayesianLinearRegression class in skpro. Included an updated implementation and a notebook demonstrating its usage.

@meraldoantonio meraldoantonio marked this pull request as ready for review August 30, 2024 06:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: PR in progress
Development

Successfully merging this pull request may close these issues.

3 participants