Skip to content
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.

Sign up for GitHub

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

Jump to bottom

PEcAn.ED2 defaults for allometry overwrite ED2 calculations and cause divide-by-zero errors #3124

Open
Aariq opened this issue Mar 1, 2023 · 3 comments · May be fixed by #3125
Open

PEcAn.ED2 defaults for allometry overwrite ED2 calculations and cause divide-by-zero errors #3124

Aariq opened this issue Mar 1, 2023 · 3 comments · May be fixed by #3125
Labels
Topic: models - ED Type: Bug

Comments

@Aariq
Copy link
Collaborator

Aariq commented Mar 1, 2023 •
edited

Bug Description

It appears that PEcAn.ED2 sets the ED2 flag NL%IALLOM = 3 in ED2IN by default. It also writes b1Ht and b2Ht to config.xml. BUT ED2 seems to want to calculate b1Ht and b2Ht from rho and some constants defined in ed_params.f90 when IALLOM = 3. I believe that config.xml gets the final say though and overwrites the calculations ED2 is doing. This could be a problem because b1Ht and b2Ht mean different things depending on the value for NL%IALLOM. The PEcAn.ED2 defaults for these parameters appear to be the ones for NL%IALLOM = 1, not 3

https://github.com/EDmodel/ED2/blob/b9c93f806d6df7a38db306d944a0695d8b40b39a/ED/src/init/ed_params.f90#L3660-L3664

Expected behavior

I'd expect PEcAn.ED2 to just not include b1Ht and b2Ht in config.xml. Anything not in config.xml will just use the ED2 defaults stored in ED. That would eliminate the need to keep an updated copy of those defaults in PEcAn.ED2. Either that, or use the default IALLOM mode (2).

Additional context

Getting divide by zero errors from ED2 and trying to eliminate potential sources. Allometry seems like a prime suspect.
@Aariq Aariq linked a pull request Mar 2, 2023 that will close this issue
Draft
13 tasks
@Aariq Aariq changed the title ED2 defaults for allometry overwrite ED2 calculations? ED2 defaults for allometry overwrite ED2 calculations Mar 2, 2023
@Aariq Aariq changed the title ED2 defaults for allometry overwrite ED2 calculations PEcAn.ED2 defaults for allometry overwrite ED2 calculations and cause divide-by-zero errors Mar 2, 2023
@Aariq
Copy link
Collaborator Author

Aariq commented Mar 2, 2023

Update. Running ED2 without PEcAn.ED2 setting ED2's defaults (i.e. letting ED2 use it's own internal defaults) has fixed all the errors I was getting in ED2 runs. I suspect most of the errors I have been getting have been a result of PEcAn.ED2 breaking allometry equations by overwriting coefficients with inappropriate values.

@Aariq
Copy link
Collaborator Author

Aariq commented Mar 3, 2023 •
edited

I can envision at least 3 possible solutions roughly in order of least to most control from PEcAn (and consequently, least to most difficulty in getting it right)

  1. Don't write any paramaters to config.xml that don't have priors. (i.e. what I did in Only write params with priors and/or data to config.xml #3125)
  2. Go through ED2 code and figure out which parameters mean different things or have different defaults depending on settings in ED2IN and don't write those to config.xml
  3. Write different defaults depending on the settings in ED2IN

I do think this is a true bug and one of these changes is needed for PEcAn.ED2 is to continue to be useful.

@Aariq
Copy link
Collaborator Author

Aariq commented Mar 6, 2023 •
edited

Another possibility is to make this work with the current machinery by:

  1. re-naming ED2IN.r2.2.0.github to ED2IN.rgit
  2. updating history.rgit so that the defaults there match the defaults that would be used by allometry equations in ED2.

Except I can't figure out how to edit the history.rgit file—it doesn't seem to correspond to internal ED2 defaults for several of the PFTs and only includes a subset of the parameters that could be overridden (e.g. missing is_conifer, is_savannah, and is_liana; has different values for is_grass from ED2 defaults). Is there a standard run that produces these outputs? Is there documentation somewhere as to what PEcAn PFTs are used to overwrite the ED2 defaults?

@Aariq Aariq linked a pull request May 25, 2023 that will close this issue
Only write params with priors and/or data to config.xml #3125
Draft
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Topic: models - ED Type: Bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Only write params with priors and/or data to config.xml Aariq/pecan
1 participant
@Aariq