CDCgov / Rt-without-renewal

https://cdcgov.github.io/Rt-without-renewal/
Apache License 2.0
21 stars 3 forks source link

Transformed data concept #161

Open seabbs opened 8 months ago

seabbs commented 8 months ago

Our current design goal is that the model specification for each module doesn't require information about the overall dimensions of the problem or observation data.

For many problems this limitation is fine but for some things that could be computed just once and used as data this leads to inefficiency. I had hoped that Turing would have an abstraction to deal with this but it doesn't appear to be the case.

A good example of this issue is LatentDelay which needs to construct a delay kernel. At the moment this is effectively data once the time index is known as we are assuming the latent delay is fixed. However, due to our current design this needs to sit within generate_observations and so is called many times (I believe)

Any potential solution to this issue needs to take into account future extensions. For this specific example these are: Uncertain delay summary parameters, time-varying delays, hierarchy in uncertain delays.

As workarounds, I have tried using Memoization.jl which results in long fit times (I think due to lack of support for reversediff with multi-threading but unsure). Another option is to use a generator function for the model macro but though I haven't tried it I think this solution doesn't work with submodels.

We could also introduce another layer of abstraction and have structs define new strusts using time and dimension index information when we combine the problem. This could be quite tricky to do potentially and would require more infrastructure but failing any other solution seems like it would definitely work with little loss of generality.

Before taking any concrete action we should also follow up to see if it is indeed the case that Turing isn't able to perform a simplification here or that if it currently isn't there isn't a feature planned in the near term that would do this.

SamuelBrand1 commented 8 months ago

We could also introduce another layer of abstraction and have structs define new trusts using time and dimension index information when we combine the problem. This could be quite tricky to do potentially and would require more infrastructure but failing any other solution seems like it would definitely work with little loss of generality.

I think this is the way we'll have to proceed.

SamuelBrand1 commented 8 months ago

Thinking about this a bit more.

  1. I think that time and dimension have to be considered separately, for the same meta reasons (essentially the Markov property in time but not necessarily other dimensions) they effectively are in (say) ODEProblem.
  2. Leaving time out of model components then using tspan from EpiProblem to fill this in is pretty straightforward.
  3. Leaving other dimensions out of model components is a bit too ambitious. My earlier thinking was that we could abstract this away by using rules to standardize broadcasting/reduce across different dimensions; but this could easily get more complicated than just defining the dimensions for
    • Latent processes
    • Infection processes
    • Observation processes
  4. Defaults above are obviously implied dimension 1
seabbs commented 8 months ago

Leaving other dimensions out of model components is a bit too ambitious. My earlier thinking was that we could abstract this away by using rules to standardize broadcasting/reduce across different dimensions; but this could easily get more complicated than just defining the dimensions for

I definitely agree this may be a bit ambitious and not possible. I like the idea of taking either an abstract model object or one that has dimensions and handling the checking of that when the problem is made (so that would let you handle complex cases or make setting domain knowledge based priors easier).

I would really love to try and stretch to just using relational rules if we can but acknowledge the answer may be no or that it may just be overly clunky. Having multiple data specifications in other packages has been a major pain point in my view and so is something that is worth trying to avoid if at all possible.

SamuelBrand1 commented 8 months ago

In #171 we define broadcasting rules... I assume this aims at this issue too?

seabbs commented 8 months ago

It aims at some of the relationship aspects yes but not for other things which are data dependent (like pmf pre-calc)