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.

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

is water_memory remembering correctly? #186

Closed
rgknox opened this issue Feb 21, 2017 · 7 comments
Closed

is water_memory remembering correctly? #186

rgknox opened this issue Feb 21, 2017 · 7 comments

Comments

@rgknox
Copy link
Contributor

rgknox commented Feb 21, 2017

Summary of Issue:

water memory remembers the water content of the top soil layer over the last 10 days. It is remembering the instantaneous water content at 0Z, which is not a great thing and needs to be fixed. The big problem I see is the order of operations in how it saves the water content to the array.

First it assigns index 1 from the boundary condition array.

Then it loops through indices 10 down to 2, assigning the values at those indices from what was in indices 9 to 1.

The problem is that when it is assigning the value to index 2, it is using the value from one which was over-written. I guess its not a huge deal, it is just weighting the first index double.

See phenolog() in EDPhysiologyMod.F90,

@rosiealice
Copy link
Contributor

rosiealice commented Feb 22, 2017 via email

@rgknox
Copy link
Contributor Author

rgknox commented Feb 22, 2017

I'm ok with keeping memory aspect of it if you are. I like the idea of using a lower layer as a surrogate for memory/smoothing etc, makes sense. I am a little wary of it only because if we are using different driver models or different layering schemes, the meaning of the third layer will get lost.

@rosiealice
Copy link
Contributor

rosiealice commented Feb 22, 2017 via email

@rgknox
Copy link
Contributor Author

rgknox commented Feb 22, 2017

sounds good to me, thanks Rosie

bandre-ucar added a commit that referenced this issue Mar 6, 2017
Merge branch 'rgknox-endrun-maxcohort-weights'

In this change-set roughly two objectives are tackled. 1) a wrapper
was created to call the CIME shutdown/end-run utitlities. The wrapper
is essentially the same wrapper that CLM uses, but we need our own to
break dependence. 2) We have various globals in FATES that are
dictated by the HLM, and vice versa. I tried to wrangle these
variables into FatesInterfaceMod, and give them some functions to set
those variables and protect them. Moreover, the new global
fates_maxElementsPerSite and fates_maxElementsPerPatch, are calculate
based on the maximum requirements for array allocation for FATES
restarts. Previously, this was a hard-coded and misleading
value. While FATES asks the HLM to allocate a "cohort" array for
restart variables, it is really a multi-purpose array, and may be
larger than the needs we have to store cohorts.

Fixes: #178 and #177 and #144 and #186
Addresses: #141

User interface changes?: No

Code review:

Testing:

  rgknox:

    Test suite: edTest, clm_short_45
    Test baseline: 8b2ca7e (PR #176)
    Test namelist changes: None
    Test answer changes: b4b with baseline
    Test summary: all PASS

  andre

    Test suite: ed - yellowstone gnu, intel, pgi
                     hobart nag
    Test baseline: cdb9db7
    Test namelist changes: none
    Test answer: bit for bit
    Test summary: all tests pass

    Test suite: clm_short - yellowstone gnu, intel, pgi
    Test baseline: clm4_5_12_r195
    Test namelist changes: none
    Test answer: bit for bit
    Test summary: all tests pass
@rgknox
Copy link
Contributor Author

rgknox commented Mar 7, 2017

Lets keep this issue open for the time being. While #180 did correct the indexing bug, this thread will help us to remember that we should continue to consider alternative ways of tracking available soil water memory.

I'm also wondering if maybe we should have a dedicated module in FATES that is in-charge of tracking and averaging input-like variables, one that accepts instantaneous HLM boundary conditions and decides how to average appropriately?

@rosiealice
Copy link
Contributor

rosiealice commented Mar 9, 2017 via email

@ckoven
Copy link
Contributor

ckoven commented Mar 18, 2019

going through old code, this can be closed.

@ckoven ckoven closed this as completed Mar 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants