editorialbot commented 1 year ago

Submitting author: !--author-handle-->@hahsan1@martinfleis<!--end-editor-- Reviewers: @jsun, @kauedesousa Archive: 10.5281/zenodo.7938714

Status

Status badge code:

HTML: <a href="https://joss.theoj.org/papers/97ae587715480c8b2b2f95d8738cf011"><img src="https://joss.theoj.org/papers/97ae587715480c8b2b2f95d8738cf011/status.svg"></a>
Markdown: [![status](https://joss.theoj.org/papers/97ae587715480c8b2b2f95d8738cf011/status.svg)](https://joss.theoj.org/papers/97ae587715480c8b2b2f95d8738cf011)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@jsun & @kauedesousa, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @martinfleis know.

✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨

Checklists

📝 Checklist for @kauedesousa

📝 Checklist for @jsun

editorialbot commented 1 year ago

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

For a list of things I can do to help you, just type:

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf

editorialbot commented 1 year ago

Software report:

github.com/AlDanial/cloc v 1.88  T=0.06 s (839.9 files/s, 128004.7 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
HTML                            16            473             91           2142
R                               11            534            710           1653
CSS                              3             98             52            442
JavaScript                       4             65             35            267
YAML                             6             34              0            167
Markdown                         6             57              0            164
TeX                              1             11              0            143
Rmd                              1            126            221             72
XML                              1              0              0             51
SVG                              1              0              1             11
-------------------------------------------------------------------------------
SUM:                            50           1398           1110           5112
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository

editorialbot commented 1 year ago

Wordcount for paper.md is 1003

editorialbot commented 1 year ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1038/nclimate2470 is OK
- 10.1088/1748-9326/aac1c2 is OK
- 10.1371/journal.pone.0237918 is OK
- 10.5194/gmd-12-677-2019 is OK
- 10.5194/gmd-13-3995-2020 is OK
- 10.1029/2008GB003435 is OK
- 10.5334/jors.232 is OK
- 10.1002/2013EF000199 is OK
- 10.1088/1748-9326/ab639b is OK
- 10.1088/1748-9326/ac2965 is OK

MISSING DOIs

- None

INVALID DOIs

- None

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

martinfleis commented 1 year ago

👋🏼 @hahsan1, @jsun, @kauedesousa this is the review thread for the paper. All of our communications will happen here from now on.

All reviewers should create checklists with the JOSS requirements using the command @editorialbot generate my checklist. As you go over the submission, please check any items that you feel have been satisfied. There are also links to the JOSS reviewer guidelines.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues (and small pull requests if needed) on the software repository. When doing so, please mention https://github.com/openjournals/joss-reviews/issues/5226 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within about 2-4 weeks, feel free to start whenever it works for you. Please let me know if any of you require significantly more time. We can also use editorialbot to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@martinfleis) if you have any questions/concerns.

Thanks!

kauedesousa commented 1 year ago

Review checklist for @kauedesousa

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the https://github.com/JGCRI/osiris?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@hahsan1) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
[x] Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
[x] Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
[x] Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
[x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

jsun commented 1 year ago

Review checklist for @jsun

Conflict of interest

[x] I confirm that I have read the JOSS conflict of interest (COI) policy and that: I have no COIs with reviewing this work or that any perceived COIs have been waived by JOSS for the purpose of this review.

Code of Conduct

[x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

[x] Repository: Is the source code for this software available at the https://github.com/JGCRI/osiris?
[x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
[x] Contribution and authorship: Has the submitting author (@hahsan1) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?
[x] Substantial scholarly effort: Does this submission meet the scope eligibility described in the JOSS guidelines
[x] Data sharing: If the paper contains original data, data are accessible to the reviewers. If the paper contains no original data, please check this item.
[x] Reproducibility: If the paper contains original results, results are entirely reproducible by reviewers. If the paper contains no original results, please check this item.
[x] Human and animal research: If the paper contains original data research on humans subjects or animals, does it comply with JOSS's human participants research policy and/or animal research policy? If the paper contains no such data, please check this item.

Functionality

[x] Installation: Does installation proceed as outlined in the documentation?
[x] Functionality: Have the functional claims of the software been confirmed?
[x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

[x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
[x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
[x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
[x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
[x] Automated tests: Are there automated tests or manual steps described so that the functionality of the software can be verified?
[x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

[x] Summary: Has a clear description of the high-level functionality and purpose of the software for a diverse, non-specialist audience been provided?
[x] A statement of need: Does the paper have a section titled 'Statement of need' that clearly states what problems the software is designed to solve, who the target audience is, and its relation to other work?
[x] State of the field: Do the authors describe how this software compares to other commonly-used packages?
[x] Quality of writing: Is the paper well written (i.e., it does not require editing for structure, language, or writing quality)?
[x] References: Is the list of references complete, and is everything cited appropriately that should be cited (e.g., papers, datasets, software)? Do references in the text use the proper citation syntax?

jsun commented 1 year ago

Hi, @hahsan1

I tried to install the package and run get_example_data() and then ran the example script of Step 1. But I got the following errors. Would you please tell me where can I download the tas_Amon_CanESM5_historical_r1i1p1f1_orig.nc file? Thanks.

Starting calculate_deltas_from_climate...
Error in R_nc4_open: No such file or directory
Error in ncdf4::nc_open(batch0[1]) :
  Error in nc_open trying to open file Osiris_Data/climate_data/tas_Amon_CanESM5_historical_r1i1p1f1_orig.nc (return_on_error= FALSE )
In addition: Warning message:
'memory.limit()' is Windows-specific

hahsan1 commented 1 year ago

Hi @jsun, Just to clarify, you are on devTests.R and you ran get_example_data() which generates a folder called Osiris_Data_Test in your directory. Then you rancalculate_deltas_from_climate()? That's strange since this function shouldn't need tas_Amon_CanESM5_historical_r1i1p1f1_orig.nc. Let me know if you have further questions please.

jsun commented 1 year ago

I'm following the vignette https://jgcri.github.io/osiris/articles/vignette.html to run the following scripts.

library(osiris)
get_example_data()

data_folder <- 'Osiris_Data'

# Step 1: calculate_deltas_from_climate
osiris::calculate_deltas_from_climate(
  climate_dir = paste0(data_folder,"/climate_data"),
  write_dir = paste0(data_folder,"/outputs_calculate_delta_from_climate"),
  monthly_growing_season = paste0(data_folder,"/growing_seasons/pmonth_gslength_unifWheat_smallareamask.csv"),
  monthly_harvest_season = paste0(data_folder,"/growing_seasons/p_h_months_unifWheat_smallareamask.csv"),
  growing_season_dir = paste0(data_folder,"/growing_season_climate_data"),
  esm_name = "CanESM5",
  crops = c("Corn", "Wheat", "Rice", "Soy"),
  irrigation_rainfed = c("IRR", "RFD"),
  minlat = -87.8638,
  minlon = -179.75,
  rollingAvgYears = 15,
  tas_historical = "tas_Amon_CanESM5_historical_r1i1p1f1_orig.nc",
  tas_projected = "tas_Amon_CanESM5_ssp585_r1i1p1f1_gn_201501-210012.nc",
  pr_historical = "pr_Amon_CanESM5_historical_r1i1p1f1_orig.nc",
  pr_projected = "pr_Amon_CanESM5_ssp585_r1i1p1f1_gn_201501-210012.nc",
  historical_start_year = 1850,
  projection_start_year = 2015
)

The version of osiris is osiris_1.0.0 running on "R version 4.2.0 (2022-04-22), Platform: x86_64-apple-darwin17.0 (64-bit), Running under macOS Big Sur/Monterey 10.16".

hahsan1 commented 1 year ago

I realized the vignette was based on an old set of example scripts. I have updated it now. Please pull in the new change. Let me know if you have any other questions. Thank you

jsun commented 1 year ago

Hi, I updated the package and followed the online vignette to try to run the package. I still got the following error (it seems that the other file is not found).

Starting calculate_deltas_from_climate...
Error in R_nc4_open: No such file or directory
Error in ncdf4::nc_open(batch0[1]) :
  Error in nc_open trying to open file Osiris_Data/climate_data/tas_Amon_CanESM5_historical_r1i1p1f1_gn_201001-201501.nc (return_on_error= FALSE )
In addition: Warning message:
'memory.limit()' is Windows-specific

hahsan1 commented 1 year ago

I see the error now, the get_example_data() function is linking to the full data set rather than the test set. I will update that and let you know. Thank you for the patience.

hahsan1 commented 1 year ago

The issue is fixed now. The get_example_data() function will install a folder called Osiris_Data_Test (which is smaller).

After running that function, just set data_folder <- 'Osiris_Data_Test', then you can continue.

The outputs for each step are generated in the Osiris_DataTest folder and they will be called outputs. You will also see folders called outputs__test. These come with the data set and are used by the osiris/tests/testthat/test-osiris.R script for doing checks for ensuring accuracy of results.

jsun commented 1 year ago

Hi, thanks to fix the downloading path. I could successfully execute all functions following the vignette. Here is my question about the osiris package. The use of this package is described in detail in the vignette and there are four core functions for executing GCAM. However, each function requires a number of files for input; and reading the function documentation did not help me to understand the format of these files. For example,

The calculate_deltas_from_climate function requires a climate_dir, but there is no description of what files should be in the climate_dir.
The monthly_growing_season requires a csv file with columns named latgrid, ..., areamask; but I can't find definitions of these column names (e.g., what is `areamsk').

The complex inputs and outputs of these functions in the osiris package gave me the impression that this package is intended to solve a specific problem and that other researchers cannot use it to solve their own problems. So my question is does the package intend to solve a specific problem (e.g., the input data must be downloaded from xxx, crop must be xxx, ... etc)? If so, it should be mentioned that the osiris package is intended to solve a specific problem in the manuscript. Whereas, if the package is intended to be used by others, it is advisable to write a more detailed description of how to prepare the input files.

hahsan1 commented 1 year ago

Thank you for the questions. I can add more detail in the user guide and a description column in the tables to offer more details about the file contents.

Some of the input files are flexible in terms of the user being able to exchange the default file with their own file, and I think we can make that clearer. For example, the climate_dir contains the climate data that the user wants to apply to the ag yield. So it needs a historical and projection netcdf each for temperature (K) and precipitation (kg m-2 s-1) on a global map. In our current test dataset, we are using data from the CanESM5 model (historical and ssp245/ssp585 projections).These files can be easily accessed from CMIP6 for download via either ESGF (https://esgf-node.llnl.gov/projects/cmip6/) or Pangeo (https://gallery.pangeo.io/repos/pangeo-gallery/cmip6/intake_ESM_example.html). We will update the description

The crop yield emulators in yield_response_fcns are also updateable. However, the package is currently geared towards the AgMIP GGCMI Phase 2 style of emulators, with the emulation of the gridded crop model LPJmL included as the default. This emulator has a yield response function for corn, rice, soy and wheat.

The files in the growing_seasons folder were specifically used for a previous study, and they are the default files we use. They can however be updated for other growing season assumptions. Regarding the areamsk column, it was a record of pre-processing used in the creation of this file and is not actually used in the processing.

The files in gcamdata_files, mapping_data, and reference_agprodchange are essentially specific to the version of GCAM being used and do not change once the gcamdata system for that version has been built.

The specific problem this package solves is getting gridded yield information into the GCAM style files in a way that is efficient and reproducible. If the user does not have gridded yield information, then the emulator can produce that using climate data from cmip (or isimip). Response functions are generally available in the literature. Best practices would be to have responses for at least Corn, Wheat, Rice, and Soy although the commodity mapping the package takes can be updated to include other crops.

kthyng commented 1 year ago

Hi all! It looks like it's been awhile since activity on this issue. @jsun, @kauedesousa do you have blockers for continuing your reviews? Thanks all!

kauedesousa commented 1 year ago

I revised the code and paper and will wrap my review comments during the week.

jsun commented 1 year ago

@kauedesousa thanks for considering my comments. @kthyng, @martinfleis, I finished my review. Please consider accepting this software paper.

kauedesousa commented 1 year ago

Thanks @hahsan1 for addressing the issues that I rose in the paper. I think it is good to go.

kthyng commented 1 year ago

@martinfleis Can you wrap this up and then ping me when you're it's ready to accept?

martinfleis commented 1 year ago

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

[x] Double check authors and affiliations (including ORCIDs)
[x] Make a release of the software with the latest changes from the review and post the version number here. This is the version that will be used in the JOSS paper.
[x] Archive the release on Zenodo/figshare/etc and post the DOI here.
[x] Make sure that the title and author list (including ORCIDs) in the archive match those in the JOSS paper.
[x] Make sure that the license listed for the archive is the same as the software license.

Editor Tasks Prior to Acceptance

[x] Read the text of the paper and offer comments/corrections (as either a list or a PR)
[x] Check the references in the paper for corrections (e.g. capitalization)
[x] Check that the archive title, author list, version tag, and the license are correct
[x] Set archive DOI with @editorialbot set <DOI here> as archive
[x] Set version with @editorialbot set <version here> as version
[x] Double check rendering of paper with @editorialbot generate pdf
[x] Specifically check the references with @editorialbot check references and ask author(s) to update as needed
[x] Recommend acceptance with @editorialbot recommend-accept

martinfleis commented 1 year ago

@editorialbot generate pdf

martinfleis commented 1 year ago

@editorialbot check references

editorialbot commented 1 year ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1038/nclimate2470 is OK
- 10.1088/1748-9326/aac1c2 is OK
- 10.1371/journal.pone.0237918 is OK
- 10.5194/gmd-12-677-2019 is OK
- 10.5194/gmd-13-3995-2020 is OK
- 10.1029/2008GB003435 is OK
- 10.5334/jors.232 is OK
- 10.1002/2013EF000199 is OK
- 10.1088/1748-9326/ab639b is OK
- 10.1088/1748-9326/ac2965 is OK

MISSING DOIs

- None

INVALID DOIs

- None

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

martinfleis commented 1 year ago

@kauedesousa and @jsun, thank you for your reviews!

@hahsan1 We are now in the final phase of the review process. Now that bother reviewers suggested acceptance, please check the checklist generated above and let me know the version and DOI of the archived version. Thanks!

hahsan1 commented 1 year ago

@martinfleis I have completed the tasks in the checklist. I have archived the R package on zenodo. The latest version is v1.0.3. The DOI for it is 10.5281/zenodo.7938714 (direct link).

martinfleis commented 1 year ago

@editorialbot set v1.0.3 as version

editorialbot commented 1 year ago

Done! version is now v1.0.3

martinfleis commented 1 year ago

@editorialbot set 10.5281/zenodo.7938714 as archive

editorialbot commented 1 year ago

Done! archive is now 10.5281/zenodo.7938714

martinfleis commented 1 year ago

@editorialbot recommend-accept

editorialbot commented 1 year ago

Attempting dry run of processing paper acceptance...

editorialbot commented 1 year ago

Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1038/nclimate2470 is OK
- 10.1088/1748-9326/aac1c2 is OK
- 10.1371/journal.pone.0237918 is OK
- 10.5194/gmd-12-677-2019 is OK
- 10.5194/gmd-13-3995-2020 is OK
- 10.1029/2008GB003435 is OK
- 10.5334/jors.232 is OK
- 10.1002/2013EF000199 is OK
- 10.1088/1748-9326/ab639b is OK
- 10.1088/1748-9326/ac2965 is OK

MISSING DOIs

- None

INVALID DOIs

- None

martinfleis commented 1 year ago

Thanks @hahsan1! That is all from my side and the paper has been recommended for acceptance and handed over to the editor in chief for the final checks and publication.

editorialbot commented 1 year ago

:wave: @openjournals/ese-eics, this paper is ready to be accepted and published.

Check final proof :point_right::page_facing_up: Download article

If the paper PDF and the deposit XML files look good in https://github.com/openjournals/joss-papers/pull/4229, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept

kthyng commented 1 year ago

Hi @hahsan1! Here are my final steps for getting this out the door.

[x] Check that version was updated
[x] Check that software archive exists, has been input to JOSS, and title and author list look good
[ ] Check paper

kthyng commented 1 year ago

Paper looks good except, please check the capitalization in your references. You can preserve capitalization by placing {} around characters/words in your .bib file.

hahsan1 commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

hahsan1 commented 1 year ago

I've made changes to the .bib file as suggested. The changes don't seem to appear on the article proof I generated though.

hahsan1 commented 1 year ago

@editorialbot set main as branch

editorialbot commented 1 year ago

Done! branch is now main

hahsan1 commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

hahsan1 commented 1 year ago

@editorialbot generate pdf

editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

hahsan1 commented 1 year ago

ok it looks good now. I had to set the branch to the one I was making the changes to (main branch).

openjournals / joss-reviews

[REVIEW]: osiris: An R package to process climate impacts on agricultural yields for the Global Change Analysis Model #5226

Status

Reviewer instructions & questions

Checklists

Review checklist for @kauedesousa

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Review checklist for @jsun

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

Post-Review Checklist for Editor and Authors

Additional Author Tasks After Review is Complete

Editor Tasks Prior to Acceptance