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

saving with media transformation config #826

Closed
wd60622 opened this issue Jul 10, 2024 · 3 comments · Fixed by #882
Closed

saving with media transformation config #826

wd60622 opened this issue Jul 10, 2024 · 3 comments · Fixed by #882
Labels
bug Something isn't working MMM
Milestone

Comments

@wd60622
Copy link
Contributor

wd60622 commented Jul 10, 2024

MMM with adstock and saturation configurations can potentially not be saved correctly since the additional kwargs are not saved off.

Having these classes serializable and taking advantage of them in save and load methods would be a good idea.

For instance, a model that would not be able to be loaded correctly would be:

from pymc_marketing.mmm import MMM, GeometricAdstock, LogisticSaturation

mmm = MMM(
    ..., 
    adstock=GeometricAdstock(l_max=4, normalize=False, mode="Before"), 
    saturation=LogisticSaturation(),
    ..., 
)

This is because both the normalize and mode are not saved off.

This would involve reconstructing the adstock and saturation upon loading here:

model = cls(
date_column=json.loads(idata.attrs["date_column"]),
control_columns=json.loads(idata.attrs["control_columns"]),
# Media Transformations
channel_columns=json.loads(idata.attrs["channel_columns"]),
adstock_max_lag=json.loads(idata.attrs["adstock_max_lag"]),
adstock=json.loads(idata.attrs.get("adstock", "geometric")),
saturation=json.loads(idata.attrs.get("saturation", "logistic")),
adstock_first=json.loads(idata.attrs.get("adstock_first", True)),
# Seasonality
yearly_seasonality=json.loads(idata.attrs["yearly_seasonality"]),
# TVP
time_varying_intercept=json.loads(
idata.attrs.get("time_varying_intercept", False)
),
time_varying_media=json.loads(
idata.attrs.get("time_varying_media", False)
),
# Configurations
validate_data=json.loads(idata.attrs["validate_data"]),
model_config=model_config,
sampler_config=json.loads(idata.attrs["sampler_config"]),
)

@wd60622 wd60622 added MMM bug Something isn't working labels Jul 10, 2024
@wd60622
Copy link
Contributor Author

wd60622 commented Jul 21, 2024

Think that this is rather important but the version of the model will likely have to be updated. Making the previous version not able to be reloaded.
Have any thoughts on this @juanitorduz ?

@wd60622
Copy link
Contributor Author

wd60622 commented Jul 21, 2024

There might be a benefit of having a to_json and from_json on the classes directly

@wd60622 wd60622 added this to the 0.8.0 milestone Jul 21, 2024
@juanitorduz
Copy link
Collaborator

Yes! Even though we "break" some model versions, we should fix this so that we can recover the complete (and consistent!) model.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working MMM
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants