-
Notifications
You must be signed in to change notification settings - Fork 312
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
Updates to facilitate FATES history variable overhaul #1542
Conversation
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.
Thanks for all of your work on this @adrifoster ! Let me know if you'd like to go through any testing stuff together.
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.
This looks great. Thank you for all your work on this.
For aux_clm tests on Cheyenne I got a DIFF for test ERS_D_Ld5.f09_g17.I2000Clm50FatesRs.cheyenne_intel.clm-FatesColdDef, ERS_D_Ld5.f10_f10_mg37.I2000Clm50Fates.cheyenne_intel.clm-FatesColdDef, ERS_Ld30.f45_f45_mg37.I2000Clm50FatesRs.cheyenne_intel.clm-FatesColdDefReducedComplexFixedBiogeo, and SMS_Ld5.f19_g17.I2000Clm50FatesRs.cheyenne_intel.clm-FatesColdDef. The DIFF seems to be coming from tests are in /glade/scratch/afoster/tests_1123-155830ch |
Further investigation showed that
Due to a line being commented out in CTSM |
@adrifoster per our conversation on slack and chat with @ekluzek, this DIFF is expected since the dimension hasn't yet been hooked up to the fates-side, so the values will be garbage. This should be fixed with reasonable values when we integrate NGEET/fates#752 after we integrate your PR. |
Just putting this here for future reference and as per @ekluzek 's suggestion, since the aim now is to wait to fix the aux_clm tests on Cheyenne that failed for DIFF and variables that were found using
fates tests on Cheyenne that failed for DIFF and variables that were found using
|
Description of changes
This is a set of changes that go along with a FATES PR intended to make the FATES history output more clear to users (new and otherwise), with standardized units and unit descriptions in the metadata, standardized naming conventions for how the variables are indexed, and more verbose long names. This also updates how we treat non-FATES columns in the upscaling to the grid-cell level output. This resolves FATES issue 630 and 535.
Specifically, almost all FATES history variable names have been updated with specific naming conventions, many of the units have been modified to be more standardized, and we update the flushing logic so that variables are flushed to
0.0_r8
on the FATES side, and the HLM ignore val (generally1E36
) on the host land model side.Updates to the CTSM code include:
We add a call to
flush_hvars
inside theinit_coldstart
andrestart
subroutines inclmfates_interfaceMod.F90
(here and here).Specific notes
Contributors other than yourself, if any: discussions with @billsacks, @glemieux, and @rgknox
Are answers expected to change (and if so in what way)?
These should not change the answers per se, but will result in factor differences between this update and master because of the units changes on the FATES code side. Also, all of the history field names have changed for FATES.
Any User Interface Changes (namelist or namelist defaults changes)?
The user_nl_clm
hist_fincl1
lists in the testmods directories are updated to reflect the new FATES history variable names.Testing performed, if any:
running aux_clm and fates suites against ctsm5.1.dev061