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

Add ability to extract raw samples #221

Merged
merged 11 commits into from
Sep 26, 2023
Merged

Add ability to extract raw samples #221

merged 11 commits into from
Sep 26, 2023

Conversation

gowerc
Copy link
Collaborator

@gowerc gowerc commented Sep 22, 2023

Closes #220

Fixes previous regression of not being able to return the individual quantity samples. I ended up having to re-work the functions a little bit as it was getting too nested. With this change the api has gone from:

survival_samples <- SurvivalSamples(stan_samples)
predict(
    survival_samples,
    patients = pts,
    type = "haz",
    time_grid = c(0, 100, 200)
)
autoplot(
    survival_samples,
    patients = pts,
    time_grid = c(0, 100, 200),
    type = "haz"
)

to

samps <- extractSurvivalQuantities(
    stan_samples,
    patients =  sample(dat_os$pt, 4),
    type = "haz",
    time_grid = c(0, 100, 200)
)
summary(samps)
as.data.frame(samps) |> tibble()
autoplot(samps)

To summarise

  • The logic of running generated_quantities has been moved into the constructor-method rather than the individual predict / autoplot methods
  • new as.data.frame method to return the raw samples
  • predict has been re-named as summary which I felt better reflected what was happening
  • Renamed the underlying object as SurvivalQuantities (think this slightly better represents what they are)

Currently marked as "draft" as I still need to update the documentation / unit tests

@danielinteractive
Copy link
Collaborator

Thanks @gowerc , sounds good - one thought on shorter naming, how about survival() instead of extractSurvivalQuantities() ? I think when applied to a samples object it is clear that we will get survival samples then.

@gowerc gowerc marked this pull request as ready for review September 25, 2023 16:32
@gowerc
Copy link
Collaborator Author

gowerc commented Sep 25, 2023

@danielinteractive - Pending CICD corrections this is now ready for review.

Regarding the function name I feel very conflicted. General convention for methods is to be verb focused "extract", "as", "sample", "get". Part of me wonders if survival is too generic in that this is really reducing the model down to pure the generated-quantities no other part of the survival model is left. In theory you could handle this with another layer but that feels a bit unnecessary in terms of user interactions e.g.

survsamps <- survival(jointsamples)
survquant <- quantities(survsamps, time_grid = ...)

I do appreciate / agree the current method name is a bit excessively verbose but the alternative doesn't quite feel "right" to me.

@danielinteractive
Copy link
Collaborator

Ok yeah good thoughts on the naming. Not super important so can leave as is

Copy link
Collaborator

@danielinteractive danielinteractive left a comment

Choose a reason for hiding this comment

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

Thanks @gowerc I browsed through it and looks good to me, only found some typos

vignettes/model_fitting.Rmd Outdated Show resolved Hide resolved
vignettes/model_fitting.Rmd Outdated Show resolved Hide resolved
@gowerc
Copy link
Collaborator Author

gowerc commented Sep 26, 2023

Another alternative is to move the implementation to the constructor so that the call is slightly shorter e.g.

samps <- SurvivalQuantities(stan_samples)

@gowerc gowerc changed the title WIP - Add ability to extract raw samples Add ability to extract raw samples Sep 26, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Sep 26, 2023

Unit Tests Summary

    1 files    25 suites   3m 8s ⏱️
  57 tests   56 ✔️ 1 💤 0
228 runs  227 ✔️ 1 💤 0

Results for commit ee032e0.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 26, 2023

badge

Code Coverage Summary

Filename                       Stmts    Miss  Cover    Missing
---------------------------  -------  ------  -------  ----------------------
R/DataJoint.R                     27       0  100.00%
R/DataLongitudinal.R              71       0  100.00%
R/DataSurvival.R                  46       0  100.00%
R/defaults.R                      11       6  45.45%   27, 57-84, 111
R/generics.R                      11       1  90.91%   173
R/JointModel.R                    46       5  89.13%   85-99
R/JointModelSamples.R             54       0  100.00%
R/Link.R                           6       0  100.00%
R/LinkGSF.R                       63      12  80.95%   109-120
R/LinkNone.R                       3       1  66.67%   36
R/LinkRandomSlope.R               10       0  100.00%
R/LongitudinalGSF.R               23       0  100.00%
R/LongitudinalModel.R             10       0  100.00%
R/LongitudinalRandomSlope.R       17       1  94.12%   43
R/LongitudinalSamples.R           15       0  100.00%
R/Parameter.R                     12       0  100.00%
R/ParameterList.R                 28       0  100.00%
R/Prior.R                         58       9  84.48%   127-135
R/simulations_gsf.R               43       0  100.00%
R/simulations_os.R                11       5  54.55%   35-39
R/simulations_rs.R                21       0  100.00%
R/simulations.R                   99       1  98.99%   122
R/StanModel.R                      7       0  100.00%
R/StanModule.R                   145       5  96.55%   185-186, 202, 235, 246
R/SurvivalExponential.R            9       0  100.00%
R/SurvivalLoglogistic.R           10      10  0.00%    31-40
R/SurvivalModel.R                 12       0  100.00%
R/SurvivalQuantities.R           156      23  85.26%   179-202
R/SurvivalWeibullPH.R             10       0  100.00%
R/utilities.R                    146       1  99.32%   13
R/zzz.R                           11      11  0.00%    4-29
TOTAL                           1191      91  92.36%

Diff against main

Filename                  Stmts    Miss  Cover
----------------------  -------  ------  -------
R/SurvivalQuantities.R     +156     +23  +85.26%
TOTAL                      +156     +23  -1.77%

Results for commit: 5971760

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@gowerc gowerc merged commit f3732a4 into main Sep 26, 2023
20 checks passed
@gowerc gowerc deleted the fix/sample-regression branch September 26, 2023 13:23
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.

Add ability to exact predicted SLD / Survival raw samples
2 participants