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

Middle task tutorials #58

Merged
merged 82 commits into from
Feb 2, 2024
Merged

Middle task tutorials #58

merged 82 commits into from
Feb 2, 2024

Conversation

amanda-minter
Copy link
Collaborator

Tutorials for middle tasks 1 (Quantifying transmissibility) and 2 (Create a short-term forecast )

@amanda-minter amanda-minter requested a review from sbfnk November 13, 2023 10:23
Copy link

github-actions bot commented Nov 13, 2023

Thank you!

Thank you for your pull request 😃

🤖 This automated message can help you check the rendered files in your submission for clarity. If you have any questions, please feel free to open an issue in {sandpaper}.

If you have files that automatically render output (e.g. R Markdown), then you should check for the following:

  • 🎯 correct output
  • 🖼️ correct figures
  • ❓ new warnings
  • ‼️ new errors

Rendered Changes

🔍 Inspect the changes: https://github.com/epiverse-trace/tutorials/compare/md-outputs..md-outputs-PR-58

The following changes were observed in the rendered markdown documents:

 config.yaml (gone)                                 |   82 -
 create-forecast.md (new)                           |  375 ++++
 ...-forecast-rendered-unnamed-chunk-11-1.png (new) |  Bin 0 -> 8461 bytes
 ...e-forecast-rendered-unnamed-chunk-3-1.png (new) |  Bin 0 -> 50631 bytes
 ...e-forecast-rendered-unnamed-chunk-9-1.png (new) |  Bin 0 -> 10349 bytes
 fig/model-choices-rendered-unnamed-chunk-3-1.png   |  Bin 18925 -> 13763 bytes
 fig/model-choices-rendered-unnamed-chunk-4-1.png   |  Bin 8795 -> 9155 bytes
 ...ssibility-rendered-unnamed-chunk-15-1.png (new) |  Bin 0 -> 31313 bytes
 ...ssibility-rendered-unnamed-chunk-16-1.png (new) |  Bin 0 -> 30715 bytes
 ...ssibility-rendered-unnamed-chunk-19-1.png (new) |  Bin 0 -> 84860 bytes
 ...issibility-rendered-unnamed-chunk-4-1.png (new) |  Bin 0 -> 5925 bytes
 ...issibility-rendered-unnamed-chunk-7-1.png (new) |  Bin 0 -> 10256 bytes
 fig/simulating-transmission-rendered-plot-1.png    |  Bin 16813 -> 17728 bytes
 md5sum.txt                                         |   30 +-
 quantify-transmissibility.md (new)                 |  531 ++++++
 reference.md                                       |   16 +-
 renv.lock (gone)                                   | 1846 --------------------
 17 files changed, 933 insertions(+), 1947 deletions(-)
What does this mean?

If you have source files that require output and figures to be generated (e.g. R Markdown), then it is important to make sure the generated figures and output are reproducible.

This output provides a way for you to inspect the output in a diff-friendly manner so that it's easy to see the changes that occur due to new software versions or randomisation.

⏱️ Updated at 2024-02-02 20:02:03 +0000

Copy link
Contributor

@sbfnk sbfnk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice tutorials! I've left some comments in specific places.

Some of the syntax is currently undergoing some breaking changes with a target of version 2.0.0 - we'll have to decide whether to publish these as 1.4.0 specific tutorials or wait until the new version and then make any necessary changes.

episodes/quantify-transmissibility.Rmd Outdated Show resolved Hide resolved
episodes/quantify-transmissibility.Rmd Outdated Show resolved Hide resolved
episodes/quantify-transmissibility.Rmd Outdated Show resolved Hide resolved
episodes/quantify-transmissibility.Rmd Outdated Show resolved Hide resolved
episodes/quantify-transmissibility.Rmd Outdated Show resolved Hide resolved
episodes/create-forecast.Rmd Outdated Show resolved Hide resolved
episodes/create-forecast.Rmd Outdated Show resolved Hide resolved
episodes/create-forecast.Rmd Outdated Show resolved Hide resolved
episodes/create-forecast.Rmd Outdated Show resolved Hide resolved
episodes/create-forecast.Rmd Show resolved Hide resolved
episodes/quantify-transmissibility.Rmd Outdated Show resolved Hide resolved
episodes/quantify-transmissibility.Rmd Outdated Show resolved Hide resolved
episodes/create-forecast.Rmd Outdated Show resolved Hide resolved
@amanda-minter
Copy link
Collaborator Author

@sbfnk thank you for the previous comments, I've added two further sections to the second tutorial "Create a short-term forecast"

  1. Forecast secondary observations : illustrates using esitimate_secondary() and forecast_secondary(). However, when plotting the forecast, there is an error about removing NAs - I think this arises because there are no secondary observations in cases_to_forecast - which I thought would be OK? There may be an error with my implementation

  2. Challenge : Ebola outbreak analysis : this is a challenge for learners to complete as part of an Ebola challenge that will be present across all tutorials

If you have time to look at these new sections before Christmas I can any make necessary changes, I will leave these changes as a draft PR regardless so that the tutorials can be updated for EpiNow2 2.0.0 before being made public

@avallecam avallecam marked this pull request as ready for review January 31, 2024 20:24
@avallecam
Copy link
Member

FYI yesterday I change this to ready to review since all workbench gh-actions passed and md-output-PR-58 was generated with pre-rendered readable .md files of each episode in:

Copy link
Contributor

@sbfnk sbfnk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great - just a few minor text changes.

Only question that remains is why we don't run the code in the solution - not doing this seems to open ourselves for issues if there are any bugs in the code.

episodes/create-forecast.Rmd Outdated Show resolved Hide resolved
episodes/create-forecast.Rmd Outdated Show resolved Hide resolved
episodes/create-forecast.Rmd Outdated Show resolved Hide resolved
episodes/create-forecast.Rmd Outdated Show resolved Hide resolved
episodes/create-forecast.Rmd Outdated Show resolved Hide resolved
episodes/create-forecast.Rmd Outdated Show resolved Hide resolved
@avallecam
Copy link
Member

why we don't run the code in the solution - not doing this seems to open ourselves for issues if there are any bugs in the code.

Good question. Days ago, I had issues making that section pass the workbench gh-actions, although running correctly locally. But I didn't consider that potential vulnerability. I'll open it again and look further if issues appear.

@sbfnk
Copy link
Contributor

sbfnk commented Feb 2, 2024

why we don't run the code in the solution - not doing this seems to open ourselves for issues if there are any bugs in the code.

Good question. Days ago, I had issues making that section pass the workbench gh-actions, although running correctly locally. But I didn't consider that potential vulnerability. I'll open it again and look further if issues appear.

Great - yes, I noticed some issues with version from a weeks ago that I managed to fix locally (but things had moved on since then). Feel free to ping me for a second pair of eyes if you run into any issues.

avallecam and others added 3 commits February 2, 2024 12:46
Co-authored-by: Sebastian Funk <[email protected]>
Co-authored-by: Sebastian Funk <[email protected]>
@avallecam
Copy link
Member

@sbfnk changes incorporated and all checks passed. Thank you for your feedback. Re-requesting review for approval and proceed.

Copy link
Contributor

@sbfnk sbfnk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super!

episodes/create-forecast.Rmd Outdated Show resolved Hide resolved
github-actions bot pushed a commit that referenced this pull request Feb 2, 2024
@avallecam avallecam merged commit b7670c5 into main Feb 2, 2024
4 checks passed
@avallecam avallecam deleted the middle_task_1 branch February 2, 2024 20:06
avallecam added a commit that referenced this pull request Feb 2, 2024
amanda-minter pushed a commit to amanda-minter/tutorials-late that referenced this pull request Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants