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

Create inverse_scaled_logistic_saturation and the corresponding class #827

Merged

Conversation

arthurmello
Copy link
Contributor

@arthurmello arthurmello commented Jul 10, 2024

Description

Offer a more intuitive alternative to logistic_saturation, where we use
$$f(x,λ)=\frac{1−e^{−xln⁡(3)/λ}}{1+e^{−xln⁡(3)/λ}}$$

Instead of the original:
$$f(x,λ)=\frac{1−e^{−xλ}}{1+e^{−xλ}}$$

Allowing for lambda to be interpreted as the half saturation point.

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--827.org.readthedocs.build/en/827/

@arthurmello arthurmello changed the title created inverse_scaled_logistic_saturation and the corresponding class Create inverse_scaled_logistic_saturation and the corresponding class Jul 10, 2024
@juanitorduz juanitorduz requested a review from wd60622 July 10, 2024 18:40
Copy link
Contributor

@wd60622 wd60622 left a comment

Choose a reason for hiding this comment

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

Amazing stuff. Thanks for this contribution!

Can you test that the tranformation at lam is about 0.5? It should be test in transformations.py

pymc_marketing/mmm/components/saturation.py Outdated Show resolved Hide resolved
@wd60622 wd60622 added enhancement New feature or request MMM labels Jul 10, 2024
Comment on lines 516 to 517
eps : float or array-like, optional, by default ln(3)
Scaling parameter.
Copy link
Contributor

Choose a reason for hiding this comment

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

add that this results in halfway saturation at lam

pymc_marketing/mmm/transformers.py Show resolved Hide resolved
@arthurmello arthurmello force-pushed the issue-220-intuitive-logistic-saturation branch from d04fb80 to 0baf5f4 Compare July 11, 2024 15:32
@arthurmello
Copy link
Contributor Author

@wd60622 I've updated the PR with those changes, let me know if there's anything else I can do :)

tests/mmm/test_transformers.py Outdated Show resolved Hide resolved
tests/mmm/test_transformers.py Show resolved Hide resolved
@wd60622 wd60622 marked this pull request as ready for review July 13, 2024 13:27
pymc_marketing/mmm/components/saturation.py Outdated Show resolved Hide resolved
pymc_marketing/mmm/transformers.py Outdated Show resolved Hide resolved
@arthurmello arthurmello requested a review from wd60622 July 16, 2024 12:58
Copy link
Contributor

@wd60622 wd60622 left a comment

Choose a reason for hiding this comment

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

One final request ☺️

tests/mmm/test_transformers.py Outdated Show resolved Hide resolved
Copy link
Contributor

@wd60622 wd60622 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 running tests one last time

@wd60622 wd60622 merged commit 6049ae8 into pymc-labs:main Jul 18, 2024
11 checks passed
@wd60622
Copy link
Contributor

wd60622 commented Jul 18, 2024

Thanks @arthurmello !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request MMM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants