Add a day-of-week effect to simulated dataset #91
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAll modified and coverable lines are covered by tests ✅ Additional details and impacted files📢 Thoughts on this report? Let us know! |
kgostic
approved these changes
Nov 29, 2024
|
Hm yeah I'll take another pass at the docs. For the simulated data, we've been entirely ignoring infection to case observation delays for now. See, for example, the predict method in the vignette: https://cdcgov.github.io/cfa-gam-rt/articles/RtGam.html Although not explored vs simulated data, we currently implement just a mean-shift as a correction. I have it on the roadmap to explore that more vs simulated data in #42 and we're discussing potential different approaches for the model in #88. |
|
[like] Gostic, Katelyn (CDC/OD/ORR/CFA) reacted to your message:
…________________________________
From: Zachary Susswein ***@***.***>
Sent: Friday, November 29, 2024 8:41:44 PM
To: CDCgov/cfa-gam-rt ***@***.***>
Cc: Gostic, Katelyn (CDC/OD/ORR/CFA) ***@***.***>; Review requested ***@***.***>
Subject: Re: [CDCgov/cfa-gam-rt] Add a day-of-week effect to simulated dataset (PR #91)
CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
________________________________
Hm yeah I'll take another pass at the docs.
For the simulated data, we've been entirely ignoring infection to case observation delays for now. See, for example, the predict method in the vignette: https://cdcgov.github.io/cfa-gam-rt/articles/RtGam.html
Although not explored vs simulated data, we currently implement just a mean-shift as a correction. I have it on the roadmap to explore that more vs simulated data in #42<#42> and we're discussing potential different approaches for the model in #88<#88>.
—
Reply to this email directly, view it on GitHub<#91 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AD6Y4ODQGMHFEWE2SAKRNTL2DDGQRAVCNFSM6AAAAABSXPUFHCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKMBYGY2DOMBUGY>.
You are receiving this because your review was requested.Message ID: ***@***.***>
|
The current timeseries doesn't have a day of week effect, which makes it hard to test how the day of week effect implementation is performing. This modifies the simulated timeseries to add a large-ish day of week reporting effect. In implementing this change, I converted the timeseries over to using a `data-raw` script for reproducibility. I moved the original Gostic "Practical considerations" dataset into an RDS object in `data-raw`. This change makes the dataset tracked and available for re-generation of the package dataset, but doesn't expose it to or ship it with the package. It's not 100% reproducible because I don't implement the git clone of the "Practical considerations" repo, but I think it gets us a fair but further than before. I also clean up the package-provided `stochastic_sir_rt` datset. I rename the `obs_incidence` col to `obs_cases`, add a `obs_cases_no_dow` for testing, and rename `date` to `reference_date` in anticipation of a future `report_date`. Closes #90
Was silently producing and broadcasting day of week vector of length 13 out to 299. This bug was leading to a phasing of the day-of-week effect and making it impossible to fit a meaningful effect.
zsusswein
force-pushed
the
add-day-of-week-to-sim-data
branch
from
63bde98 to
064b4e5
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The current timeseries doesn't have a day of week effect, which makes it hard to test how the day of week effect implementation is performing.
This modifies the simulated timeseries to add a large-ish day of week reporting effect.
In implementing this change, I converted the timeseries over to using a
data-rawscript for reproducibility.I moved the original Gostic "Practical considerations" dataset into an RDS object in
data-raw.This change makes the dataset tracked and available for re-generation of the package dataset, but doesn't expose it to or ship it with the package.
It's not 100% reproducible because I don't implement the git clone of the "Practical considerations" repo, but I think it gets us a fair but further than before.
I also clean up the package-provided
stochastic_sir_rtdatset.I rename the
obs_incidencecol toobs_cases, add aobs_cases_no_dowfor testing, and renamedatetoreference_datein anticipation of a futurereport_date.Note that this change causes the non-day-of-week model performance to decrease in the vignette.
I'm expecting this change will be temporary until we can implement #89, which this PR unblocks.
Closes #90