Skip to content

Replaced RLL with faster version#7

Open
sapphrodite wants to merge 2 commits intoLanceFiondella:masterfrom
sapphrodite:master
Open

Replaced RLL with faster version#7
sapphrodite wants to merge 2 commits intoLanceFiondella:masterfrom
sapphrodite:master

Conversation

@sapphrodite
Copy link

No description provided.

Copy link
Collaborator

@jacobaubertine jacobaubertine left a comment

Choose a reason for hiding this comment

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

  • The print() statements are still included. These print a lot of lines and seem to impact performance, so they should be removed or replaced with logging functions.
  • Using the current RLL function with the 8 hazard functions on DS1, 62 of the 64 hazard function/covariate combinations are considered converged according to scipy.optimize.root(). With the new RLL, only 56 of 64 combinations converge.
  • 5 of the Type III discrete Weibull combinations look like they do not converge properly when compared to the current implementation.
  • All of the discrete Weibull (order 2) combinations that converged report 'nan' for log-likelihood, AIC, and BIC values in the model comparison table. The same is true for the negative binomial combinations, except they report '-inf'.

@sapphrodite
Copy link
Author

These were both caused by an improper integration - C-SFRAT expects NANs to be returned as NANs (not INFs), and I made an off-by-one error while transcribing hazard logic. I ran the test on all 64 combinations for DS1, and couldn't find any erroneous values. I've pushed the fix - let me know if there's anything else.

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