Write model run outputs #41

zsusswein · 2024-09-16T16:48:14Z

At long last, the output schema PR is ready to go. There are a few actions failing; these should be ignored (see #49). I'm sorry about the size here -- it's really much fewer than ~1200 lines of code, but still not awesome of me.

These are the functions and documentation to go from fitted EpiNow2 model to a directory of outputs ready to be uploaded to Azure Blob. I feel good about all the functionality and that the schema includes most of what we want (in particular both pre- and post-nowcast cases).

It would be particularly helpful to get review on:

Clarity and robustness of documentation. Does the readme make clear what is produced and where? I'd like to start pointing people to the readme when they have questions.
Output schema. I went with a hybrid format on task-grouped and output-grouped. I think it'll make it easier to both glob over files and do manual investigation, but open to feedback here. Does anyone foresee issues with extensibility if there's a new output type we want? Or total dealbreakers if/when plugging in a new type of model?
Missing outputs. I tried to be reasonably comprehensive for everything we currently use. It'd be good to get feedback if there's something important missing.

codecov · 2024-09-16T16:50:49Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Additional details and impacted files

📢 Thoughts on this report? Let us know!

R/write_output.R

DESCRIPTION

R/extract_diagnostics.R

natemcintosh

I think this looks great!

Had one or two questions, but overall love the high code quality, general readability, and the nice thick set of tests.

R/write_output.R

README.md

tests/testthat/test-write_output.R

athowes · 2024-10-02T13:35:07Z

Note on this that I don't know that there is an issue describing the plan for this PR (?) so in reviewing I am somewhat unsure what the goals are.

athowes

Part way through.

R/extract_diagnostics.R

kgostic

I'm halfway through. Will resume later

R/extract_diagnostics.R

R/write_output.R

athowes

Looks like good things happening here. Somewhat hard to say with how large it is

R/write_output.R

README.md

DESCRIPTION

R/write_output.R

kgostic

I still need to finish going through test-write-output.R, but I don't have any blocking concerns at this point!

R/extract_diagnostics.R

README.md

tests/testthat/helper-write_parameter_file.R

tests/testthat/test-extract_diagnostics.R

To fix weirdness with unicode parsing error from.....somewhere?

@natemcintosh

h/t @natemcintosh

Co-authored-by: Adam Howes <[email protected]>

for more information, see https://pre-commit.ci

Co-authored-by: Katie Gostic (she/her) <[email protected]>

Co-authored-by: Adam Howes <[email protected]>

Co-authored-by: Katie Gostic (she/her) <[email protected]>

zsusswein · 2024-10-08T12:54:07Z

The failing check is a known, unrelated issue. It's being handled in #60

@natemcintosh

* Ignore CI/CD stuff in Rbuildignore * Extract diagnostics from fitted model * Basic output schema * Use `.pre-commit.config.yaml` from main To fix weirdness with unicode parsing error from.....somewhere? * Update output schema * Bump NEWS * Bump NEWS * Expand on readme * Use setequal for column name checks h/t @natemcintosh * Apply suggestions from code review Co-authored-by: Adam Howes <[email protected]> * Update with Adam's review * Update R/write_output.R * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Update R/extract_diagnostics.R Co-authored-by: Katie Gostic (she/her) <[email protected]> * Add alert with dates for low case count diagnostic * Apply suggestions from code review Co-authored-by: Adam Howes <[email protected]> * Use new R-universe Stan repository * Update README.md Co-authored-by: Katie Gostic (she/her) <[email protected]> * Update README.md Co-authored-by: Katie Gostic (she/her) <[email protected]> * Condense dir creation * Expose quantiles for summarization * Save the description of the different EpiNow2 params * Add comment explaining why dates work * Clarify comment on EpiNow2 param outputs * Add `reports` to output --------- Co-authored-by: Adam Howes <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: Katie Gostic (she/her) <[email protected]>

zsusswein force-pushed the zs-write-output branch 3 times, most recently from b329b8f to 4681589 Compare September 16, 2024 20:42

natemcintosh reviewed Sep 27, 2024

View reviewed changes

R/write_output.R Outdated Show resolved Hide resolved

zsusswein mentioned this pull request Sep 27, 2024

Fix dependency caching in Docker image build #49

Closed

zsusswein force-pushed the zs-write-output branch 3 times, most recently from 02fec28 to cb634e4 Compare October 1, 2024 20:39

zsusswein commented Oct 1, 2024

View reviewed changes

DESCRIPTION Show resolved Hide resolved

zsusswein commented Oct 1, 2024

View reviewed changes

R/extract_diagnostics.R Show resolved Hide resolved

zsusswein marked this pull request as ready for review October 1, 2024 20:54

zsusswein requested review from natemcintosh, kgostic and athowes October 1, 2024 20:54

natemcintosh approved these changes Oct 1, 2024

View reviewed changes

athowes reviewed Oct 2, 2024

View reviewed changes

zsusswein mentioned this pull request Oct 2, 2024

Should diagnostic thresholds be exposed in the config #53

Open

kgostic reviewed Oct 2, 2024

View reviewed changes

zsusswein requested a review from athowes October 3, 2024 12:59

athowes approved these changes Oct 3, 2024

View reviewed changes

R/write_output.R Show resolved Hide resolved

R/write_output.R Show resolved Hide resolved

R/write_output.R Outdated Show resolved Hide resolved

R/write_output.R Show resolved Hide resolved

R/write_output.R Outdated Show resolved Hide resolved

README.md Outdated Show resolved Hide resolved

seabbs reviewed Oct 3, 2024

View reviewed changes

DESCRIPTION Outdated Show resolved Hide resolved

seabbs reviewed Oct 3, 2024

View reviewed changes

R/write_output.R Show resolved Hide resolved

seabbs reviewed Oct 3, 2024

View reviewed changes

R/write_output.R Show resolved Hide resolved

R/write_output.R Show resolved Hide resolved

kgostic approved these changes Oct 4, 2024

View reviewed changes

zsusswein added 4 commits October 7, 2024 14:41

Ignore CI/CD stuff in Rbuildignore

add75ca

Extract diagnostics from fitted model

ace1ddd

Basic output schema

ab5efc6

Use .pre-commit.config.yaml from main

1beddcf

To fix weirdness with unicode parsing error from.....somewhere?

zsusswein and others added 15 commits October 7, 2024 14:43

Update output schema

737e90a

Bump NEWS

c0e5593

Bump NEWS

0d45e73

Expand on readme

0247413

Use setequal for column name checks

cea2741

h/t @natemcintosh

Apply suggestions from code review

2f15a27

Co-authored-by: Adam Howes <[email protected]>

Update with Adam's review

6c75733

Update R/write_output.R

5ea8b3b

[pre-commit.ci] auto fixes from pre-commit.com hooks

5f9bc2c

for more information, see https://pre-commit.ci

Update R/extract_diagnostics.R

d32b490

Co-authored-by: Katie Gostic (she/her) <[email protected]>

Add alert with dates for low case count diagnostic

a67036b

Apply suggestions from code review

688a3a6

Co-authored-by: Adam Howes <[email protected]>

Use new R-universe Stan repository

5d91425

Update README.md

21339d8

Co-authored-by: Katie Gostic (she/her) <[email protected]>

Update README.md

b4f1089

Co-authored-by: Katie Gostic (she/her) <[email protected]>

zsusswein force-pushed the zs-write-output branch from 589324e to b4f1089 Compare October 7, 2024 19:00

zsusswein added 3 commits October 7, 2024 20:04

Condense dir creation

c80c8cf

Expose quantiles for summarization

b8c352e

Save the description of the different EpiNow2 params

5e92d5a

zsusswein force-pushed the zs-write-output branch from ee4dd2f to 5e92d5a Compare October 8, 2024 00:19

zsusswein added 3 commits October 7, 2024 20:59

Add comment explaining why dates work

f92ccb8

Clarify comment on EpiNow2 param outputs

1c69a0a

Add reports to output

4287bf1

zsusswein force-pushed the zs-write-output branch from 467a01b to 4287bf1 Compare October 8, 2024 01:33

Merge branch 'main' into zs-write-output

4b23e3c

zsusswein merged commit edb4fbd into main Oct 8, 2024
9 checks passed

zsusswein deleted the zs-write-output branch October 8, 2024 13:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Write model run outputs #41

Write model run outputs #41

zsusswein commented Sep 16, 2024 •

edited

Loading

codecov bot commented Sep 16, 2024 •

edited

Loading

natemcintosh left a comment

athowes commented Oct 2, 2024

athowes left a comment

kgostic left a comment

athowes left a comment

kgostic left a comment

zsusswein commented Oct 8, 2024

Write model run outputs #41

Write model run outputs #41

Conversation

zsusswein commented Sep 16, 2024 • edited Loading

codecov bot commented Sep 16, 2024 • edited Loading

Codecov Report

natemcintosh left a comment

Choose a reason for hiding this comment

athowes commented Oct 2, 2024

athowes left a comment

Choose a reason for hiding this comment

kgostic left a comment

Choose a reason for hiding this comment

athowes left a comment

Choose a reason for hiding this comment

kgostic left a comment

Choose a reason for hiding this comment

zsusswein commented Oct 8, 2024

zsusswein commented Sep 16, 2024 •

edited

Loading

codecov bot commented Sep 16, 2024 •

edited

Loading