JGCRI / hectordata

Prepare and format the inputs for hector.
Other
0 stars 1 forks source link

Generate input tables #7

Closed kdorheim closed 4 years ago

kdorheim commented 4 years ago

@crvernon and @bpbond this is a larger PR than I would have liked and plan on doing smaller PRs in the future. However I felt a PR of this size was needed to give an idea of the package structure.

The objective here is to create the ability to generate Hector input csv files and ini files for the CMIP6 scenarios with a minimal effort from a user. Right now it is quite a hassle and has been a reoccurring problem for the RCMIP and hector calibration work. So this should be something that is ideally easy for any user to use and makes our lives easier when we have to generate new scenarios in the future.

In this PR there are helper functions that would be useful for developers/advanced users that are trying to generate their own hector inputs. But I think that the average Hector user would be interacting with generate function, which would allow users to generate the Hector csv inputs and ini files (not implemented yet) that are canonical aka the RCP, SPPs, and DECK scenarios.

If you have comments, concerns, or would like to chat before looking over this PR please let me know. I look forward to working with the both of you on this and hearing your feedback. Thank you very much!

kdorheim commented 4 years ago

@bpbond and @crvernon the tests are failing on git hub actions because I don't have the rpackageutils importing properly for the tests. Do you want me to try and work that out before of after you take a look at the PR?

bpbond commented 4 years ago

@kdorheim Re failing tests, no worries for now, thanks. Will look at this shortly.

kdorheim commented 4 years ago

@bpbond thanks for the suggestions @crvernon whenever you have a chance to take a look at this that would be great!

kdorheim commented 4 years ago

Some functions duplicate quite a bit of functionality for different constraints. For example, from R/generate_fxns.R the generate_input_tables function could be broken down into (1) a function that processes a constraint being passed, and (2) a function that uses function (1) to process each constraint and then return your tables. This allows you to reduce the size of your codebase, reduce the possibility for error since you only have to make changes to a block of code when needed, and allow you to write succinct tests that target specific functionality.

hmmm this is true and what I was aiming to do 🙈... Which is why convert_rcmipCMIP6_hector is separate from save_hector_table and wrapped inside generate_input_tables hmmm.. it is a tad confusing though I'll go back to the drawing board to try to stream some of the code. Thanks!

kdorheim commented 4 years ago

@crvernon and @bpbond thanks for taking a look at this! I really appreciate your input, I'm struggling with getting package checks to pass because of a dependency with assertthat. But as soon as that passes I'm going to merge this and start working on the functions that will be used to set up the ini files.

codecov-commenter commented 4 years ago

Codecov Report

Merging #7 into master will increase coverage by 38.52%. The diff coverage is 93.06%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master       #7       +/-   ##
===========================================
+ Coverage   54.54%   93.06%   +38.52%     
===========================================
  Files           1        3        +2     
  Lines          11      101       +90     
===========================================
+ Hits            6       94       +88     
- Misses          5        7        +2     
Impacted Files Coverage Δ
R/generate_fxns.R 92.85% <92.85%> (ø)
R/helper_fxns.R 93.02% <93.02%> (ø)
R/convert_RCMIP.R 93.33% <93.33%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e9b6ea1...b6295ab. Read the comment docs.