Open ebuhle opened 4 years ago
Hey @ebuhle,
Thanks for sharing. I clicked on the HTML link you shared but it doesn't appear to work - I got the following message "Sorry about that, but we can’t show files that are this big right now". Nonetheless, the PDF works fine. I quickly scanned the document. As of now, I don't have any specific feedback. I will try to take a closer look next week while also spending more time reading through your 2018 IPM memo.
FYI - I forward your message to Todd and Thomas. Lastly, I've reminded Thomas numerous times to get a GitHub account but as far as I am aware he still hasn't.
Right, what I was suggesting was to open the files from your local copy of the R project. For example, if you click on LCRchumIPM_analysis.html in the file explorer pane of RStudio, you'll see an option to open the file in a browser.
Anyway, thanks for passing along to Todd and Thomas -- who, yeah, really needs to get on GH! :smile: Meanwhile, I'll keep plugging away at the analysis and the vignette.
Sorry, where is the "LCRchumIPM_analysis.html" file located?
I see the "LCRchumIPM_analysis.R" file located in the R folder:
Also, the .Rmd file in this analysis --> R folder appears to be the same as the original .Rmd file that Mark created two years ago and is located in the "docs" folder:
@ebuhle Ignore my previous post. Right as I hit "comment", I remembered that I need to "pull" - literally the conversation we just had last week. Ugh. I got it now.
LOL, you beat me to it! I was just grabbing a screenshot.
HAHA. I quickly using up what limited credibility I have...
Also, FWIW, having GitHub-enabled RStudio projects located in directories that are automatically synced with the cloud (e.g., Dropbox or OneDrive) is often asking for trouble. I imagine you might be obligated to do that, and if it ain't broke don't fix it, but just for future reference. (And to enhance your cred.)
FYI, @ebuhle, you can add some other options to the YAML to dynamically update the date and help with the pdf formatting. For example,
date: "`r format(Sys.time(), "%d %B, %Y")`"
output:
pdf_document:
highlight: haddock
toc: yes
number_sections: true
toc_depth: '3'
fontsize: 11pt
geometry: margin=1in
IMO, the code highlighting in haddock
blends more nicely with the regular text. If you want to manually insert page breaks to help with the code formatting, you can use \newpage
.
Also, you don't need to keep the intermediate .md
file if you're planning to include equations because they won't render with GitHub flavored Markdown.
Thanks @mdscheuerell, good suggestions. Most of that YAML was copypasted from previous projects, and I'm more accustomed to knitting to HTML than PDF.
Hey @kalebentley, still not quite there but getting close. You can check out the knitted HTML and/or the .Rmd to see how it's looking. As indicated, I plan to drop in some mostly boilerplate background on IPMs and LCR chum. Let me know if you want to add something more extensive, but I was thinking we can always expand it later in a "living document". Aside from a few other formatting niceties like author affiliations, most of the content is there. Per our email conversation, there is very little displayed code for now, just a chunk showing the call to salmonIPM()
.
One question: there are a few things that really work better in HTML (e.g., there's no satisfactory way to show the fish_data
selection without scrollbars). Is that format OK?
Thanks for the update, @ebuhle. This all sounds good including using an HTML format.
Hey @kalebentley, to download the knitted HTML as a single file, you can go here
and right-click-Save-As on either "View Raw" or "Download". No need to change the filename or extension.
To download a specific stable version, you just go to the commit in question, "Browse the repository at this point in the history", and do the same thing.
There's been some discussion offline about updating the Forecasting section of the vignette. This is straightforward in principle, but the model now includes some new external inputs / boundary conditions that weren't present a year ago when we last generated forecasts. Specifically, we would now need to specify p_G_obs
and add_S_obs
in the case of Duncan Channel
. Fortunately, if we restrict ourselves to 1-year-ahead forecasts (i.e. 2021, given that our data are current through 2020), these conditions are basically moot -- we'd be predicting adult returns from spawning in Duncan Channel
, not spawners taken into the channels, and likewise the relative fecundity of this year's spawners is irrelevant.
If that sounds good, I'll go ahead and prioritize getting this section done by COB tomorrow.
@ebuhle One year ahead forecast was what I was looking for. Thanks
@ebuhle I understand adding this will delay finishing the updated vignette. If it's going to cause more than say a weeks delay in finishing we should talk about it. I want to attach the new/updated vignette to the final status report to BPA (due 6/30/21), I'm fine with some delay ( a week or so late) but don't want to push it much beyond that. I've been "moderately" late on these before and it's not a big deal.
Thanks @Hillsont, good to know. Hopefully this will still be done by tomorrow; I'll just reshuffle some other tasks. Worst-case scenario is that I will need to devote tomorrow and Thurs to those tasks, in which case I'll come back and finish up the vignette on Fri. I'm currently waiting for the "forecast" version of the model (i.e., with empty rows appended for 2021) to run, which should be done in a few hours.
@ebuhle your plan re: 1 year ahead forecasts makes sense. I also don't think it's that problematic that we'd have to supply the number of translocated spawners (p_G_obs and add_S_obs) to forecast past 1 year ahead--forecasting way in the future is less about conducting a purely data driven exercise, than evaluating sensitivity to management options (which could include choices about how many spawners to translocate...
Also, FWIW, having GitHub-enabled RStudio projects located in directories that are automatically synced with the cloud (e.g., Dropbox or OneDrive) is often asking for trouble. I imagine you might be obligated to do that, and if it ain't broke don't fix it, but just for future reference. (And to enhance your cred.)
I have run into such version control issues...after reading online (and maybe talking with you, eric??), i landed on having a local copy of each repo on each machine I use (so I never work on my "laptop" local while working on my desktop. This method seems to work well!
I also don't think it's that problematic that we'd have to supply the number of translocated spawners (p_G_obs and add_S_obs) to forecast past 1 year ahead--forecasting way in the future is less about conducting a purely data driven exercise, than evaluating sensitivity to management options (which could include choices about how many spawners to translocate...
Oh, I agree. My point was just that if we had wanted to do that for this reporting deadline, we would have had to discuss and reach consensus about it and update the code accordingly within the next few hours. I'm glad we don't have to do that.
OK, see what you think of the current version. If you're happy, I'm ready to call it good for the purposes of this week's report.
@ebuhle Looks good to me, thanks for getting the forecast part added last minute.
Hey Eric, I didn't have much time to review the updated Markdown document but I think it looks great based on a quick scroll through. I'll plan on taking a deeper dive at some point in the future and providing more feedback. Thank you so much for your hard work and time. This is really starting to come together nicely!!!
Todd - do I need to send you a copy of the link to submit to BPA or can you get one using the instructions Eric provided yesterday?
Kale
@kalebentley The instructions @ebuhle provided work great, I'll handle the reporting to BPA part. Thanks again to everyone for the work on this.
OK, see what you think of the current version. If you're happy, I'm ready to call it good for the purposes of this week's report.
@ebuhle I know you weren't waiting on me, and for good reason, but I just wanted to echo the others' sentiments about a job really well done—super sharp presentation!
@ebuhle the html looks fantastic! Great work on this! Really impressive! So lucky to have you working on this project with us!
Thanks @tbuehrens, the feeling is mutual!
A draft of the updated vignette is now available. I need to add a few citations and clean up some other loose ends, but all the main pieces are there. Let me know if you have any proposed edits before we submit it to BPA tomorrow. (I would say you're welcome to edit the RMarkdown yourself and submit a pull request, but that would inevitably create merge conflicts to be resolved since I'll be actively working on it as well.)
As always, if you're not working in RStudio you can download the HTML directly from GitHub by clicking the button in the upper right:
I just pushed a revised version incorporating comments from @kalebentley, among other edits. Unless anyone has further edits, I'm happy submitting the current version.
Hi @kalebentley @Hillsont @tbuehrens, an updated vignette is now available. Per conversations with @kalebentley, this is just a quick rerun of last year's version with the models (retrospective and prospective) re-fitted to include the latest available data. Nothing has substantively changed about any of the estimates or inferences.
Two notes about formatting: first, you'll notice the tables are not as nice as before. That's because I'm now getting an inscrutable error when trying to load kableExtra. I first encountered this a couple months ago and have not been able to figure it out yet, so in the interim I used the bare-bones knitr::kable()
. The most annoying difference is with the sample of fish_data
in the Data Structure section, where the absence of horizontal scrollbars makes the table run past the right margin (you can still see all of it, but you have to right-scroll the entire document). If this is unacceptable, I can print the raw data frame instead, which doesn't look great either (columns are wrapped like output to the R console) but at least fits within the margins.
Second, I left in various editorial notes that I added in the weeks after the end of last year's contract period with an eye toward future refinements of this document and its eventual transformation into a manuscript. I figured this was fine since the current version is mainly for our own internal use, but if you'd prefer not to see them I can comment them out so they don't appear in the rendered output.
Hey @ebuhle, FYI, I've been using {kableExtra} for tables recently without issue, but without first loading the package (ie, kableExtra::kable(...)
). I just tried loading the package and it does, without error. In case it's helpful, here's my session info:
> library(kableExtra)
> sessionInfo()
R version 4.3.0 (2023-04-21)
Platform: x86_64-apple-darwin20 (64-bit)
Running under: macOS Ventura 13.6.7
Matrix products: default
BLAS: /System/Library/Frameworks/Accelerate.framework/Versions/A/Frameworks/vecLib.framework/Versions/A/libBLAS.dylib
LAPACK: /Library/Frameworks/R.framework/Versions/4.3-x86_64/Resources/lib/libRlapack.dylib; LAPACK version 3.11.0
locale:
[1] en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8
time zone: America/Los_Angeles
tzcode source: internal
attached base packages:
[1] stats graphics grDevices utils datasets methods base
other attached packages:
[1] kableExtra_1.3.4
loaded via a namespace (and not attached):
[1] httr_1.4.6 svglite_2.1.1 cli_3.6.2 knitr_1.45 rlang_1.1.2
[6] xfun_0.41 stringi_1.8.3 glue_1.6.2 colorspace_2.1-0 htmltools_0.5.7
[11] scales_1.3.0 rmarkdown_2.25 evaluate_0.23 munsell_0.5.0 fastmap_1.1.1
[16] lifecycle_1.0.4 stringr_1.5.1 compiler_4.3.0 rvest_1.0.3 rstudioapi_0.14
[21] systemfonts_1.0.4 digest_0.6.33 viridisLite_0.4.2 R6_2.5.1 magrittr_2.0.3
[26] webshot_0.5.4 tools_4.3.0 xml2_1.3.4
Thanks for that, @mdscheuerell, and nice to see you on here! The error message is
Error: package or namespace load failed for ‘kableExtra’ in inDL(x, as.logical(local), as.logical(now), ...):
unable to load shared object 'C:/R/R-4.3.2/library/systemfonts/libs/x64/systemfonts.dll':
LoadLibrary failure: The specified procedure could not be found.
I have no idea what "the specified procedure" means, but that file does exist. I tried googling when this first came up, without much luck. But you've at least confirmed that the problem isn't my less-than-current version of R, although it could be a Windows thing. :man_shrugging:
Edit to add that kableExtra::kable()
throws the same error.
Hi @kalebentley, I'm working on an RMarkdown report describing the analyses, as we discussed last week. To keep things (relatively) simple I decided to just focus on the two-stage models. I also generated some forecasts, using a 5-year (roughly one maximum lifespan) horizon. The caveat here, which will be explained in the document, is that forecasting requires assumptions about the external inputs -- in particular, broodstock removals and p_HOS. Lacking any detailed scenarios and for the sake of the exercise, I set both of these to zero, so we're forecasting natural-origin adults in the absence of hatchery operations.
The text is still a work in progress, but you can look at the knitted HTML and PDF to get an idea of the layout and (most of) the figures and tables, minus one or two others I'm considering adding. Either output file can be opened directly from the file explorer pane in your RStudio project. As you'll see, the PDF doesn't look as nice as the HTML; it's hard to get this code-heavy, literate-programming style to work well with page breaks.
I'll start fleshing out the text, but in the meantime I welcome any feedback and suggestions.