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

Handling GIs #51

Open
zsusswein opened this issue Oct 1, 2024 · 2 comments
Open

Handling GIs #51

zsusswein opened this issue Oct 1, 2024 · 2 comments
Labels
bug Something isn't working v0.1.0

Comments

@zsusswein
Copy link
Collaborator

zsusswein commented Oct 1, 2024

In light of epiforecasts/EpiNow2#806 and epiforecasts/EpiNow2#799 (comment), we should probably implement our own check or handling for GIs with density on day 0. I'd think that would go here:

format_generation_interval <- function(pmf) {
if (
rlang::is_na(pmf) || rlang::is_null(pmf)
) {
cli::cli_abort("No generation time PMF specified but is required",
class = "Missing_GI"
)
}

The correct way to handle this is a bit subtle because there's a few conflicting things going on:

  1. We'll want our own check because we're running an older version of EpiNow2 (at least until Manage version bump of EpiNow2 #9)
  2. We're updating the formatting in some way in https://github.com/cdcent/cfa-parameter-estimates/issues/67, so we'll want this to respect whatever that formatting is. It's a bit tricky here if that doesn't include a left-hand-side zero-pad because we'll have to guess whether to zero-pad or not. We wont be able to know whether an input is wrong except by convention.
  3. We'll want the ability to backtest on the old GI to compare to old prod estimates to new model setups. I don't quite know how we do that -- is it to save the biased GI in a readable-for-EpiNow2 format? Something else?

And I'm not sure how to handle this so (2) and (3) are both achievable. It's not hard on the face of it because we could make (1) a warning, but epiforecasts/EpiNow2#806 makes up-to-date EpiNow2's check an error. So we'd need to reformat our old estimate to be both different and still wrong if we want to be able to do a true backtest once we bump the version.

I think the answer is probably we throw a warning and zero-pad the left-hand side of the PMF here if the PMF is not already zero-padded. And we overwrite the old PMF to be knowingly biased by zero-padding the current prod estimate so we can reproduce old behavior. Thoughts @athowes @kgostic?

@zsusswein zsusswein added bug Something isn't working v0.1.0 labels Oct 1, 2024
@zsusswein
Copy link
Collaborator Author

ping @athowes. I think final resolution here is dependent on https://github.com/cdcent/cfa-parameter-estimates/issues/67 but that's marked as V2. Do you have guidance on how I should handle here?

@athowes
Copy link
Collaborator

athowes commented Oct 15, 2024

ping @athowes. I think final resolution here is dependent on https://github.com/cdcent/cfa-parameter-estimates/issues/67 but that's marked as V2. Do you have guidance on how I should handle here?

For the time being until V2 we are going to do zero PMF at zero and include left pad with zero as output of cfa-parameter-estimates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v0.1.0
Projects
None yet
Development

No branches or pull requests

2 participants