hesim-dev / hesim

Health economic simulation modeling and decision analysis
https://hesim-dev.github.io/hesim/
62 stars 15 forks source link

params_surv dist="fixed" #105

Closed caterinagregorio closed 1 year ago

caterinagregorio commented 1 year ago

Hello, I realized that the est in params_surv when dist="fixed" needs to be in the clock reset time even when the clock in the state is forward mode. Is this a bug or there is a specific reason for that?

Thank you!

dincerti commented 1 year ago

Hi @caterinagregorio what is the error you are getting?

caterinagregorio commented 1 year ago

Thank you for your quick reply! Actually, the function does not throw any error. However, looking at the results of the microsimulation, the subjects transition as expected only if the est is the time reset formulation.

dincerti commented 1 year ago

What about the transitions is not as expected?

caterinagregorio commented 1 year ago

Let's say that I define a transition from state 3 to 4 with the dist="fixed" and for example est=6, I expect that all subjects that are in state 3 at time 6 from the start of the microsimulation go to 4 at that time, if the microsimulation is clock-forward. However, at the moment, the function interprets 6 as the time since the entry in state 3, using a clock reset approach even if the state has been defined as clock-forward. Therefore, subjects go to state 4 after 6 years they have been in the state instead of at time 6 of the microsimulation.

I hope my explanation is clear, in case let me know.

dincerti commented 1 year ago

@caterinagregorio thanks for the report. I think it may be best to throw an error in cases where a "fixed" distribution is used in a clock-forward model. The problem is that a clock-forward model requires simulation from truncated distributions where the lower bound is model time. It would, for instance, not make sense if your fixed time was 6, but your model time was 9, since there would be 0 probability that the (deterministic) variable would be > 9 .

caterinagregorio commented 1 year ago

That's a good point and I agree that the fixed time should be in the reset time. However, I can think of cases for which it would be useful to use the fixed distribution for a transition (saying that the fixed time is for example 3 months after entering the state) even if the other non-deterministic transitions from the state are clock-forward. So, instead of throwing an error if the state is clock-forward, would it be possible to just state that the fixed distribution works only in reset time even tough the state is clock-forward?

I am using a lot your package and I personally find it amazing. I think it can be useful more many applications beyond health-economics so that is why I will probably write to you more in the future. I am also happy to discuss more on this point and give you a practical example of why I think it would be useful not to throw an error but just add more info on this in the help file, or a warning saying that the fixed distribution works differently than the other ones.

dincerti commented 1 year ago

First, its great that you are finding the package useful!

PR #106 might be a path forward. It will allow hazards for certain transitions to be "reset" and hazards at other transitions to be "forward". So you could let all transitions with fixed times be "reset" and the remaining be "forward". What do you think?

caterinagregorio commented 1 year ago

Yes, definitely! That looks great!