Skip to content

Conversation

@glemieux
Copy link
Collaborator

@glemieux glemieux commented May 22, 2023

Description of changes

This PR should be coordinated with FATES pull request NGEET/fates#958. This PR makes updates to the FATES parameter file, changes the namespace of fates history outputs, and updates the fates dynamics driver.

Specific notes

The fates site-level history outputs associated with drought (e.g. FATES_DROUGHT_STATUS) have been converted to pft-level outputs. As such, the site-level variables have been removed from the bases Fates testmod user namelist and the pft-level variables have been added to the FatesAllVars test module.

A fix for #2043 was included here when it was rediscovered during testing.

Contributors other than yourself, if any: @rgknox

CTSM Issues Fixed (include github issue #): #2043

Are answers expected to change (and if so in what way)? Yes, due to expected scientific updates since the previous fates tag. There will also be FIELDLIST changes due to changing history output names.

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

Testing performed, if any: regular testing

glemieux added 6 commits May 17, 2023 09:48
These outputs have been converted to pft-level variables.  Given
that we avoid multiplexed outputs for the baseline fates these
are not replaced.
this has been removed with fates pull request 958
@rgknox
Copy link
Collaborator

rgknox commented Jun 26, 2023

This is straight forward, we may need to update the name of the parameter files because of an update, but this looks good

rgknox and others added 5 commits June 27, 2023 10:01
NEON fixes for TOOL and user-mods, add SP for NEON, some history file updates, black refactor for buildlib/buildnml

Merge the NEON fixes for TOOL and allowing SP mode, as well as a few simple history PR's, and a black reformat.

Fixes NEON bug identified at the NCAR-NEON workshop. Corrects the dominant PFT at TOOL site & usermods_dirs
Some small changes for quality of life improvements for the run_neon script. Some documentation and code
cleanup type changes regarding history code (delete a unused subroutine). Do a black reformat of python files
buildlib/buildnml (and CTSM SystemTests) for consistency across CESM. Also add running them through black in the python

Specific notes:

 - fixed a couple lines so that python will stop complaining about deprecated things
 - added print statements about warning messages being not an issue, and that building/running may take a while
 - added a "success" print statement - I'm not sure this will actually only print if "successful"
 - Add more documentation to history tape code
 - Add more breadcrumbs between related variables and methods
 - Put related history namelist flags/methods together
 - Update some out of date comments in history code
@glemieux
Copy link
Collaborator Author

Regression testing on cheyenne and izumi is underway.

Copy link
Collaborator

@rgknox rgknox left a comment

Choose a reason for hiding this comment

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

Looks good, assuming we will update the Externals pointer to the new fates tag once we integrate the fates-side changes.

@glemieux
Copy link
Collaborator Author

glemieux commented Jun 30, 2023

Testing aux_clm on cheyenne against ctsm5.1.dev129 in complete. All expected non-fates test are b4b. Test results on izumi are forthcoming.

@glemieux
Copy link
Collaborator Author

glemieux commented Jun 30, 2023

Testing aux_clm on izumi against ctsm5.1.dev129 is complete. All expected non-fates test are b4b. This includes a fix to #2043.

Folder location: /home/glemieux/scratch/ctsm-tests/tests_pr2009-Ld10_fix

This pull request should be ready to go.

@glemieux
Copy link
Collaborator Author

glemieux commented Jul 8, 2023

I updated the fates externals tag to include a bug fix. Re-testing using aux_clm against ctsm5.1.dev129 shows that all expected non-fates tests pass b4b as expected. All fates tests are diffs as expected due to science updates.

cheyenne: /glade/u/home/glemieux/scratch/ctsm-tests/tests_pr2009-auxclm-fates_66_1
izumi: /home/glemieux/scratch/ctsm-tests/tests_pr2009-fates_66_1

@ekluzek ekluzek self-assigned this Jul 10, 2023
@ekluzek ekluzek added FATES API update Changes to the FATES version that also REQUIRE an API change in CTSM PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete labels Jul 10, 2023
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.

Looks good. Glad you were able to figure this out and get it working.

I only have one comment that I'll make into an issue if it seems worth it.


do j = 1,nlevsoil
if(this%fates(nc)%bc_out(s)%active_suction_sl(j)) then
s_node = max(waterstatebulk_inst%h2osoi_vol_col(c,j)/soilstate_inst%eff_porosity_col(c,j) ,0.01_r8)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The 0.01_r8 magic number should likely be made a parameter. But, we should do that later since testing is already in. I'll look at this more carefully, and if I think it's worth doing turn it into an issue.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I made this into an issue in #2049.

@ekluzek ekluzek merged commit 4a868c1 into ESCOMP:master Jul 10, 2023
@ekluzek ekluzek deleted the fates-drdt-decid-updates branch July 10, 2023 05:31
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 PR status: ready PR: this is ready to merge in, with all tests satisfactory and reviews complete

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Five izumi NEON tests fail (for me) because the testnames include L10d instead of Ld10

3 participants