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

add episodes on introduction and read delays #104

Merged
merged 130 commits into from
Feb 6, 2024
Merged

add episodes on introduction and read delays #104

merged 130 commits into from
Feb 6, 2024

Conversation

avallecam
Copy link
Member

@avallecam avallecam commented Dec 23, 2023

Fix #88
Fix #67

For context: I wrote two episodes, "introduction" and "read delays", as starting episodes around {EpiNow2} and {epiparameter} as a motivational example:

  • {epiparameters} fit as an authentic task: quick to master and immediately useful once mastered.
  • quantifying transmissibility is a familiar task to learners that immediately benefits from {epiparameter} as input (reference)
  • this joint presentation could also contribute to the vision of connecting packages to build a pipeline.
  • The {epiparameter} + {EpiNow2} interoperability code was based on recent code updates in Update {epiparameter} usage episoap#100

I'll appreciate your specific feedback providing a clear next step to remove, change or add content to this PR.

Some questions to guide this are:

  • Should we cut the episode on "read delays" in two or three? Where? (possibly between continuous distribution and Adjusting for reporting delays)
  • Should we briefly warn about biases of the {EpiNow2} demo in "introduction" episode and redirect to "quantify-transmissibility" (#58) after "read-delays"?
  • Should we add and adapt the {epiparameter} concept map as in #103?

For review, you can:

Copy link

github-actions bot commented Dec 23, 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-104

The following changes were observed in the rendered markdown documents:

 config.yaml (gone)                                 |   84 -
 create-forecast.md                                 |   45 +-
 ...create-forecast-rendered-unnamed-chunk-11-1.png |  Bin 8461 -> 8320 bytes
 fig/create-forecast-rendered-unnamed-chunk-3-1.png |  Bin 49982 -> 50619 bytes
 fig/create-forecast-rendered-unnamed-chunk-9-1.png |  Bin 10363 -> 10338 bytes
 fig/disease-reporting.jpg (new)                    |  Bin 0 -> 44277 bytes
 fig/fig5a-normaldistribution.png (new)             |  Bin 0 -> 111729 bytes
 fig/incubation-period-serial-interval.jpg (new)    |  Bin 0 -> 231856 bytes
 fig/infectiousness-covid19.jpg (new)               |  Bin 0 -> 720176 bytes
 ...troduction-rendered-unnamed-chunk-5-1.png (new) |  Bin 0 -> 51068 bytes
 fig/model-choices-rendered-unnamed-chunk-3-1.png   |  Bin 13458 -> 13483 bytes
 fig/model-choices-rendered-unnamed-chunk-4-1.png   |  Bin 9206 -> 9063 bytes
 fig/pkgs-hexlogos.png (new)                        |  Bin 0 -> 590622 bytes
 ...ransmissibility-rendered-unnamed-chunk-15-1.png |  Bin 31592 -> 30774 bytes
 ...ransmissibility-rendered-unnamed-chunk-16-1.png |  Bin 30628 -> 30666 bytes
 ...ransmissibility-rendered-unnamed-chunk-19-1.png |  Bin 84498 -> 84456 bytes
 ...ad-delays-rendered-unnamed-chunk-20-1.png (new) |  Bin 0 -> 11595 bytes
 fig/reproduction-generation-time.png (new)         |  Bin 0 -> 725371 bytes
 fig/reproduction-number-pre-symptomatic.png (new)  |  Bin 0 -> 721253 bytes
 fig/rt-adjusting-delays.png (new)                  |  Bin 0 -> 34455 bytes
 fig/seria-interval-fitted-distributions.jpg (new)  |  Bin 0 -> 200736 bytes
 fig/serial-interval-covid-sars.jpg (new)           |  Bin 0 -> 122860 bytes
 fig/serial-interval-observed.jpeg (new)            |  Bin 0 -> 319571 bytes
 fig/serial-interval-pairs.jpg (new)                |  Bin 0 -> 923818 bytes
 fig/simulating-transmission-rendered-plot-1.png    |  Bin 16469 -> 16390 bytes
 fig/time-periods.jpg (new)                         |  Bin 0 -> 300672 bytes
 introduction.md (new)                              |  210 ++
 md5sum.txt                                         |   34 +-
 quantify-transmissibility.md                       |  116 +-
 read-delays.md (new)                               | 1675 +++++++++++++++
 reference.md                                       |   58 +-
 renv.lock (gone)                                   | 2262 --------------------
 32 files changed, 2034 insertions(+), 2450 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-06 13:41:38 +0000

@joshwlambert
Copy link
Member

The error I get when trying to run sandpaper::manage_deps() is:

clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [Matrix.so] Error 1
ERROR: compilation failed for packageMatrix

@joshwlambert
Copy link
Member

I have {Matrix} installed:

> library(Matrix)
> packageVersion("Matrix")
[1] ‘1.5.3

I've updated to the latest version, which installs correctly:

> install.packages("Matrix")
trying URL 'https://cran.rstudio.com/bin/macosx/big-sur-arm64/contrib/4.2/Matrix_1.6-5.tgz'
Content type 'application/x-gzip' length 5348209 bytes (5.1 MB)
==================================================
downloaded 5.1 MB


The downloaded binary packages are in
	/var/folders/c2/bn46nm4j7mj2_zyktygmfnk40000gp/T//RtmpAofVU8/downloaded_packages
> library(Matrix)
> packageVersion("Matrix")
[1] ‘1.6.5

These are the versions of the carpentries packages I have installed:

library(sandpaper)
library(varnish)
library(pegboard)
library(tinkr)
sessionInfo()
#> R version 4.2.3 (2023-03-15)
#> Platform: aarch64-apple-darwin20 (64-bit)
#> Running under: macOS 14.1.2
#> 
#> Matrix products: default
#> BLAS:   /Library/Frameworks/R.framework/Versions/4.2-arm64/Resources/lib/libRblas.0.dylib
#> LAPACK: /Library/Frameworks/R.framework/Versions/4.2-arm64/Resources/lib/libRlapack.dylib
#> 
#> locale:
#> [1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8
#> 
#> attached base packages:
#> [1] stats     graphics  grDevices utils     datasets  methods   base     
#> 
#> other attached packages:
#> [1] tinkr_0.2.0           pegboard_0.7.1.9000   varnish_0.3.3.9000   
#> [4] sandpaper_0.14.1.9000
#> 
#> loaded via a namespace (and not attached):
#>  [1] rstudioapi_0.14   xml2_1.3.3        knitr_1.44        magrittr_2.0.3   
#>  [5] R.cache_0.16.0    R6_2.5.1          rlang_1.1.2       fastmap_1.1.1    
#>  [9] styler_1.10.0     tools_4.2.3       xfun_0.40         R.oo_1.25.0      
#> [13] cli_3.6.2         withr_2.5.2       htmltools_0.5.6   yaml_2.3.7       
#> [17] digest_0.6.33     assertthat_0.2.1  lifecycle_1.0.4   processx_3.8.2   
#> [21] purrr_1.0.2       callr_3.7.3       ps_1.7.5          vctrs_0.6.5      
#> [25] R.utils_2.12.2    fs_1.6.3          glue_1.6.2        evaluate_0.23    
#> [29] rmarkdown_2.25    reprex_2.0.2      compiler_4.2.3    R.methodsS3_1.8.2

Created on 2024-01-15 with reprex v2.0.2

@joshwlambert
Copy link
Member

I've updated the carpentries packages to see if that was the issue:

library(sandpaper)
library(varnish)
library(pegboard)
library(tinkr)
sessionInfo()
#> R version 4.2.3 (2023-03-15)
#> Platform: aarch64-apple-darwin20 (64-bit)
#> Running under: macOS 14.1.2
#> 
#> Matrix products: default
#> BLAS:   /Library/Frameworks/R.framework/Versions/4.2-arm64/Resources/lib/libRblas.0.dylib
#> LAPACK: /Library/Frameworks/R.framework/Versions/4.2-arm64/Resources/lib/libRlapack.dylib
#> 
#> locale:
#> [1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8
#> 
#> attached base packages:
#> [1] stats     graphics  grDevices utils     datasets  methods   base     
#> 
#> other attached packages:
#> [1] tinkr_0.2.0      pegboard_0.7.3   varnish_1.0.1    sandpaper_0.16.2
#> 
#> loaded via a namespace (and not attached):
#>  [1] rstudioapi_0.14   xml2_1.3.3        knitr_1.44        magrittr_2.0.3   
#>  [5] R.cache_0.16.0    R6_2.5.1          rlang_1.1.2       fastmap_1.1.1    
#>  [9] styler_1.10.0     tools_4.2.3       xfun_0.40         R.oo_1.25.0      
#> [13] cli_3.6.2         withr_2.5.2       htmltools_0.5.6   yaml_2.3.7       
#> [17] digest_0.6.33     assertthat_0.2.1  lifecycle_1.0.4   processx_3.8.2   
#> [21] purrr_1.0.2       callr_3.7.3       ps_1.7.5          vctrs_0.6.5      
#> [25] R.utils_2.12.2    fs_1.6.3          glue_1.6.2        evaluate_0.23    
#> [29] rmarkdown_2.25    reprex_2.0.2      compiler_4.2.3    R.methodsS3_1.8.2

Created on 2024-01-15 with reprex v2.0.2

However, the same issue persists:

clang: error: linker command failed with exit code 1 (use -v to see invocation)
make: *** [Matrix.so] Error 1
ERROR: compilation failed for packageMatrix

This could be due to compiling the package on apple silicon. As the package version required by {tutorials} is v1.6-4, and I'm unsure where {sandpaper}/{renv} is installing from, but if a binary is not available it may cause issues with code requiring compilation, especially on Macs.

@Bisaloo
Copy link
Member

Bisaloo commented Jan 15, 2024

Do you have the full install log from the failing Matrix installation please?

@joshwlambert
Copy link
Member

Do you have the full install log from the failing Matrix installation please?

What is the best way to share this? Paste a reprex of sandpaper::manage_deps()?

@Bisaloo
Copy link
Member

Bisaloo commented Jan 15, 2024

I suspect you're missing a fortran compiler / R doesn't find it. Can you try installing https://github.com/coatless-mac/macrtools first please? Carmen has had success with this in the past.

@joshwlambert
Copy link
Member

I've used {macrtools} to install gfortran. Running sandpaper::manage_deps() now gets further (i.e. installs more packages including {Matrix}) but now fails when trying to install {ragg}.

The head and tail of the error message are:

Error: 
! in callr subprocess.
Caused by error: 
! Error installing package 'ragg':
================================
** testing if installed package can be loaded from temporary location
Error: package or namespace load failed for ‘ragg’ in dyn.load(file, DLLpath = DLLpath, ...):
 unable to load shared object '/Users/lshjl15/Documents/LSHTM_code/tutorials/renv/profiles/lesson-requirements/renv/library/R-4.2/aarch64-apple-darwin20/.renv/1/00LOCK-ragg/00new/ragg/libs/ragg.so':
  dlopen(/Users/lshjl15/Documents/LSHTM_code/tutorials/renv/profiles/lesson-requirements/renv/library/R-4.2/aarch64-apple-darwin20/.renv/1/00LOCK-ragg/00new/ragg/libs/ragg.so, 0x0006): symbol not found in flat namespace '_WebPConfigInitInternal'
Error: loading failed
Execution halted
ERROR: loading failed
* removing ‘/Users/lshjl15/Documents/LSHTM_code/tutorials/renv/profiles/lesson-requirements/renv/library/R-4.2/aarch64-apple-darwin20/.renv/1/ragg’
install of package 'ragg' failed [error code 1]
ℹ See `$stdout` and `$stderr` for standard output and error.

@Bisaloo
Copy link
Member

Bisaloo commented Jan 22, 2024

It looks like you're probably missing system dependencies for ragg. Can you try running:

pak::sysreqs_check_installed("ragg")

please?

@joshwlambert
Copy link
Member

pak::sysreqs_check_installed("ragg")
#> Error: ! error in pak subprocess
#> Caused by error in `sysreqs2_command(sysreqs_platform, "query")`:
#> ! Unknown OS. Don't know how to query or install system packages for
#> aarch64-apple-darwin20.

Created on 2024-01-22 with reprex v2.0.2

@joshwlambert
Copy link
Member

r-lib/pak#563

@Bisaloo
Copy link
Member

Bisaloo commented Jan 22, 2024

That's unfortunate. Could you then look manually into ragg system dependencies and search if they are installed / how to install them on macOS?

@joshwlambert
Copy link
Member

4 system dependencies for {ragg}: https://cran.r-project.org/web/packages/ragg/index.html

I seem to have all system dependencies installed. Some possible problems:

  • freetype2 is specified as the system dependency but brew only has freetype (https://formulae.brew.sh/formula/freetype) so this might be an issue recognising the right program
  • libjpeg might not be recognised by the compiler toolchain in R due for reasons outlined by brew info libjpeg
brew info libjpeg
==> jpeg: stable 9f (bottled) [keg-only]
Image manipulation library
https://www.ijg.org/
/opt/homebrew/Cellar/jpeg/9f (21 files, 900.9KB)
  Poured from bottle using the formulae.brew.sh API on 2024-01-22 at 11:48:13
From: https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/j/jpeg.rb
License: IJG
==> Caveats
jpeg is keg-only, which means it was not symlinked into /opt/homebrew,
because it conflicts with `jpeg-turbo`.

If you need to have jpeg first in your PATH, run:
  echo 'export PATH="/opt/homebrew/opt/jpeg/bin:$PATH"' >> ~/.zshrc

For compilers to find jpeg you may need to set:
  export LDFLAGS="-L/opt/homebrew/opt/jpeg/lib"
  export CPPFLAGS="-I/opt/homebrew/opt/jpeg/include"
==> Analytics
install: 25,267 (30 days), 41,108 (90 days), 97,414 (365 days)
install-on-request: 8,179 (30 days), 16,181 (90 days), 40,597 (365 days)
build-error: 0 (30 days)

@Bisaloo
Copy link
Member

Bisaloo commented Jan 23, 2024

I'm not really sure how to help. This is very specific to macOS and my knowledge is limited, as are my options to test potential fixes.

If I was in this situation, my gut feeling would be to try and install webp related libraries that you may(???) be missing. No idea if that will help but probably worth a try.

Otherwise:

  • we can clarify that rendering the materials is not expected from reviewers and improve the online preview mechanism.
  • we can set up a Docker container for this repo so we at least all have the same setup, which makes it easier to debug potential issues.

@sbfnk
Copy link
Contributor

sbfnk commented Jan 23, 2024

symbol not found in flat namespace '_WebPConfigInitInternal'

I presume you've got webp installed?
brew info webp

@joshwlambert
Copy link
Member

Reinstalled webp:

brew info webp
==> webp: stable 1.3.2 (bottled), HEAD
Image format providing lossless and lossy compression for web images
https://developers.google.com/speed/webp/
/opt/homebrew/Cellar/webp/1.3.2 (63 files, 2.3MB) *
  Poured from bottle using the formulae.brew.sh API on 2024-01-23 at 18:25:21
From: https://github.com/Homebrew/homebrew-core/blob/HEAD/Formula/w/webp.rb
License: BSD-3-Clause
==> Dependencies
Build: cmake ✘
Required: giflib ✔, jpeg-turbo ✘, libpng ✔, libtiff ✔
==> Options
--HEAD
	Install HEAD version
==> Analytics
install: 75,767 (30 days), 250,667 (90 days), 1,036,950 (365 days)
install-on-request: 18,342 (30 days), 69,549 (90 days), 336,029 (365 days)
build-error: 65 (30 days)

However, the {ragg} install still fails:

** testing if installed package can be loaded from temporary location
Error: package or namespace load failed for ‘ragg’ in dyn.load(file, DLLpath = DLLpath, ...):
 unable to load shared object '/Users/lshjl15/Documents/LSHTM_code/tutorials/renv/profiles/lesson-requirements/renv/library/R-4.2/aarch64-apple-darwin20/.renv/1/00LOCK-ragg/00new/ragg/libs/ragg.so':
  dlopen(/Users/lshjl15/Documents/LSHTM_code/tutorials/renv/profiles/lesson-requirements/renv/library/R-4.2/aarch64-apple-darwin20/.renv/1/00LOCK-ragg/00new/ragg/libs/ragg.so, 0x0006): symbol not found in flat namespace '_WebPConfigInitInternal'
Error: loading failed
Execution halted
ERROR: loading failed
* removing ‘/Users/lshjl15/Documents/LSHTM_code/tutorials/renv/profiles/lesson-requirements/renv/library/R-4.2/aarch64-apple-darwin20/.renv/1/ragg’
install of package 'ragg' failed [error code 1]
ℹ See `$stdout` and `$stderr` for standard output and error.

@chartgerink
Copy link
Member

For what it's worth - I just went through the install steps on a macOS (Intel-based) device, with no major issues (I only had to manually install pak::pak("epiverse-trace/epidemics")). Might it be worth to open up a separate issue to track this problem?

@avallecam
Copy link
Member Author

avallecam commented Jan 24, 2024

(I only had to manually install pak::pak("epiverse-trace/epidemics")). Might it be worth to open up a separate issue to track this problem?

I think yes. That's a step we can add to the guidelines. I also noticed that for a successful local run of sandpaper::build_lesson(), we need to install any new package used in the lesson manually. This happened to me while working in epimodelac, where, for a moment, I did not have {vaccineff} and {serofoi} locally.

@joshwlambert
Copy link
Member

Might it be worth to open up a separate issue to track this problem?

I agree, these problems are holding up this PR. I will unassign myself as a reviewer of this PR and will try to resolve these issues locally and with the team.

github-actions bot pushed a commit that referenced this pull request Feb 5, 2024
Copy link
Member

@Bisaloo Bisaloo left a comment

Choose a reason for hiding this comment

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

Two comments that we should address after the split:

  • there is a cache/ folder in the output that probably shouldn't be here
  • most of the image should be converted to .png. .jpg is good for photographs but not for line art or scientific figures.

@avallecam
Copy link
Member Author

avallecam commented Feb 6, 2024

Thank you for your feedback.

  • there is a cache/ folder in the output that probably shouldn't be here

Could it be because I used cache=TRUE in one epinow2 chunk in the intro file? I'll remove it to see how the md-outputs-PR-104 reacts to this change.

  • most of the image should be converted to .png. .jpg is good for photographs but not for line art or scientific figures.

Do you have a recommended method to do this conversion? I'll see if I can do this locally.

@avallecam
Copy link
Member Author

  • most of the image should be converted to .png. .jpg is good for photographs but not for line art or scientific figures.

Do you have a recommended method to do this conversion? I'll see if I can do this locally.

I can do this locally 😅

@Bisaloo
Copy link
Member

Bisaloo commented Feb 6, 2024

Do you have a recommended method to do this conversion? I'll see if I can do this locally.

Ideally, it shouldn't be a conversion since jpeg is a lossy format. The ideal solution would be to resave the images directly as png.

@avallecam
Copy link
Member Author

Do you have a recommended method to do this conversion? I'll see if I can do this locally.

Ideally, it shouldn't be a conversion since jpeg is a lossy format. The ideal solution would be to resave the images directly as png.

thanks, hopefully, I did a direct resave as png.

Copy link
Member

@Bisaloo Bisaloo left a comment

Choose a reason for hiding this comment

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

I see some quality issues with some images. Please leave me a moment to update them from my side.

@avallecam
Copy link
Member Author

avallecam commented Feb 6, 2024

I see some quality issues with some images. Please leave me a moment to update them from my side.

thanks, after your update, it would be informative to refer to the changes you perceived and how you solved them in the update.

What I see when comparing the figures in the previous commit and the last commit is a brief increase in the file size.

@Bisaloo
Copy link
Member

Bisaloo commented Feb 6, 2024

I didn't realize these figures were from articles, and they were provided as jpeg. This hurts my eyes 😭, and the proofing service should have fixed it before publication.

But I'll remove the last commit then. If a figure has been saved as jpeg, we should keep it this way, even though it should have been a png in the first place.

The idea of my first comment was that if we create a figure ourselves, it should be saved as png, not jpeg.

@avallecam
Copy link
Member Author

avallecam commented Feb 6, 2024

The idea of my first comment was that if we create a figure ourselves, it should be saved as png, not jpeg.

Oh! now I understand. Yes, I did not create a figure yet. All jpeg and jpg files are original figures that I downloaded from papers.

I'd be happy if you remove the last commit, then.

@avallecam
Copy link
Member Author

avallecam commented Feb 6, 2024

commit 0641875 is my worst commit ever! 🤣 I just realized I changed the figure's format but not change their names in the Rmd files, haha

github-actions bot pushed a commit that referenced this pull request Feb 6, 2024
@avallecam
Copy link
Member Author

avallecam commented Feb 6, 2024

cache/ folder now is not present in md-outputs-PR-104. thank you @Bisaloo for flagging this!

@avallecam avallecam mentioned this pull request Feb 6, 2024
@avallecam avallecam merged commit 08a73e4 into main Feb 6, 2024
4 checks passed
@avallecam avallecam deleted the early-task-1 branch February 6, 2024 16:19
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.

add episodes on Introduction and Read Delays add introduction episode on the epiverse-trace approach
5 participants