Phydro merge into main #222
Draft
+5,995
−953
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merged
Previous commit (34b6531) added 'vwind' to forcing, to respect the testing with continuous integration this should have included updating of the - vignettes/*.Rmd - tests/testthat/*.R - and documentation Points 1 and 2 can be tested locally with devtools::build_vignettes() and with devtools::test() Ideally, this should be enforced by CI also on branches.
Previous commit (34b6531) added 'vwind' to forcing, to respect the testing with continuous integration this should have included updating of the - data/*.rda This is now done in hindsight.
Previous commit (b6ab2d4) modified model output by multiplying surface conductance by a factor of two. This change in model output must be reflected by updating the reference values in the regression tests. Ideally this would be done in the same commit that alters the model structure to have a clear record of which commit changes model behavior in what way. This is now done in hindsight.
This reverts commit f0441b5.
Phydro: make test pass (except for 1 (issue #289)) and updated documentation, vignettes and example data sets due to changes in: - input parameters (vwind) - model computation (factor x2 surface conductance)
* update * update * update * Homogenize whitespace * Revert removal of data files * Update tests, vignette, and documentation for gw change Previous commits introduced a new input parameter gw_calib. To respect the testing with continuous integration any change should include updating of the - vignettes/*.Rmd - tests/testthat/*.R - and documentation - data/*.rda Ideally this would be done in the same commit as the change to the model itself. Note that steps 1 and 2 can be tested locally with devtools::build_vignettes() and with devtools::test() * Fix phydro bug by requiring `gw_calib` * Use single Makevars as in branches 'main' and 'luluc' * Remove daily and night rad output * Remove daily and night rad output from tests * Reduce whitespace changes by this PR * Add gw_calib also to vignettes * Fix gw_calib in examples of documentation * Include gw_calib in sensitivity analysis --------- Co-authored-by: Fabian Bernhard <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the discussion thread for planning the merge of the
phydrobranch into the main model.TODO before the merge:
rsofundocumentation paper as isTODO for the merge:
leis not equal toaet#220rsofun::p_model_driverswithreadRDS(file = here::here("data/p_model_drivers_newformat.rds")), as well asrsofun::p_model_drivers_vcmax25forcing,forcing_24h,forcing_daytime,forcing_3hrmaxfor the driverswhcfrom aparams_modlto a site parameterrunread_pmodel_f.Rthat sends the multi-sitedata.frameto individual simualtions?Other suggestions:
run_pmodel_f_bysite()need to be an exported (and documented) function? Or will it anyway always be called fromrunread_pmodel_f, and we can thus keep it internal (and undocumented)?parallelform therunread_pmodel_f()andrunread_biomee_f()functions since it is equivalent with settingncores = 1