-
Notifications
You must be signed in to change notification settings - Fork 239
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
Only write params with priors and/or data to config.xml #3125
Conversation
If this is a desirable PR, then I also would need to:
|
While I understand the disadvantages of the existing approach, and agree that things could be done to more easily keep the defaults more up-to-date, the decision to overwrite PFTs entirely was definitely done intentionally. If this approach is implemented I would strongly prefer that it be done as an optional flag rather than as the default behavior. |
I understand the motive behind overwriting all the parameters, but there's at least one case where this is having unintended consequences. For example at least |
Very few ED2 parameters are actually calculated based on other parameters. Most "calculations" are actually empirical correlations among traits that are hard coded into ED2 and in general those equations are old and ignore both uncertainties and correlations with other variables. The decision to avoid ED2's "calculations", and to handle these correlations within PEcAn, was very deliberate. I understand that some parameters only make sense in combination with specific flags. If individuals updated the default flags without updating parameters that's a genuine error. If one wants to use different flags from the default they should probably set that as a different model version and adjust the parameters & priors accordingly. |
This appears to be what happened. If ED version 2.2.0 or the github version of ED2 are used, the corresponding ED2IN file uses IALLOM = 3 and this bug (#3124) happens. The correct value of iallom3 <- function(rho) {
c14f15_ht_xx <- c(0.5709,-0.1007,0.6734)
b1Ht = c14f15_ht_xx[1] + c14f15_ht_xx[2] * log(rho)
b2Ht = c14f15_ht_xx[3]
return(c(b1Ht = b1Ht, b2Ht = b2Ht))
} I could try updating these two values for tropical PFTs in history.csv, or I could change IALLOM to the ED2 default (2) and change the values in history.csv to those defaults, OR I could revert IALLOM back to the legacy setting to match what's in history.csv. Any preference? |
If I wanted to use a different value for, say, IALLOM, how would I know which parameters in config.xml I would also have to change for it to work correctly? Would I have to look into the ED2 source code? Could I remove them from config.xml with PEcAn in some way, or would I have to write a script to edit all the config.xml files that PEcAn generates? I'm just trying to figure out how this is supposed to work. |
If you want to PEcAn to use a specific non-default value for an ED2 parameter, rather than sample it from a distribution, then you put them in the As for how you know what different modes do, in theory that should be in the ED2 wiki, but I don't think people have been keeping it updated as they've made changes. So yeah, you mostly end up looking at the code. |
…into dont-write-defaults
My work with |
Description
Testing out what happens when PEcAn.ED2 doesn't write default constants to config.xml. An extreme fix for #3124
Motivation and Context
Would simplify PEcAn.ED2 code a lot and be less likely to accidentally overwrite constants in ways that break ED2
Review Time Estimate
Types of changes
Checklist: