Phydro merge into main #222
Draft
+5,859
−890
Conversation
This file contains 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
…fferent model flags
…Priestley-Taylor coefficient); scaling canopy conductance with LAI (calculated as -1/k*log(1-fapar) with k=0.5: Beer-Lambert coefficient for light extinction; multiplying canopy conductance with 2.0 - to be considered as a globally calibratable parameter.
…troduced fixed factor 2.0 for multiplication of LAI in surface conductance calculation.
Open
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)
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