Remove dynamic risk factor mechanism #189
Conversation
|
clang-tidy review says "All clean, LGTM! 👍" |
There was a problem hiding this comment.
Is this model something truly obsolete that will not be needed anymore? Or is it still potentially useful but does not fit within the new way of defining models?
I'm just asking because deleting unused things is always good... as long as it is certain they will no longer be needed.
|
I discussed it with Israel, also included here, and the consensus seemed to be exactly that. I'm to understand that neither models use it, nor will use it. |
|
I will be removing e.g, the risk factors from the main config file, but it's up to you. If so, I'll try to be careful and synchronise. Edit: I need to look over the course material for Thursday anyway, so if you like I can hold fire until you finish entirely. |
Co-authored-by: Alex Dewar <[email protected]>
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
|
clang-tidy review says "All clean, LGTM! 👍" |
There was a problem hiding this comment.
Looks good, just two minor suggestions.
Note:
The dynamic factor was originally defined as part of a generic regression framework to represent dynamic risk factor for adults, it included a time dimension, usually year in the regression variables to move the population over time. This model was dropped in favor of the current EB model for children, never used, and the code removed from the main branch.
The new EB model is not regression-based, it is safe to remove the dynamic risk factor concept from the source code and configuration.
|
@jamesturner246 Ok cool. For future reference, I'd probably have updated the config files in this PR too just to make sure the code still works. It's fine to do it later though. Maybe open an issue? I'm happy to hold off on #161 until you've made those changes. |
|
@alexdewar , the modifications to the config file required for this change are already done. I simply removed |
|
clang-tidy review says "All clean, LGTM! 👍" |
1 similar comment
|
clang-tidy review says "All clean, LGTM! 👍" |
Should be straightforward. Just a bunch of deleted code for the unused dynamic risk factor mechanism that is no longer needed in the hierarchical linear regression models.
I'll include @israel-vieira in the loop, if interested. Obviously no commitment to a review expected though!
Fixes #183