While from a technical point the app works like a charm, it is not really usable for me as a beginner as explanations of the different variables (including their units) are missing.
Without it, its is neither clear what the different parameter settings mean, nor what the tables or figures show.

I would suggest to either add explanations/legends to the app explaining all the different variables and their units (best, but also very time consuming solution) or to put a disclaimer into the documentation of this function and the vignette, explaining that this app is targeted at advanced users and requires a priori knowledge about the meaning of all these variables. Ideally, such a disclaimer should come with an explanation how that knowledge can be obtained.

openjournals/joss-reviews#2890

Change from Travis-CI and Appveyor to GitHub Actions. Swap badges in the README once operable. Also add coverage indicator.

See https://ropenscilabs.github.io/actions_sandbox/

Change the name of the repository and the R package to ambrosia - all lower case.

Guidelines are missing for third parties wishing to 1) Contribute to the software (README states, that one should get in contact with the team, but does not state how to do so) 2) Report issues or problems with the software (should that be done via Github Issues, or differently?) 3) Seek support (unclear who to approach on which channel in this case)

openjournals/joss-reviews#2890

I see some potential for improvement in the provided vignette. In particular, I see the following issues:

• Currently, explanatory text either comes in the form of a header or as a comment in the code snippets. It would improve the readability to convert the lengthy header to short titles of the sections and use some explanatory sentences for each example (in addition to the comments in the code). See for instance vignette("knitr") as an example how something like that could look like.
• The flow of the examples is somehow inconsistent in the sense that sometimes an example builds on the calculations of the previous example (e.g. 1.3 continues using the object Food_Demand created in 1.2), but in other cases it does not (e.g. 1.1 builds a parameter object, while 1.2 seems to use the same parameter object but rebuilds it from scratch rather than using the result from 1.1). It would be nice to just reuse the objects from previous code snippets.
• As a vignette is typically meant as an exercise that user can easily run the heavily time consuming snippets in 2.1 and 2.4 are in my view not very useful. Wouldn't it be possible for these examples to apply the calculations to dummy data sets and reduce the runtime to a few seconds. Even if this would not provide reasonable results it would teach how the functions need to be called and what objects are returned by it.

Solving these three issues whould in my opinion greatly improve the readability of the vignette.

openjournals/joss-reviews#2890

Add Zenodo badge for release DOI.

The LICENSE file states that the software package is licensed under BSD-2, but the provided license actually extends the original BSD-2 license text (compare the original license text here: https://opensource.org/licenses/BSD-2-Clause) and adds additional burden on the user, making it a custom license.

Please remove the license restrictions which do not belong to the BSD-2 license or change the applied license.

openjournals/joss-reviews#2890

Below find my review comments for the JOSS review at openjournals/joss-reviews#2890. Any questions, please let me know.

General comments

Thank you for the chance to review this package. Subject matter is mostly outside of my area of expertise and I did learn quite a bit. That being said, anytime software can be written to help ease implement important modelling efforts, then that is worthy of sharing. Your ambrosia package is no exception! Nicely done.

A couple of other general comments:

1. I like the shiny app. I am mostly ignorant of food demand models so having this helped me.
2. Your documentation is very extensive and you provide a detailed vignette. This will be appreciated by your users.

Below I have provide some comments related to the items on the checklist that I have not yet checked off on. I have also included some specific comments for you to consider in your revisions. Nothing major, just some items that I think will help improve upon an already good contribution.

Checklist comments

General Checks: Contribution and authorship:

• Robert Link (rplzz) had the most commits to repo (granted they are a bit old) but not listed as an author on the paper or the package. Also authors James Edmunds and Ryna Cui did not commit (not questioning their inclusion as many other contributions besides commits!). Just take a second look at authors to make sure everyone is good!

Functionality: Installation:

• In README's installation section think about using remotes::install_github() instead of devtools::install_github(). remotes is much lighter weight and focuses on the install which is the specific use case here. There is some disagreement on this but I prefer remotes. Again, up to you all.

• A few Rd warnings on install. First three are from including () in the \link{}. Last one appears to be from line 172 in food-demand-mc.R. Nothing to link to in those.

Rd warning: C:/Users/JHollist/AppData/Local/Temp/RtmpOSX3FJ/R.INSTALL516c76db875/ambrosia/man/calculate_ambrosia_params.Rd:5: missing link 'create_dataset_for_parameter_fit()'

Rd warning: C:/Users/JHollist/AppData/Local/Temp/RtmpOSX3FJ/R.INSTALL516c76db875/ambrosia/man/calculate_ambrosia_params.Rd:59: missing link 'create_dataset_for_parameter_fit()'

Rd warning: C:/Users/JHollist/AppData/Local/Temp/RtmpOSX3FJ/R.INSTALL516c76db875/ambrosia/man/calculate_ambrosia_params.Rd:23: missing link 'create_dataset_for_parameter_fit()'

Rd warning: C:/Users/JHollist/AppData/Local/Temp/RtmpOSX3FJ/R.INSTALL516c76db875/ambrosia/man/vec2param.Rd:69: missing link 'A_s, A_n, xi_ss, xi_cross, xi_nn, nu1_n, lambda_s, k_s, Pm, psscl, pnscl'

Documentation: A statement of need:

• Need a bit more on this in the README. Target Audience? Why is this needed as an R package (or as any software solution)?
• A struggle I always have with JOSS reviews is the distinction between what is in the repository documentation (e.g. README) and what should go in the paper. Summary's in this case are the same and Statement of need is in the paper but not currently in the README. I personally think having them repeated is OK, because I think you are getting this information to people at different times with the different formats so some repeat is good. That being said, a little clarification from editor is probably warranted.

Software Paper: References:

• DOI for Wilkinson et al: https://doi.org/10.1038/sdata.2016.18
• DOI for Calvin et al: https://doi.org/10.5194/gmd-12-677-2019

Specific comments

• Even though this is not necessarily headed to CRAN, I would make sure that all of the CRAN checks pass. On my machine I had 5 warnings and 4 notes when I ran CRAN checks.
• Vignette Examples: Stick with left assign on on the examples. You have a few cases where you use it. I do it too, but for examples may be best to stick to the traditional left assign.
• Vignette Example 3.1: Needs shiny installed in order to run. Make note for users that may not have shiny already installed.
• Vignette Example 4.4: Extra "," in calculate_ambrosia_params?
• Vignette Example 5.1: Also long running. Add note to vignette. Also any way to speed up/reduce runtime of examples? Took 50+ on my laptop
• Vignette Example 5.1: Extra "," in calculate_ambrosia_params?
• clean up actions. Build and r-cmd-check appear to be doing mostly the same thing (with build sending test coverage) and both are macOS with old versions of R. Minimum, in rdmd.yml add in a windows build on release and bump mac0S R version in both yml to release.

matrix:
        config:
          - {os: windows-latest, r: 'release'}
          - {os: macos-10.15, r: 'release'}

• Function names use both dot case and snake case. Having different naming conventions for the functions in the package will make it a challenge for your users. Pick one and stick with it.
• Paper: line 57: "of goods" should it be "of goods: "
• Paper: line 87: No need to refer directly to an rmd here. Link text could simply be "ambrosia vignette" the links to the html.
• Paper: line 107 "User" should be "Users"

It seems that by default the supplied vignette is not being installed when running devtools::install_github('JGCRI/ambrosia')

I would suggest to ask users to run devtools::install_github('JGCRI/ambrosia', build_vignettes = TRUE) instead.

Doing so would also allow to refer to the vignette via the command vignette("ambrosia_vignette") instead of referring to the vignettes directory.

(this is related to openjournals/joss-reviews#2890)

Add PNNL License and Disclaimer once approved.

Prepare JOSS submission directory and contents:

• README.md
• paper.md
• paper.bib
• paper_local.pdf

Archive the code found in the paper1 directory for the following paper:

Edmonds J.A., R.P. Link, S.T. Waldhoff, and Y. Cui. 2017. "A Global Food Demand Model for the Assessment of Complex Human-Earth Systems." Climate Change Economics 8, no. 4:Article No. 1750012. PNNL-SA-121534. doi:10.1142/S2010007817500129

Remove paper1 recursively after archival and cite in the package.

NOTE: This should not be done until the new version has been tested. Potentially setup a meta-repository for the Edmonds paper.

README.md refers to Calvin et al. 2019 but does not provide the corresponding citation.

openjournals/joss-reviews#2890

As the parameters seem to have a central role in the whole process a bit more explanation and a better visibility of these explanations throughout the documentation would be useful. Some examples

• A_s is described as "Scaling parameter for staples (constant)", but it is unclear to me how it works. What exactly is scaled here and which range of scaling factors makes sense? How can these parameters be estimated?
• All parameters lack unit information
• In the vignette the parameter names are introduced and used, but they are not explained. Also a link to a page explaining these parameters is not available.
• vec2param (and probably other documentation pages) are mentioning the parameter names, but not explaining what it is (sidnode: in case of 8 parameters it looks like the documentation is only explaining what the first 7 are, but not what the 8th parameter is).

Please check again the documentation with a focus on the parameters and make sure that the correspoding parameters are either explained on the page or the parameter description is linked along with the use of these parameter names

openjournals/joss-reviews#2890