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

corPFSOS #91

Merged
merged 27 commits into from
Nov 22, 2023
Merged

corPFSOS #91

merged 27 commits into from
Nov 22, 2023

Conversation

holgstr
Copy link
Contributor

@holgstr holgstr commented Nov 21, 2023

Fixes #90

@holgstr holgstr added the enhancement New feature or request label Nov 21, 2023
@holgstr holgstr self-assigned this Nov 21, 2023
@holgstr
Copy link
Contributor Author

holgstr commented Nov 21, 2023

@danielinteractive this is the draft for the corPFSOS function, without docu & tests :)

Copy link
Contributor

github-actions bot commented Nov 21, 2023

Unit Test Performance Difference

Test Suite $Status$ Time on main $±Time$ $±Tests$ $±Skipped$ $±Failures$ $±Errors$
getSimulatedDataDistib 💔 $95.65$ $+1.87$ $0$ $0$ $0$ $0$
Additional test case details
Test Suite $Status$ Time on main $±Time$ Test Case
getSimulatedDataDistib 💔 $49.72$ $+1.99$ getSimulatedData_generates_distributions_as_expected_Weibull

Results for commit c3401c7

♻️ This comment has been updated with latest results.

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 @holgstr , nice work, please see for suggested next steps below

R/corPFSOS.R Outdated Show resolved Hide resolved
R/corPFSOS.R Outdated Show resolved Hide resolved
R/corPFSOS.R Outdated Show resolved Hide resolved
R/estimateParams.R Outdated Show resolved Hide resolved
Copy link
Contributor

github-actions bot commented Nov 22, 2023

badge

Code Coverage Summary

Filename                     Stmts    Miss  Cover    Missing
-------------------------  -------  ------  -------  -----------
R/assertions.R                  15       0  100.00%
R/corPFSOS.R                    68      68  0.00%    14-300
R/empSignificant.R              59       0  100.00%
R/estimateParams.R              75       5  93.33%   35, 138-141
R/eventTracking.R               88       2  97.73%   48, 244
R/getClinicalTrials.R           71       0  100.00%
R/getSimulatedData.R           109       0  100.00%
R/getWaitTimeSum.R              29       2  93.10%   45, 48
R/hazardFunctions.R             26       0  100.00%
R/piecewiseDistribution.R       32       0  100.00%
R/piecewiseHazards.R            18       0  100.00%
R/survivalFunctions.R          111       0  100.00%
R/transitionParameters.R        42       0  100.00%
TOTAL                          743      77  89.64%

Diff against main

Filename              Stmts    Miss  Cover
------------------  -------  ------  --------
R/corPFSOS.R            +68     +68  +100.00%
R/estimateParams.R       +4      +4  -5.26%
TOTAL                   +72     +72  -9.62%

Results for commit: 2505e0d

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

Copy link
Contributor

github-actions bot commented Nov 22, 2023

Unit Tests Summary

    1 files    15 suites   2m 12s ⏱️
  80 tests   79 ✔️ 1 💤 0
184 runs  183 ✔️ 1 💤 0

Results for commit 6bd2761.

♻️ This comment has been updated with latest results.

R/corPFSOS.R Outdated Show resolved Hide resolved
@holgstr
Copy link
Contributor Author

holgstr commented Nov 22, 2023

@danielinteractive I have now documented all the functions. The main functions are:

  • corTrans(transition) computes the correlation for a given transition with parameters stored inside
  • corPFSOS(data, transition, boostrap = TRUE) estimates correlation for a given data set and distribution assumed via transition. I have also included a prototype bootstrap procedure inside, but this uses a for-loop. We would probably want to speed this up, so I thought about parallelization. Is there any good package/way of doing this you could recommend?

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 @holgstr , let's please separate out the bootstrap part in a new issue and PR. You can still add the corPFSOS function but still without the bootstrap here

R/corPFSOS.R Outdated Show resolved Hide resolved
R/corPFSOS.R Outdated Show resolved Hide resolved
R/corPFSOS.R Outdated Show resolved Hide resolved
R/corPFSOS.R Outdated Show resolved Hide resolved
R/corPFSOS.R Outdated Show resolved Hide resolved
R/corPFSOS.R Outdated Show resolved Hide resolved
R/corPFSOS.R Outdated Show resolved Hide resolved
R/corPFSOS.R Outdated Show resolved Hide resolved
R/corPFSOS.R Outdated
}
cor_res <- NULL
for (i in seq_len(bootstrap.n)) {
b_sample <- data[sample(nrow(data), replace = TRUE), ]
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this the way the survival bootstrap works? see Kaspar's reference

holgstr and others added 7 commits November 22, 2023 20:30
Co-authored-by: Daniel Sabanes Bove <[email protected]>
Signed-off-by: Holger Löwe <[email protected]>
Co-authored-by: Daniel Sabanes Bove <[email protected]>
Signed-off-by: Holger Löwe <[email protected]>
Co-authored-by: Daniel Sabanes Bove <[email protected]>
Signed-off-by: Holger Löwe <[email protected]>
Co-authored-by: Daniel Sabanes Bove <[email protected]>
Signed-off-by: Holger Löwe <[email protected]>
@holgstr holgstr merged commit c14d11f into main Nov 22, 2023
@holgstr holgstr deleted the corfunction branch November 22, 2023 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add function for PFS-OS correlation
2 participants