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

Speed up dev_beta_binom() by adding a scalar version of the log-likelihood function, a vectorized optimization routine, and memoization. #69

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

nehill197
Copy link
Member

  • Added a scalar case to the log-likelihood function to improve speed for scalar inputs.
  • Add memoization (if package is installed) and data has > 800 rows to gain speed from repeated function calls.
  • Use a vectorized optimization function to improve speed of optimization required for deviance calculation.

@nehill197 nehill197 marked this pull request as draft August 12, 2024 16:47
@nehill197 nehill197 self-assigned this Aug 12, 2024
@nehill197 nehill197 marked this pull request as ready for review August 12, 2024 16:59
@nehill197 nehill197 requested a review from joethorley August 12, 2024 16:59
Copy link
Member

@joethorley joethorley left a comment

Choose a reason for hiding this comment

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

I would move the optimize_R code to a .R file in /scripts so that not cluttering up the /R folder

R/log-lik.R Outdated
# x and size unchanged; memoize it to reduce repeated calls
# when length(x) is large enough to outweigh the overhead required by memoize.
# For length(x) < 800, memoize is slower
# https://poissonconsulting.slack.com/archives/C07FC57Q346/p1723248220478269
Copy link
Member

Choose a reason for hiding this comment

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

I would delete this link as it's public repo and only works for poisson folks. Add info to an issue in extras and link to that if critical.

Copy link
Member

@joethorley joethorley left a comment

Choose a reason for hiding this comment

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

Awesome

@nehill197
Copy link
Member Author

Going to wait to merge until current CRAN submission is complete.

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.

2 participants