Add HSGP prior #415
Add HSGP prior #415
Conversation
|
Is setting time-varying coefficients something one wants per default though? |
It won't be default, it'll be an option within the config - in combination with the upcoming pr on prior distribution flexibility. Model fit times using the HSGP are very efficient too, I found in testing there's minimal difference when using them in a single KPI marketing-mix model on all channels. |
|
I suggest we first merge #397 and then keep building this PR on top of it. This PR adds much more functionality and will also require more careful testing / design tweaking. |
|
I think flexibility in the order of adstock--> saturation and saturation --> adstock would also be useful. Additionally, It'd be nice to enable assignment of different adstock functions to different channels. Then, it would maybe make sense to instead of having a model_config, you have a adstock_config and saturation_config. |
ricardoV94
changed the title
| priors[param] = self.gp_wrapper(name=param, X=np.arange(len(self.X[self.date_column]))[:, None], config=config, positive=is_positive) | ||
| continue | ||
|
|
||
| length = dimensions.get(config.get("dims", [None, None])[1], 1) |
There was a problem hiding this comment.
needlessly terse. can be simplified to
dimensions.get(config.get("dims", None), 1)
There was a problem hiding this comment.
my apologies, my suggesion won't work and this is actually decent code.
| # Initial value based on parameter name | ||
| is_positive = param in positive_params | ||
|
|
||
| # Override if the config explicitly sets the 'positive' key | ||
| if 'positive' in config: | ||
| is_positive = config.get('positive') |
There was a problem hiding this comment.
This can be simplified to:
is_positive = config.get('positive', param in positive_params)| eta = pm.Exponential(f"_eta_{name}", lam=1) | ||
| # cov = eta ** 2 * pm.gp.cov.ExpQuad(1, ls=ell) | ||
|
|
||
| cov = eta ** 2 * pm.gp.cov.Matern32(1, ls=ell) |
There was a problem hiding this comment.
I don't get this. eta ** 2 is going to blow up for negative x. Plotting eta ** 2 * pm.gp.cov.Matern32(1, ls=ell) for x' = 0 and lam = 1:
Is the point to only let coef value at t = 0 be influenced by coef values at t < 0?
There was a problem hiding this comment.
Hey, apologies, I've been a bit slow on this one. There's been quite a few changes to the main with the custom priors/likelihood pr & shortly more with the out-of-sample prediction.
@bwengals is going to be helping out implementing this one too
There was a problem hiding this comment.
No worries. For context, I'm here because I'm adding TVC to my company's MMM, and thought it would make sense to drop review comments for clarification sake (and to give back, thanks for the beautiful lib). But if any of these comments are out of place, please just ignore them.
|
No worries at all. The more people contributing behind pymc-marketing the better! Want to have a catch up anyway? I would be interested in hearing how you're using the library too.
https://calendly.com/niall-oulton
Get Outlook for iOS<https://aka.ms/o0ukef>
________________________________
From: Ulf Aslak ***@***.***>
Sent: Tuesday, January 16, 2024 8:41:27 PM
To: pymc-labs/pymc-marketing ***@***.***>
Cc: Niall Oulton ***@***.***>; Author ***@***.***>
Subject: Re: [pymc-labs/pymc-marketing] Add HSGP prior (PR #415)
@ulfaslakprecis commented on this pull request.
________________________________
In pymc_marketing/mmm/delayed_saturated_mmm.py<#415 (comment)>:
+ print(dims)
+ gp_list = [self.gp_wrapper(name=f"{param}_{i}", X=np.arange(len(self.X[self.date_column]))[:, None], config=config, positive=positive) for i in range(length)]
+ stacked_gp = pt.stack(gp_list, axis=1)
+ return pm.Deterministic(f"{param}", stacked_gp, dims=dims)
+
+
+ def gp_wrapper(self, name, X, config, positive=False, **kwargs):
+ return self.gp_coeff(X, name, config=config, positive=positive, **kwargs)
+
+ def gp_coeff(self, X, name, mean=0.0, positive=False, config=None):
+ params = pm.find_constrained_prior(pm.Gamma, 8, 12, init_guess={"alpha": 1, "beta": 1}, mass=0.8)
+ ell = pm.Gamma(f"ell_{name}", **params)
+ eta = pm.Exponential(f"_eta_{name}", lam=1)
+ # cov = eta ** 2 * pm.gp.cov.ExpQuad(1, ls=ell)
+
+ cov = eta ** 2 * pm.gp.cov.Matern32(1, ls=ell)
No worries. For context, I'm here because I'm adding TVC to my company's MMM, and thought it would make sense to drop review comments for clarification sake (and to give back, thanks for the beautiful lib). But if any of these comments are out of place, please just ignore them.
—
Reply to this email directly, view it on GitHub<#415 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/A5SZLZIFLIY26B3DXTAIEBDYO3QXPAVCNFSM6AAAAAA6WOHAVSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQMRUHEZTOMJUGM>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
|
@ulfaslak do we have a new pr for TVPs or are we doing it within this one? |
Might suggest new one unless ulf is rebase genie 😁 |
|
@nialloulton @wd60622 I'm working on a new one :). |
|
Good to close this one @ulfaslak @nialloulton ? |
|
Closing following #628 |
Essentially 3 things:
Some inefficient functions can be deleted e.g GP_wrapper can be edited out of the code
📚 Documentation preview 📚: https://pymc-marketing--415.org.readthedocs.build/en/415/