-
Notifications
You must be signed in to change notification settings - Fork 4
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
Improve the GSF model implementation #249
Conversation
Code Coverage Summary
Diff against main
Results for commit: 887a170 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Unit Tests Summary 1 files 34 suites 2m 16s ⏱️ Results for commit 887a170. ♻️ This comment has been updated with latest results. |
Unit Test Performance Difference
Additional test case details
Results for commit e6412cc ♻️ This comment has been updated with latest results. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @gowerc , lots of work here, please see few comments below
@danielinteractive - I have addressed the comments you made, please let me know if there is anything else |
Closes #123
Extends the GSF model to have a
centered
argument so that users can toggle between the centered and non-centered parameterisationsThere wasn't a natural conversion between centered and non-centered for the phi parameter so I have converted that to be a beta distribution in both cases (it appears to be stable in this formulation regardless.
Introduced a cron job that runs twice a month that runs the full joint GSF model as this unit test currently takes 30 minutes to run so didn't want to include it as part of the default run. This test checks that the model can recover known parameter values within the full model
Fixed a bug where the sparse matrix representation of subject -> visit mapping for the longitudinal data was incorrectly using a full intercept column instead of correctly specifying the index's for patient 1 in the first column
A handful of minor tweaks (added rounding to print messages, fixed some minor bugs that were blocking unit tests finishing)
Converted the GSF simulation function to just use the centered parameterisation (there was literally no point in this function using the non-centered parameterisation, it just made the code more complicated).
Includes a directory of example models that build up the GSF model in stages (can be removed as not needed for the package but figured it might be useful if we need to experiment in the future).
Added an up-to-date outline of the GSF model specification to the stats-specification vignette
For the non-centered parameterisation I converted the means to be on the log-scale so that the parameter estimates are consistent with the estimates in the centered parameterisation (as it is the log(mean) that you specify to the lognormal distribution).