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

#1035 Distribution new customer enhancements #1061

Merged

Conversation

Ishaanjolly
Copy link
Contributor

@Ishaanjolly Ishaanjolly commented Sep 22, 2024

Distribution new customer enhancements

Description

I added n_samples: int = 1000 to _distribution_new_customers and modified this line within:

        if dataset.sizes["chain"] == 1 and dataset.sizes["draw"] == 1:
            # For map fit add a dummy draw dimension
            dataset = dataset.squeeze("draw").expand_dims(draw=range(n_samples))

Additionally:
I added n_samples: int = 1 to distribution_new_customer_recency_frequency and modified this:

self._distribution_new_customers(
            data=data,
            T=T,
            random_seed=random_seed,
            var_names=["recency_frequency"],
            n_samples=n_samples,
        )["recency_frequency"]

Related Issue

Checklist

Modules affected

  • MMM
  • CLV

Type of change

  • New feature / enhancement
  • Bug fix
  • Documentation
  • Maintenance
  • Other (please specify):

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

@@ -570,7 +570,7 @@ def plot_curve(
subplot_kwargs : dict, optional
Addtional kwargs to while creating the fig and axes
sample_kwargs : dict, optional
Kwargs for the :func:`plot_curve` function
Kwargs for the :func:`plot_sample` function
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we leave this change out of this PR? 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just done this thanks

Copy link

codecov bot commented Sep 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.60%. Comparing base (1a1703c) to head (7b14ccc).
Report is 95 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1061      +/-   ##
==========================================
- Coverage   95.85%   95.60%   -0.26%     
==========================================
  Files          39       39              
  Lines        3934     3934              
==========================================
- Hits         3771     3761      -10     
- Misses        163      173      +10     
Flag Coverage Δ
95.60% <100.00%> (-0.26%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@juanitorduz juanitorduz merged commit d05c2d8 into pymc-labs:main Sep 23, 2024
12 checks passed
@juanitorduz
Copy link
Collaborator

Thank you @Ishaanjolly !

@ColtAllen
Copy link
Collaborator

Sorry that I was late in seeing this, but the same changes can also be applied to ParetoNBDModel. It's also worth noting this PR is related to #878 but I'm not entirely sure if that issue is worth pursuing.

@juanitorduz
Copy link
Collaborator

Thanks @ColtAllen ! @Ishaanjolly would you wanna do something similar to ParetoNBDModel?

@Ishaanjolly
Copy link
Contributor Author

Sure, I'll create a new issue for it and pursue it. Thanks @juanitorduz and @ColtAllen

@ColtAllen would also appreciate your suggestions on best way to cache log_p outputs.

jsnyde0 pushed a commit to jsnyde0/pymc-marketing that referenced this pull request Sep 24, 2024
* feat: test.txt added for commit check

* feat: replaced plot_curve with plot_samples within ./mmm/plot.py

* feat: n_samples added to distributions_new_customers

* revert the plot.py changes

---------

Co-authored-by: Juan Orduz <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants