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

MMM load updates #317

Merged
merged 8 commits into from
Jul 13, 2023
Merged

Conversation

michaelraczycki
Copy link
Contributor

@michaelraczycki michaelraczycki commented Jul 5, 2023

fixed dims format, added save_load test to delayed_saturated_mmm. Now MMM classes can properly use save/load functionality


📚 Documentation preview 📚: https://pymc-marketing--317.org.readthedocs.build/en/317/

@michaelraczycki
Copy link
Contributor Author

the failure of test CI is expected, as a release on pymc-experimental is needed. Please add your thoughts regardless, so we can proceed when release is done

@codecov
Copy link

codecov bot commented Jul 12, 2023

Codecov Report

Merging #317 (8755a41) into main (b8116e1) will increase coverage by 0.29%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #317      +/-   ##
==========================================
+ Coverage   95.16%   95.45%   +0.29%     
==========================================
  Files          19       19              
  Lines        1406     1430      +24     
==========================================
+ Hits         1338     1365      +27     
+ Misses         68       65       -3     
Impacted Files Coverage Δ
pymc_marketing/clv/models/basic.py 100.00% <100.00%> (+2.22%) ⬆️
pymc_marketing/mmm/base.py 97.50% <100.00%> (+0.02%) ⬆️
pymc_marketing/mmm/delayed_saturated_mmm.py 99.46% <100.00%> (+0.08%) ⬆️

@twiecki
Copy link
Contributor

twiecki commented Jul 12, 2023

@ricardoV94 can we merge this?

Copy link
Contributor

@ricardoV94 ricardoV94 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a nitpick with the name

@@ -30,6 +30,8 @@

class BaseMMM(ModelBuilder):
model: pm.Model
_model_type = "baseMMM"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_model_type = "baseMMM"
_model_type = "BaseMMM"

@michaelraczycki michaelraczycki merged commit 9c609e7 into pymc-labs:main Jul 13, 2023
11 checks passed
michaelraczycki added a commit to michaelraczycki/pymc-marketing that referenced this pull request Jul 26, 2023
* fixed dims format, added save_load test to delayed_saturated_mmm

* removing temp file after test_save_load in DelayedSaturatedMMM tests

* unifying save tests, adding model.id checks into load

* pymc-experimental version bump

* removing property @posterior_predictive from basic.py

* extending tests to cover id preservation with load

* _model_type update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants