-
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
Rework link functions #259
Conversation
Just read through the description, and looks very good to me @gowerc ! |
Just added my first attempt at the documentation pages, though still need to add a general "extending jmpost vignette" |
Ok I think this is 95% done now. Only things outstanding that I still need to add are:
|
Code Coverage Summary
Diff against main
Results for commit: 835098c Minimum allowed coverage is ♻️ This comment has been updated with latest results |
Unit Test Performance Difference
Additional test case details
Results for commit 30a7319 ♻️ This comment has been updated with latest results. |
Unit Tests Summary 1 files 34 suites 5m 39s ⏱️ Results for commit 835098c. ♻️ This comment has been updated with latest results. |
@danielinteractive - Pending fixing up any CICD findings this is now complete and ready for review/merging |
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 , looks nice, see a few minor comments below
Co-authored-by: Daniel Sabanes Bove <[email protected]>
Closes #238
I think I've finished the core functionality but still need to re-do the docs and unit tests. Was just hoping to share to see if there was any initial concerns / feedback.
Theres a lot going on in this PR so the below attempts to highlight the core components + how it works:
In terms of the UI the end users would now call the modelling functions like:
Or if they have multiple links:
Going one layer down, the workhorse object is the
LinkComponent
. The idea is these would be exported so users can define their own link code if needed:In terms of the required stan code, the current interface for creating your own link is a stan function of the signature:
Where
link_function_inputs
is an arbitrary matrix that is defined by the longitudinal model. For example for the GSF model it is defined as:Technically it is possible for users to extend the
link_function_inputs
matrix from the default though the process of doing so is a little involved. That is, under the hood the longitudinal models define this Stan code by implementing theenableLink()
method, so users can subclass the model and then implement their ownenableLink()
method to override the default. For reference this is the current enableLink for the RandomSlope model:In terms of our implementation of the common links (e.g. dsld/ttg) we allow the
stan
argument toLinkComponent
to be a method which will be called against the Longitudinal model object allowing the models to provide their own implementations of each link. For example the actuallink_dsld()
function is:And then we implement model specific methods to provide individual model implementations e.g.
And finally as all models subclass from the base
LongitudinalModel
class I've also provided default implementations to throw meaningful errors if a model doesn't have a method defined e.g.