FredHutch / VISCtemplates

Tools for writing reproducible reports at VISC
Other
4 stars 2 forks source link

Fix for issue with latest RStudio version rendering PDFs #159

Closed kelliemac closed 1 week ago

kelliemac commented 1 month ago

Relates to: https://github.com/FredHutch/VISCtemplates/issues/150

The CSLReferences environment in template.tex has been redefined to be consistent with the latest pandoc template, and this seems to solve the issue with rendering the references section in PDFs that has been observed with the latest RStudio version

@mayerbry can you test with your version of RStudio to see if the fix works with multiple versions?

mayerbry commented 1 month ago

Getting the error trying to compile a report with the older version of RStudio :(

! LaTeX Error: Something's wrong--perhaps a missing \item.
slager commented 1 month ago

Interesting, I guess a slightly different error message than in #150?

kelliemac commented 4 weeks ago

working for @mayerbry now, as of 6/17. I think we should leave this PR unmerged for now, and just refer people to this branch as they need it

kelliemac commented 3 weeks ago

@slager was able to reproduce on statsrv today the error that @mayerbry was getting previously:

LaTeX Font Info:    Trying to load font information for TS1+lmr on input line 578.
(/home/dslager/.TinyTeX/texmf-dist/tex/latex/lm/ts1lmr.fd
File: ts1lmr.fd 2015/05/01 v1.6.1 Font defs for Latin Modern
)

! LaTeX Error: Something's wrong--perhaps a missing \item.

See the LaTeX manual or LaTeX Companion for explanation.
Type  H <return>  for immediate help.
 ...                                              

l.592 \end{CSLReferences}

Here is how much of TeX's memory you used:
 14007 strings out of 475560
 225513 string characters out of 5777741
 1949905 words of memory out of 5000000
 36284 multiletter control sequences out of 15000+600000
 622124 words of font info for 83 fonts, out of 8000000 for 9000
 14 hyphenation exceptions out of 8191
 89i,15n,95p,532b,520s stack positions out of 10000i,1000n,20000p,200000b,200000s

!  ==> Fatal error occurred, no output PDF file produced!

here are some details that may help us triangulate the reason for inconsistencies across systems:

> sessionInfo()
R version 4.0.4 (2021-02-15)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: openSUSE Leap 42.2

Matrix products: default
BLAS:   /usr/local/apps/R/R-4.0.4-validated/lib64/R/lib/libRblas.so
LAPACK: /usr/local/apps/R/R-4.0.4-validated/lib64/R/lib/libRlapack.so

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C               LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8    
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8    LC_PAPER=en_US.UTF-8       LC_NAME=C                 
 [9] LC_ADDRESS=C               LC_TELEPHONE=C             LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

loaded via a namespace (and not attached):
 [1] compiler_4.0.4           fastmap_1.1.1            cli_3.6.2                htmltools_0.5.8         
 [5] tools_4.0.4              rstudioapi_0.16.0        VISCtemplates_1.2.0.9000 yaml_2.3.8              
 [9] tinytex_0.50             rmarkdown_2.26           knitr_1.45               xfun_0.43               
[13] digest_0.6.35            VISCfunctions_1.2.2      rlang_1.1.3              evaluate_0.23           
> rmarkdown::pandoc_version()
[1] ‘2.11.4’
> tinytex::tlmgr_version()
tlmgr revision 71079 (2024-04-26 00:15:30 +0200)
tlmgr using installation: /home/dslager/.TinyTeX
TeX Live (https://tug.org/texlive) version 2024
kelliemac commented 3 weeks ago

on my system with the following the fix works:

> rmarkdown::pandoc_version()
[1] '3.1.11'

> sessionInfo()
R version 4.4.0 (2024-04-24)
Platform: aarch64-apple-darwin20
Running under: macOS Sonoma 14.5

I am betting on the pandoc version being the issue, but it could be the R version too, or even RStudio (the statsrv version of RStudio is old). tinytex::tlmgr_version() on my system gives the same result as on statsrv, so I don't think that's the issue.

slager commented 3 weeks ago

No bug on windows with new R and Rstudio versions

RStudio 2024.04.1+748 "Chocolate Cosmos" Release (3ada7c6ddc8fcdb86a727a4f0ae467b9d9a7296c, 2024-05-07) for windows
Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) RStudio/2024.04.1+748 Chrome/120.0.6099.291 Electron/28.2.6 Safari/537.36, Quarto 1.4.553
> sessionInfo()
R version 4.4.0 (2024-04-24 ucrt)
Platform: x86_64-w64-mingw32/x64
Running under: Windows 10 x64 (build 19045)

Matrix products: default

locale:
[1] LC_COLLATE=English_United States.utf8  LC_CTYPE=English_United States.utf8   
[3] LC_MONETARY=English_United States.utf8 LC_NUMERIC=C                          
[5] LC_TIME=English_United States.utf8    

time zone: America/Los_Angeles
tzcode source: internal

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

loaded via a namespace (and not attached):
 [1] compiler_4.4.0           fastmap_1.2.0            cli_3.6.2               
 [4] htmltools_0.5.8.1        tools_4.4.0              rstudioapi_0.16.0       
 [7] VISCtemplates_1.2.0.9000 yaml_2.3.8               rmarkdown_2.27          
[10] knitr_1.47               xfun_0.44                digest_0.6.35           
[13] rlang_1.1.4              evaluate_0.23           

> rmarkdown::pandoc_version()
[1] ‘3.1.11’

> tinytex::tlmgr_version()
tlmgr revision 71079 (2024-04-26 00:15:30 +0200)
tlmgr using installation: C:/Users/dslager/AppData/Roaming/TinyTeX
TeX Live (https://tug.org/texlive) version 2024
slager commented 3 weeks ago

No bug on my home Linux with new R and Rstudio versions:

RStudio 2024.04.2+764 "Chocolate Cosmos" Release (e4392fc9ddc21961fd1d0efd47484b43f07a4177, 2024-06-05) for Ubuntu Jammy
Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) rstudio/2024.04.2+764 Chrome/120.0.6099.291 Electron/28.3.1 Safari/537.36, Quarto 1.3.450 (/opt/quarto/bin/quarto)
> sessionInfo()
R version 4.4.0 (2024-04-24)
Platform: x86_64-pc-linux-gnu
Running under: Linux Mint 21.3

Matrix products: default
BLAS:   /usr/lib/x86_64-linux-gnu/blas/libblas.so.3.10.0 
LAPACK: /usr/lib/x86_64-linux-gnu/lapack/liblapack.so.3.10.0

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C               LC_TIME=en_US.UTF-8       
 [4] LC_COLLATE=en_US.UTF-8     LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8   
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C                  LC_ADDRESS=C              
[10] LC_TELEPHONE=C             LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C       

time zone: America/Los_Angeles
tzcode source: system (glibc)

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base     

loaded via a namespace (and not attached):
 [1] compiler_4.4.0           fastmap_1.2.0            cli_3.6.2               
 [4] htmltools_0.5.8.1        tools_4.4.0              rstudioapi_0.16.0       
 [7] VISCtemplates_1.2.0.9000 yaml_2.3.8               rmarkdown_2.27          
[10] knitr_1.47               xfun_0.45                digest_0.6.35           
[13] rlang_1.1.4              evaluate_0.24.0         

> rmarkdown::pandoc_version()
[1] ‘3.1.11’

> tinytex::tlmgr_version()
tlmgr revision 71079 (2024-04-26 00:15:30 +0200)
tlmgr using installation: /home/dave/.TinyTeX
TeX Live (https://tug.org/texlive) version 2024
kelliemac commented 3 weeks ago

So in summary, we have confirmed that this fix works for Mac, Windows, and Linux running up-to-date R (version 4.4.0), RStudio (version 2024.04.X+Y "Chocolate Cosmos" Release), and pandoc (version 3.1.11). But, it does not work on statsrv, which uses older versions (R version 4.0.4, pandoc version 2.11.4, RStudio version ?).

I am hesitant to merge this PR because I do not want to break anyone's workflow on statsrv, but I do want them to be able to get any other VISCtemplates updates and improvements by installing the latest package version.

Does anyone know what the process is for updating statsrv, and how likely it is that this might happen in the near future?

mayerbry commented 3 weeks ago

thanks @slager for checking on this. I haven't dealt directly with statsrv, but I do know that updates from them can be slow since it is a validated instance.

I'm not sure what the best strategy is. Assuming windows and linux work fine regardless, we could continually merge major updates into this branch, and then us Mac users have to install from here until statsrv is updated.

This makes development a bit difficult though since we cannot natively compile reports from branches that use the original template.

slager commented 3 weeks ago

I can successfully knit on statsrv when the current develop branch of VISCtemplates is installed. I'll bisect VISCtemplates as needed and look into it more, now that I have a reprex and access to a platform to reliably reproduce the latex error.

slager commented 2 weeks ago

Can confirm on statsrv @kelliemac that 0c2ded3 is the commit that breaks knitting the generic report pdf. When I use this branch and revert only that commit, knitting works again on statsrv.

slager commented 2 weeks ago

There it is! @kelliemac do any alternative approaches to https://github.com/FredHutch/VISCtemplates/pull/159/commits/0c2ded3a4dc887351377e163a7d11292b28ed854 come to mind that might have a chance at being more robust to working across pandoc versions?

kelliemac commented 2 weeks ago

Hmm, not sure. The issue, though, seems to be with the \begin{list} that has been added. Here are a couple of snapshots to illustrate:

References working properly in .tex file not on statsrv:

\phantomsection\label{refs}
\begin{CSLReferences}{1}{0}
\bibitem[\citeproctext]{ref-deCamp2014}
deCamp, A., P. Hraber, R. T. Bailer, M. S. Seaman, C. Ochsenbauer, J. Kappes, R. Gottardo, et al. 2014. {``Global Panel of HIV-1 Env Reference Strains for Standardized Assessments of Vaccine-Elicited Neutralizing Antibodies.''} \emph{J. Virol.} 88 (5): 2489--2507.

\bibitem[\citeproctext]{ref-Fay2012}
Fay, MP, and Shih JH. 2012. {``Weighted Logrank Tests for Interval Censored Data When Assessment Times Depend on Treatment.''} \emph{Statistics in Medicine} 31: 3760--72.

\bibitem[\citeproctext]{ref-Huang:2013fl}
Huang, Yunda, and Raphaël Gottardo. 2013. {``Comparability and Reproducibility of Biomedical Data.''} \emph{Briefings in Bioinformatics} 14 (4): 391--401.

\bibitem[\citeproctext]{ref-Lyderson2009}
Lydersen, Stian, Morten W. Fagerland, and Petter Laake. 2009. {``Recommended Tests for Association in 2×2 Tables.''} \emph{Statistics in Medicine} 28 (7): 1159--75. \url{https://doi.org/10.1002/sim.3531}.

\bibitem[\citeproctext]{ref-Montefiori2009}
Montefiori, D. C. 2009. {``Measuring HIV Neutralization in a Luciferase Reporter Gene Assay.''} \emph{Methods Mol. Biol.} 485: 395--405.

\bibitem[\citeproctext]{ref-Sarzotti-Kelsoe2014}
Sarzotti-Kelsoe, M., R. T. Bailer, E. Turk, C. L. Lin, M. Bilska, K. M. Greene, H. Gao, et al. 2014. {``Optimization and Validation of the TZM-Bl Assay for Standardized Assessments of Neutralizing Antibodies Against HIV-1.''} \emph{J. Immunol. Methods} 409 (July): 131--46.

\bibitem[\citeproctext]{ref-Suissa1985}
Suissa, Samy, and Jonathan J. Shuster. 1985. {``Exact Unconditional Sample Sizes for the 2 × 2 Binomial Trial''} 148 (January): 317--27.

\end{CSLReferences}

References not working properly in statsrv like testing:

\hypertarget{refs}{}
\begin{CSLReferences}{1}{0}
\leavevmode\hypertarget{ref-deCamp2014}{}%
deCamp, A., P. Hraber, R. T. Bailer, M. S. Seaman, C. Ochsenbauer, J. Kappes, R. Gottardo, et al. 2014. {``Global Panel of HIV-1 Env Reference Strains for Standardized Assessments of Vaccine-Elicited Neutralizing Antibodies.''} \emph{J. Virol.} 88 (5): 2489--2507.

\leavevmode\hypertarget{ref-Fay2012}{}%
Fay, MP, and Shih JH. 2012. {``Weighted Logrank Tests for Interval Censored Data When Assessment Times Depend on Treatment.''} \emph{Statistics in Medicine} 31: 3760--72.

\leavevmode\hypertarget{ref-Huang:2013fl}{}%
Huang, Yunda, and Raphaël Gottardo. 2013. {``Comparability and Reproducibility of Biomedical Data.''} \emph{Briefings in Bioinformatics} 14 (4): 391--401.

\leavevmode\hypertarget{ref-Lyderson2009}{}%
Lydersen, Stian, Morten W. Fagerland, and Petter Laake. 2009. {``Recommended Tests for Association in 2×2 Tables.''} \emph{Statistics in Medicine} 28 (7): 1159--75. \url{https://doi.org/10.1002/sim.3531}.

\leavevmode\hypertarget{ref-Montefiori2009}{}%
Montefiori, D. C. 2009. {``Measuring HIV Neutralization in a Luciferase Reporter Gene Assay.''} \emph{Methods Mol. Biol.} 485: 395--405.

\leavevmode\hypertarget{ref-Sarzotti-Kelsoe2014}{}%
Sarzotti-Kelsoe, M., R. T. Bailer, E. Turk, C. L. Lin, M. Bilska, K. M. Greene, H. Gao, et al. 2014. {``Optimization and Validation of the TZM-Bl Assay for Standardized Assessments of Neutralizing Antibodies Against HIV-1.''} \emph{J. Immunol. Methods} 409 (July): 131--46.

\leavevmode\hypertarget{ref-Suissa1985}{}%
Suissa, Samy, and Jonathan J. Shuster. 1985. {``Exact Unconditional Sample Sizes for the 2 × 2 Binomial Trial''} 148 (January): 317--27.

\end{CSLReferences}
kelliemac commented 1 week ago

FWIW, this issue seems to have appeared due to changes in the CSLReferences environment as implemented in pandoc version 3.1.7: https://pandoc.org/releases.html#pandoc-3.1.7-2023-08-31

kelliemac commented 1 week ago

status update: just need to figure out how to dynamically set new yaml header parameter use-old-csl-refs to be true or false. I tried use-old-csl-refs: "r rmarkdown::pandoc_version() < '3.1.7'" but this doesn't work because you end up with a string (I think) instead of a boolean value. use-old-csl-refs: r rmarkdown::pandoc_version() < '3.1.7' didn't seem to work either. but if we can correctly toggle this based on the pandoc version, we should be golden! I tested hard-coding use-old-csl-refs: true and use-old-csl-refs: false and the r-cmd-checks on the appropriate systems pass (i.e., all but the statsrv-like system passes for value false, and only the statsrv-like system passes for value true)

slager commented 1 week ago

I haven't been able to get it. Even the "passing" jobs in my attempts above still have warnings that the R expression isn't producing a valid boolean value.

kelliemac commented 1 week ago

IT WORKS!!!! All reports now rendering across all systems.

I don't love the current solution, though, because there is a lot of duplicate code across the main template.tex file and the new template_with_old_csl_refs.tex file. Going to try to split this up to reduce duplication.

kelliemac commented 1 week ago

Ok, I've investigated this enough for now. I can't seem to get the includes argument to actually work, so I can't split up the template tex file.

kelliemac commented 1 week ago

@slager I am giving up on reducing the duplicate code for now, but feel free to try some things on your end if you want!

slager commented 1 week ago

I have an idea that should work; I'll give it a try.

slager commented 1 week ago

Looks like that worked!

The key to this puzzle was in the pandoc documentation:

Note that in YAML metadata (and metadata specified on the command line using -M/--metadata), unquoted true and false will be interpreted as Boolean values. But a variable specified on the command line using -V/--variable will always be given a string value. Hence a conditional if(foo) will be triggered if you use -V foo=false, but not if you use -M foo=false.

kelliemac commented 1 week ago

yay!!! awesome work @slager!

kelliemac commented 1 week ago

@mayerbry when you have a chance can you review this PR and let us know if you have any feedback?

slager commented 1 week ago

Brian OK'd merging via Slack. Merging...