hesim icon indicating copy to clipboard operation
hesim copied to clipboard

Transition-specific costs and cCTSTMs

Open mclements opened this issue 1 year ago • 6 comments

Devin,

I have been working on a few changes to the hesim package.

For this pull request, I have implemented transition-specific costs, which currently only has an application for iCTSTMs.

Note that this folds in the unresolved inline extension from pull request #122.

As a signpost: I am preparing a presentation for the International Society of Clinical Biostatistics for later this month that implements cCTSTMs using ordinary differential equations. I have a gist and will prepare a pull request. I have a number of questions to ask you about that pull request:).

Sincerely, Mark.

mclements avatar Jul 13 '24 20:07 mclements

I have added some changes to address the recent R check notes and warnings:

  • I incremented the version number
  • To remove a note about R6 not having any imports, I added @importFrom R6 R6Class in ctstm.R -- although this is not strictly necessary.
  • Most of the other notes had to do with documentation external links not having packages.

mclements avatar Jul 17 '24 08:07 mclements

Devin: pkgdown is complaining about the documentation for CohortCtstm in ctstm.R. I can't see the error - would you be able to help here, please?

mclements avatar Jul 30 '24 16:07 mclements

Some commentary on the implementation:

  • I was not able to find another package on CRAN that allows LinkingTo: for ordinary differential equations in C++. I investigated adapting the rehuel library for CRAN, but found that the package is not well maintained and some of the tests were failing. I have kept the BH dependency. For further discussion?
  • As discussed at #126, I have assumed that the point mass distribution is Markov. I am not happy with the implementation for the point masses: I needed dynamic_cast to determine (a) if a transmod was an mstate_list, and then (b) whether an element of mstate_list->survmod_ is a point mass (after adding an hesim::stats::distribution* get_dist(int trans) method to mstate_list). The code would be cleaner and clearer if we did not allow for point masses.
  • For the standard hip replacement vignette, I have assumed that the fifth transition is exponential. This is strictly Markov (clock-forward), whereas the individual-based vignette is a mix of clock-forward and clock-reset transitions.
  • I have some test code in preparation, but not completed.

mclements avatar Jul 30 '24 18:07 mclements

I have now fixed the pkgdown issue.

mclements avatar Jul 31 '24 10:07 mclements

@dincerti, I have now added some tests for this. This is now ready for review -- although admittedly this is a larger pull request than initially anticipated.

mclements avatar Aug 01 '24 12:08 mclements

Devin:

  • I have now added an experimental cCTSTM implementation for semi-Markov models.
  • The implementation is similar to the Markov case, except that one needs to discretise for duration and keep track of both time in state and time in study.
  • However, the implementation is slow and needs +to be optimised.
  • ~~TODO: I need to address some check warnings.~~

mclements avatar Aug 08 '24 16:08 mclements