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

Optimize _geometric_adstock for 99% function speedup, reducing total model train time by ~40% - Python version #1217

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

benshush
Copy link

@benshush benshush commented Feb 16, 2025

Project Robyn

Summary

This PR optimizes _geometric_adstock, the main performance bottleneck, reducing its execution time by ~99% and improving the total model train time by ~40% on Python version.

Fixes

This PR does not address an existing GitHub issue but significantly improves performance.

Type of change

  • fix: Performance optimization (non-breaking change that improves efficiency)
  • test: Added unit test to validate correctness

Motivation & Context

  • _geometric_adstock previously accounted for 39% of the total model train time, significantly slowing down model execution.
  • This function was rewritten using scipy.signal.lfilter, replacing a Python loop with a highly efficient SciPy-based implementation (built on NumPy).
  • This change improves execution time from 108.9s → 0.495s (~99% speedup), leading to an overall app model train time reduction of ~40%.

Performance Benchmark (tutorial1.ipynb, Trial=1, iterations=2000)

Before Optimization:

  • _geometric_adstock: 108.9s (39% of total model train time)
  • Total model train time: 277.9s

After Optimization:

  • _geometric_adstock: 0.495s (99% faster)
  • Total model train time: 166.7s (40% faster)

…~40%

- Replaced loop-based implementation with `scipy.signal.lfilter`
- Improved `_geometric_adstock` execution time from **108.9s → 0.495s** (~99% faster) per trial
- Reduced overall app runtime from **~277.9s → ~166.7s** (~40% speedup) per trial
- Added unit test to ensure numerical correctness (`np.allclose`)
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 16, 2025
@benshush benshush changed the title Optimize _geometric_adstock for 99% function speedup, reducing total model train time by ~40% Optimize _geometric_adstock for 99% function speedup, reducing total model train time by ~40% - Python version Feb 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants