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

User Guide responses #10

Open
jrising opened this issue Jan 14, 2025 · 4 comments
Open

User Guide responses #10

jrising opened this issue Jan 14, 2025 · 4 comments

Comments

@jrising
Copy link

jrising commented Jan 14, 2025

openjournals/joss-reviews#7538

These comments refer to the User Guide vignette (https://jgcri.github.io/gaia/articles/vignette.html).

  • What is the CMIP-ISIMIP format? I am not familiar with this data format: please provide a reference to a specification (also include this in example 2). Also, I wouldn't use raw CMIP data-- one needs downscaled data. Do you mean the protocol data produced for GGCMI?
  • Sacks et al., (2020) link is broken. Also, that paper is from 2010, not 2020.
  • Sacks et al. reports day-of-year, not month-level crop calendars (as referred in the text). While the Waldhoff paper derives predictors from month-level data, much other work starts with daily data, so presumably you don't want to drop to month-level.
  • The user guide should include installation instructions.
  • It is very odd that you are translating data to ".txt format". This isn't even a format-- the extension just suggests that the file has some kind of text encoding.
  • Why do you recommend the use of WATCH climate observations? This is a very old dataset, based on an outdated ERA model, and you only need temperature and precipitation anyway.
  • It is odd that Example 1 under Run gaia! uses both Example 1 and Example 2 from the climate data section. If both are required, then these are not separate examples.
  • Run gaia! Example 2 refers to weighted climate data from Example Climate Data Example 2, but Example Climate Data Example 2 does not refer to weighted data.
  • It seems like Run gaia! Example 3 is just more information on the API and diagnostic outputs. It should be presented as such.
@jrising
Copy link
Author

jrising commented Jan 15, 2025

  • output_dir <- file.path(getwd(), 'gaia_output')
    : This is identical to output_dir <- 'gaia_output'.
  • Most research that might want to use gaia will not be at the country level. Is this possible?
  • weighted_climate: What happens if the weather data does not cover all countries?
  • There is a printed error in the user guide under weighted_climate.
  • crop_calendars: You don't explain what this function does. I think it outputs the crop calendars-- possibly both as a return value and to the output directory?
  • You say that new crops can be added to the crop calendar under the existing format. I think you mean that one should add an additional column, filled with 0s, and then additional rows with 1 in that column for each country, and then fill in plan and harvest. It's not clear from the example of plant and harvest months are 1-based or 0-based. It's also not clear where this should be added-- maybe you mean that crop_calendars produces the default crop_calendar, but one wouldn't run it if one has a modified crop calendar with additional crops?
  • Table 3: Why does the output produced by data_aggregation have a separate yield for each month? What is grow_season? I can guess the others, but they should be defined. Why does Table 3 have temp and precip if the weather data is intended for table 4?
  • Table 4: What are the min and max calculated over? Is it over mean monthly temperature and total monthly precip?
  • yield_regression: Report the default yield regression used here.
  • Report the meaning of the output values for yield_shock_projection. I'm guessing it's the ratio of projected yields in each year to the projected yield in the base year.

@jrising
Copy link
Author

jrising commented Jan 15, 2025

General comments:

  • Based on the User Guide, this strikes me as more as a repo repository than a tool. It is not clear that the provided functions can be used for a different project, and we are only given examples that are part of a single workstream.
  • Although the guide mentions in multiple places that inputs can be changed-- as long as they follow the same format-- the user guide does not particularly facilitate. The formats aren't provided, and it's often not clear how changes at one point will affect things later on. Someone just has to follow the single workstream and hope for the best.
  • The regression uses a log dependent variable, so a weighted average isn't theoretically justified. It's okay at higher resolutions, but at the country level, you end up with a log-sum-exp problem.
  • It should be clarified that projected climate data needs to be BCSD relative to the same observational dataset used in the regression.
  • Standard errors are reported in the regression, but I don't see how uncertainty is handled. Ignoring it wouldn't be consistent with the regression results.

@mengqi-z
Copy link
Collaborator

@jrising - Thank you for your comments! We are working on addressing the issues you've highlighted and preparing responses to some questions you've raised. Thanks for the time and effort you put into reviewing our work!

@jrising
Copy link
Author

jrising commented Jan 16, 2025

One more point: I was trying to follow the vignette, but the second example download (to https://zenodo.org/records/13976521/files/gaia_example_climate.zip) has timed out on me 5 times. I suggest you offer an option for people to download the files outside of that function, or reduce the size.

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

No branches or pull requests

2 participants