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

Fates next api uninitialized hlm_model_day fix #820

Merged
merged 10 commits into from
Dec 16, 2019

Conversation

glemieux
Copy link
Collaborator

@glemieux glemieux commented Oct 11, 2019

Description of changes

Specific notes

This change fixes an uninitialized variable hlm_model_day. It was found while trying to run in debug mode on Lawrencium, which builds with the intel compiler option -ftrapuv which initializes the variable to NaN. FatesHistoryInterfaceMod.F90 tries to reference the variable and as such, causes a floating invalid error. See also discussion from ESMCI/cime#3205.

Contributors other than yourself, if any: @rgknox

CTSM Issues Fixed (include github issue #): NGEET/fates#548

Are answers expected to change (and if so in what way)? Yes
Due to the structure of the init_coldstart procedure the model day will be set to one during the initialization (which is technically day zero), resulting in the cleafoff (and related variables) being off by one day compare to the baseline for the initialization step only. Howver, the cleafoffvalues for the timesteps greater than zero all match.

Any User Interface Changes (namelist or namelist defaults changes)? No

Testing performed, if any:
Comparision testing against CTSM baseline hash. All expected PASS except ERP_Ld9.f45_f45_mg37.I2000Clm50FatesCruGs.cheyenne_intel.clm-FatesAllVars due to changes mentioned above.
CTSM baseline hash: 9c12e40
Output location:

/glade/u/home/glemieux/scratch/clmed-tests/hlm-uninit-fix.fates.cheyenne.intel.Cbd70a67d-F393b9054

…nes. First appears to be not necessary and second screws up the cold start"

This reverts commit ac16068.
@glemieux glemieux marked this pull request as ready for review November 13, 2019 18:52
@glemieux glemieux added FATES API update Changes to the FATES version that also REQUIRE an API change in CTSM and removed PR status: work in progress labels Nov 13, 2019
Copy link
Collaborator

@ekluzek ekluzek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One small request in a comment. But, always good to move related lines of code into a subroutine or function. So this is a great change to get in.

src/utils/clmfates_interfaceMod.F90 Outdated Show resolved Hide resolved
@rgknox rgknox self-requested a review December 16, 2019 17:46
@glemieux glemieux requested a review from ekluzek December 16, 2019 21:44
@rgknox rgknox merged commit 978068a into ESCOMP:fates_next_api Dec 16, 2019
@glemieux glemieux deleted the fates_next_api-model-day-fix branch February 7, 2023 15:22
AGonzalezNicolas pushed a commit to HPSCTerrSys/clm5_0 that referenced this pull request Jun 27, 2024
Fates next api uninitialized hlm_model_day fix
AGonzalezNicolas pushed a commit to HPSCTerrSys/clm5_0 that referenced this pull request Jul 5, 2024
Fates next api uninitialized hlm_model_day fix
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FATES API update Changes to the FATES version that also REQUIRE an API change in CTSM
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants