pymc-labs / pymc-marketing

Bayesian marketing toolbox in PyMC. Media Mix (MMM), customer lifetime value (CLV), buy-till-you-die (BTYD) models and more.
https://www.pymc-marketing.io/
Apache License 2.0
683 stars 190 forks source link

MAP fits Failing for `BetaGeoModel` #214

Closed ColtAllen closed 1 year ago

ColtAllen commented 1 year ago

Should probably fix this before the Official Release:

from lifetimes.datasets import load_cdnow_summary
from pymc_marketing.clv import BetaGeoModel

df = load_cdnow_summary()

model = BetaGeoModel(customer_id=df['ID'], 
                     frequency=df['frequency'], 
                     recency=df['recency'],
                     T=df['T'],
                    )

model.fit(fit_method='map')
SamplingError: Initial evaluation of model at starting point failed!
Starting values:
{'a_log__': array(0.), 'b_log__': array(0.), 'alpha_log__': array(0.), 'r_log__': array(0.)}

Initial evaluation results:
{'a': 0.0, 'b': 0.0, 'alpha': 0.0, 'r': 0.0, 'likelihood': nan}
ricardoV94 commented 1 year ago

Can probably use a custom initial value for the priors if that's the problem.

larryshamalama commented 1 year ago

I think that custom initial values can work.

However, why are the parameters in the initial evaluations 0? The starting values have their log values to be 0. Shouldn't the initial evaluations yield parameter values of 1?

ricardoV94 commented 1 year ago

This has caused a lot of confusion: the second dictionary contains the log-probabilities. We should write it explicitly in the error message

larryshamalama commented 1 year ago

This has caused a lot of confusion: the second dictionary contains the log-probabilities. We should write it explicitly in the error message

I can create a beginner friendly issue in the PyMC repo later today

ricardoV94 commented 1 year ago

This has caused a lot of confusion: the second dictionary contains the log-probabilities. We should write it explicitly in the error message

I can create a beginner friendly issue in the PyMC repo later today

That would be good!

ricardoV94 commented 1 year ago

For sorting the issue, we need to know why is a = b = alpha = r = 1 invalid, and what would be a safe default instead. This could also mean that simple positive priors may be incorrect for this model?

ColtAllen commented 1 year ago

For sorting the issue, we need to know why is a = b = alpha = r = 1 invalid, and what would be a safe default instead. This could also mean that simple positive priors may be incorrect for this model?

Parameter values must be positive, but a and b are parameters of a beta distribution. Might work better to pool those priors. Here's some code I wrote in the past for this model:

https://github.com/ColtAllen/btyd/blob/main/btyd/models/beta_geo_model.py#L97

ricardoV94 commented 1 year ago

Regularizing the priors is certainly important, but the snippet above is failing already at the initial point. That means those values can't all be 1 for this dataset (or if they can, our logp implementation is too unstable)

ColtAllen commented 1 year ago

In the logp expression:

c4 = a / (b + x - 1)

logp = d1 + d2 + pt.log(c3 + c4 * pt.switch(x_zero, 1, 0))

c4 will throw a DIV/0 if b=1 and x=0. Should it be placed inside pt.switch()?

larryshamalama commented 1 year ago

In the logp expression:

c4 = a / (b + x - 1)

logp = d1 + d2 + pt.log(c3 + c4 * pt.switch(x_zero, 1, 0))

c4 will throw a DIV/0 if b=1 and x=0. Should it be placed inside pt.switch()?

That sounds right! Good catch

ricardoV94 commented 1 year ago

Cool, yeah sounds like that would solve it

larryshamalama commented 1 year ago

I can spin a PR for this