PecanProject / pecan

The Predictive Ecosystem Analyzer (PEcAn) is an integrated ecological bioinformatics toolbox.
www.pecanproject.org
Other
200 stars 231 forks source link

Only write params with priors and/or data to config.xml #3125

Closed Aariq closed 1 month ago

Aariq commented 1 year ago

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:

Aariq commented 1 year ago

If this is a desirable PR, then I also would need to:

mdietze commented 1 year ago

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.

Aariq commented 1 year ago

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 b1Ht, b2Ht, and hgt_ref have different defaults (and meanings) depending on the choice of allometry equations set in ED2IN and in some cases these parameters don't have defaults, but are calculated from other parameters (rho) in ED2. There's more detail in #3124 as well as some ideas for alternative approaches to this PR. I'll keep this open as a draft for the time being if that's alright, as I'm using this version of PEcAn.ED2 to wrap up a project, at least until there is a better alternative.

mdietze commented 1 year ago

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.

Aariq commented 1 year ago

If individuals updated the default flags without updating parameters that's a genuine error.

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 b1Ht and b2Ht for tropical PFTs can be calculated from rho with this function adapted from ED2 code:

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?

Aariq commented 1 year ago

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.

mdietze commented 1 year ago

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 <constants> section of the pft config https://pecanproject.github.io/pecan-documentation/develop/xml-core-config.html#xml-pft

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.

Aariq commented 9 months ago

My work with PEcAn.ED2 is done, so you can close this if you'd like, but to be clear PEcAn.ED2 is currently broken without the changes in this PR (#3124), unless some other recent change has addressed this problem.