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

Dynamics Refactor #176

Merged
merged 13 commits into from
Feb 2, 2017
Merged

Dynamics Refactor #176

merged 13 commits into from
Feb 2, 2017

Conversation

rgknox
Copy link
Contributor

@rgknox rgknox commented Jan 13, 2017

Removed usage of CLM variables from the dynamics portion of the code. 24hour average boundary conditions were added to the bc_in structure. Timing information was moved from udata to FatesGlobals. ldomain(g)%area was removed from spitfire code. The wrapper to litter flux was removed and the contents was added to the dynamics wrapper.

Fixes: none
User interface changes?: none
Code review: requesting @rosiealice among others

Test suite: lawrencium lr3, edTest
Test baseline: d116008
Test namelist changes: none
Test answer changes: none, b4b

Test summary: all PASS, including baseline comparisons

rgknox and others added 12 commits January 6, 2017 16:05
…s the calculation of model-day for phenology.
…placing it with bc_in. Temperature_inst for all calls but fire are also purged. Compiles and runs, not regression tested.
…. ldomain%area(g) is still called in fire model, as well as masterproc and use_spitfire.
…arameters passed during initialization. Tracked down other instances of use, and also fixed calls to clm version of r8 or iulog when found.
…he globals at the top of FatesInterfaceMod. master had a slightly different variable name, while the head had added cp_masterproc.
…on of boundary conditions was using numPatchesPerCohort while master changed that to maxPatchesPerCohort.
…wrapper. Essentially it was just a wrapper that appeared only once within another wrapper.
@rgknox
Copy link
Contributor Author

rgknox commented Jan 13, 2017

@rosiealice, can you review my changes to the fire model, specifically the removal of the grid area?

…oc variable, while master had the use_fates_plant_hydro variable, both added to same section. Both are valid in the merge.
@rgknox
Copy link
Contributor Author

rgknox commented Jan 17, 2017

Resolved conflicts from merging master. All tests PASS.

@rgknox
Copy link
Contributor Author

rgknox commented Jan 24, 2017

From email discussion with @rosiealice:
"Hi Ryan,

This is OK if the ignition rate that we use is per unit area. In that case, the number of fires is proportional to the size of the grid cell, and so everything drops out. If we accidentally made number of ignitions on a per grid cell basis, that wouldn't be true. But I can't really see why we would do that. At the moment the number of ignitions is sort of a placeholder anyway.

So, yes, I think this is OK, particularly if it cleans things up. "

I'm going to assign @bandre-ucar to start testing on Yellowstone if I don't hear anything else by tomorrow.

@rgknox
Copy link
Contributor Author

rgknox commented Jan 24, 2017

Assigning @bandre-ucar for testing.

@bandre-ucar
Copy link
Contributor

Testing today.

@bandre-ucar
Copy link
Contributor

ed - hobart nag : passed
clm_short - yellowstone gnu, intel, pgi : passed
ed - yellowstone gnu, intel, pgi : problem with baselines, rerunning.

@bandre-ucar bandre-ucar merged commit 8b2ca7e into NGEET:master Feb 2, 2017
bandre-ucar added a commit that referenced this pull request 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
@bandre-ucar bandre-ucar removed their assignment Apr 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants