-
Notifications
You must be signed in to change notification settings - Fork 92
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
dynamic numpft, dimension interface, removal of edecophyscon #262
Conversation
… Anyway, had to resolve a conflict.
…ace, for some reason it was confusing as an argument passed to nc_io routines.
… For static allocations, we now use maxpft exclusively. A check exists to see if the dynamic numpft is lower than max. All loops are now on numpft (dynamic parameter in FatesInterfaceMod).
…ndices. Exending the zeroing range to include maxpft instead of numpft where appropriate.
@@ -11,6 +11,7 @@ module EDTypesMod | |||
save | |||
|
|||
integer, parameter :: maxPatchesPerSite = 10 ! maximum number of patches to live on a site | |||
integer, parameter :: maxCohortsPerPatch = 160 ! maximum number of cohorts per patch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I reading this right that the "maxCohortsPerPatch = 160" is added back in in place of the dynamic version (maxCohortsPerPatch = nclmax * numpft_ed * nlevleaf ) ? Is that to stop something breaking?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @rosiealice ,
This parameter is now independent, and really doesn't have to equal nclmax * numpft_ed * nlevleaf. The previous reason it had to, was because we needed to allocate space in the restart files to accomodate the maximum of the number of cohorts and that multiple.
We have a variable in FatesInterfaceMod called maxElementsPerPatch = max(maxCohortsPerPatch,nclmax * numpft_ed * nlevleaf ) that is used to achieve that end.
The user is free to reduce or increase maxCohortsPerPatch as they see fit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look good to me. I also checked the history output and the new version of the scpf dimension looks good in those files. Glad to be rid of edecophyscon! Thanks ryan.
sweet, merging |
This changeset introduces a group of enhancements and improvements mostly related to how the number of PFTs is set in FATES.
Previously, we had a constant parameter that was supposed to mach what exists in the parameter file. Now, the number of PFT's is diagnosed from the parameter file. There are many local data structures in the model that want to allocate pft space, thus the constant parameter "maxpft" was retained and used in these cases. Some minor adjustments were made to the code to make sure that these arrays are fully initialized to zero, this was not previously a problem, but could present future problems if not initialized properly.
The special FATES dimensions that are sent to the HLM history file were cleaned up a little bit and moved their allocations, specifications and call to a more appropriate part of the interface. The HLM is still expected to use the FATES structures explicitly, without a generic calling function. But at least they are in the FatesInterfaceMod.F90 now, which is the file where fates data structures used externally should be housed.
EDecophyscon was redundant with EDPftvarcon_inst, and was removed.
A special check was also added to the control parameter interface, that forces FATES to check if the number of patches it specifies per site, is larger than the memory allocation that the HLM has provided (ie natveg_pft-1).
This PR required interface changes and is coupled with fates-clm PR 20: https://github.com/NGEET/fates-clm/pull/20
Code review: pending
Test suite: ed, clm_short_45, clm_short_50
Test namelist changes: none
Test answer changes: B4B except for changes in the output dimension, which is expected (output dimensions went from multiples of maxpft to numpft.
Test summary:
lawrencium(intel): all expected PASS
cheyenne(intel): all expected PASS