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 history flushing #2594

Merged
merged 14 commits into from
Sep 5, 2024
Merged

fates history flushing #2594

merged 14 commits into from
Sep 5, 2024

Conversation

rgknox
Copy link
Collaborator

@rgknox rgknox commented Jun 10, 2024

Description of changes

FATES flushes history output arrays to the ignore value, and then zero's out values that are located on fates columns. The flushing need only happen once, since the non-fates columns aren't touched. And the zero'ing should happen on the FATES side of the code since this removes dependence on the interface code.

Specific notes

Contributors other than yourself, if any:

@samsrabin @glemieux

CTSM Issues Fixed (include github issue #):

Are answers expected to change (and if so in what way)?

Yes, brings in a FATES science update with small answer changes due to the fire weather refactor

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

No

Does this create a need to change or add documentation? Did you do so?

No

Testing performed, if any: standard fates testing

@glemieux glemieux self-requested a review July 5, 2024 16:25
Update submodule tags to pass runoff from cism to rof

- Update MOSART, CMEPS, and CISM so CISM runoff goes to ROF rather than CTSM
- Update RTM with fix needed for Paleo LGM work

Contributors:
@mvertens, @jedwards4b, @billsacks, @Katetc, @ekluzek, @slevis-lmwg

 Fixes ESCOMP#2590 Update CMEPS/MOSART/CISM/RTM tags
 Fixes ESCOMP/RTM#50 Likely wrong RTM river flux to MOM6 within cesm2_3_beta17

 Differences in namelist 'mosart_inparm':
  missing variable: 'do_rtmflood'
  missing variable: 'finidat_rtm'
  missing variable: 'frivinp_rtm'
  missing variable: 'rtmhist_fexcl1'
  missing variable: 'rtmhist_fexcl2'
  missing variable: 'rtmhist_fexcl3'
  missing variable: 'rtmhist_fincl1'
  missing variable: 'rtmhist_fincl2'
  missing variable: 'rtmhist_fincl3'
  missing variable: 'rtmhist_mfilt'
  missing variable: 'rtmhist_ndens'
  missing variable: 'rtmhist_nhtfrq'
  found extra variable: 'budget_frq'
  found extra variable: 'fexcl1'
  found extra variable: 'fexcl2'
  found extra variable: 'fexcl3'
  found extra variable: 'fincl1'
  found extra variable: 'fincl2'
  found extra variable: 'fincl3'
  found extra variable: 'finidat'
  found extra variable: 'frivinp'
  found extra variable: 'mfilt'
  found extra variable: 'mosart_euler_calc'
  found extra variable: 'mosart_tracers'
  found extra variable: 'ndens'
  found extra variable: 'nhtfrq'
  found extra variable: 'use_halo_option'

Changes answers
- what code configurations: mosart and rtm
- what platforms/compilers: all
- nature of change: mosart roundoff; rtm larger than roundoff due to bug fix; the latter also affects bgc variables

 We are ignoring strange diffs from baseline in two tests in variable
 FATES_TRANSITION_MATRIX_LULU as explained in issue ESCOMP#2656.
This moves the land use outputs from the baseline fates testmod into the
LUH2 base testmod
This moves one of the FatesColdSatPhen tests to a gnu compiler to
provide compiler coverage for issues like ESCOMP#2656.  This also adds an
nvhpc compiler test to the fates test suite.
@glemieux
Copy link
Collaborator

@ekluzek once regression testing for NGEET/fates#1231 is done (likely today), I could update this PR to use the new fates tag to address #2656. Assuming aux_clm testing goes well, this could be brought in to master pretty quickly I think.

@glemieux
Copy link
Collaborator

Running regression tests.

@glemieux
Copy link
Collaborator

@rgknox regression testing aux_clm and fates suites on derecho looks pretty good. Issue #2656 is confirmed fixed with this update.

For aux_clm, all non-fates testmods are b4b with one exception,ERP_P64x2_Lm13.f10_f10_mg37.IHistClm60Bgc.derecho_intel.clm-monthly--clm-matrixcnOn_ignore_warnings, which appears to be a known issue per #2619 (comment). There are fates FIELDLIST and NLCOMP diffs that are expected as I removed the LULU dimension output from the base Fates tesmod namelist. The AllVars testmod only has fill pattern differences for FATES_EXCESS_RESP, which I think is expected per history flushing PR NGEET/fates#1208, based on the update to that output's upfreq.

For the fates test suite, the FatesColdLUH2 tests failed RUN due a typo in the history output. I've since corrected this and kicked off rerun of just those tests. Otherwise the same diff types are seen as above in the aux_clm tests, although for the fill pattern only diffs there are more variables represented. Also, there is a BFAIL as I've added an nvhcp compiler version of the SatPhen testmod for more coverage, so it has nothing to compare against.

Test results:

  • /glade/derecho/scratch/glemieux/ctsm-tests/tests_pr2594-aux
  • /glade/derecho/scratch/glemieux/ctsm-tests/tests_pr2594-fates

This still needs testing on izumi

@rgknox rgknox added the FATES A change needed for FATES that doesn't require a FATES API update. label Aug 26, 2024
@glemieux
Copy link
Collaborator

Merging in latest master updates and rerunning tests on derecho and izumi.

Bring b4b-dev branch to main CTSM development.

- PLUMBER2 for ctsm5.2 datasets
- Last bit of PPE changes for namelist and parameter file settings
- Update run_sys_tests on Derecho for compiler jobs to run using 16 tasks
- Bring in a fix for dust emissions for coupling with CAM

Update cs.status parsing script to make expected BASELINE fails more obvious

Fix some issues with finding IC files for certain lnd_tuning_modes: all for cam7,
   clm5_0_cam6.0, and clm6_0_cam6.0
This brings fates up to the latest tag (fire weather refactor)
@glemieux
Copy link
Collaborator

glemieux commented Aug 30, 2024

Testing results on for aux_clm against ctsm5.2.027 on izumi are b4b with the expected exception of the fates testmods. The fates testmod diffs are consistent with the diffs per NGEET/fates#1215.

Location: /home/glemieux/scratch/ctsm-tests/tests_0829-105549iz

@glemieux
Copy link
Collaborator

glemieux commented Sep 4, 2024

Regression testing on derecho is complete. All expected non-fates tests pass B4B. There are expected NLCOMP differences due to updates to the fates testmod outputs per this PR. The fates DIFFs are consistent with NGEET/fates#1215 as mentioned above.

Location: /glade/u/home/glemieux/scratch/ctsm-tests/tests_pr2594-aux_clm

@ekluzek ekluzek self-assigned this Sep 4, 2024
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.

Nice improvement.

@ekluzek
Copy link
Collaborator

ekluzek commented Sep 5, 2024

On derecho the list of changes to baseline ctsm5.2.027 is:

ERP_Ld9.f45_f45_mg37.I2000Clm50FatesRs.derecho_intel.clm-FatesColdAllVars
ERP_P128x2_Ld30.f45_f45_mg37.I2000Clm60FatesSpCruRsGs.derecho_intel.clm-FatesColdSatPhen
ERP_P64x2_Lm13.f10_f10_mg37.IHistClm60Bgc.derecho_intel.clm-monthly--clm-matrixcnOn_ignore_warnings EXPECTED POSSIBILITY
ERS_D_Ld3_PS.f09_g17.I2000Clm50FatesRs.derecho_intel.clm-FatesCold
ERS_D_Ld5.f10_f10_mg37.I2000Clm50Fates.derecho_intel.clm-FatesCold
ERS_D_Mmpi-serial_Ld5.1x1_brazil.I2000Clm50FatesRs.derecho_gnu.clm-FatesCold
ERS_D_Mmpi-serial_Ld5.5x5_amazon.I2000Clm50FatesRs.derecho_gnu.clm-FatesCold
ERS_D_Mmpi-serial_Ld5.5x5_amazon.I2000Clm60FatesRs.derecho_intel.clm-FatesCold
ERS_Ld30.f45_f45_mg37.I2000Clm50FatesRs.derecho_intel.clm-FatesColdFixedBiogeo
ERS_Ld9.f10_f10_mg37.I2000Clm50FatesCruRsGs.derecho_intel.clm-FatesColdCH4Off
ERS_P128x1_Lm25.f10_f10_mg37.I2000Clm60Fates.derecho_intel.clm-FatesColdNoComp
SMS.f45_f45_mg37.I2000Clm60FatesSpRsGs.derecho_nvhpc.clm-FatesColdSatPhen
SMS_D_Ld5.f10_f10_mg37.I2000Clm45Fates.derecho_intel.clm-FatesCold
SMS_D_Ld5.f10_f10_mg37.I2000Clm50FatesRs.derecho_gnu.clm-FatesCold
SMS_D_Ld5.f10_f10_mg37.I2000Clm50FatesRs.derecho_intel.clm-FatesCold
SMS_D_Lm6_P256x1.f45_f45_mg37.I2000Clm50FatesRs.derecho_intel.clm-FatesCold
SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Fates.derecho_gnu.clm-FatesPRISM--clm-NEON-FATES-YELL
SMS_Ld5.f10_f10_mg37.I2000Clm45Fates.derecho_intel.clm-FatesCold
SMS_Ld5.f10_f10_mg37.I2000Clm50FatesRs.derecho_intel.clm-FatesCold
SMS_Ld5_PS.f19_g17.I2000Clm50FatesRs.derecho_gnu.clm-FatesCold

In those tests the only field that changes is: FATES_EFFECT_WSPEED

@ekluzek
Copy link
Collaborator

ekluzek commented Sep 5, 2024

On izumi the following tests compare different to baseline, but just for FATES_EFFECT_WSPEED

ERS_D_Ld5.f10_f10_mg37.I2000Clm50Fates.izumi_nag.clm-FatesCold
ERS_D_Mmpi-serial_Ld5.1x1_brazil.I2000Clm50FatesRs.izumi_nag.clm-FatesCold
ERS_Ld30.f45_f45_mg37.I2000Clm50FatesRs.izumi_intel.clm-FatesColdFixedBiogeo
SMS_D_Ld5.f10_f10_mg37.I2000Clm50FatesRs.izumi_nag.clm-FatesCold
SMS_D_Ld5.f45_f45_mg37.I2000Clm60Fates.izumi_nag.clm-FatesCold
SMS_D_Mmpi-serial_Ld5.5x5_amazon.I2000Clm60FatesRs.izumi_nag.clm-FatesCold
SMS_Ld10_D_Mmpi-serial.CLM_USRDAT.I1PtClm60Fates.izumi_nag.clm-FatesPRISM--clm-NEON-FATES-YELL
SMS_Ld5.f10_f10_mg37.I2000Clm45Fates.izumi_intel.clm-FatesCold
SMS_Ld5.f10_f10_mg37.I2000Clm50FatesRs.izumi_intel.clm-FatesCold

@ekluzek
Copy link
Collaborator

ekluzek commented Sep 5, 2024

OK, the changes to baseline are just due to the minimal changes that came in the fire weather refactor which just affects: FATES_EFFECT_WSPEED. I double checked that the FATES tag is correct and it is. And both aux_clm and fates baselines are in place, so this is good to go.

@ekluzek ekluzek merged commit b4199d8 into ESCOMP:master Sep 5, 2024
2 checks passed
@ekluzek ekluzek deleted the fates-hist-flush branch September 5, 2024 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FATES A change needed for FATES that doesn't require a FATES API update.
Projects
Status: Done (non release/external)
Status: Done
Development

Successfully merging this pull request may close these issues.

Add FATES_TRANSITIONS_MATRIX_LULU to FatesColdLUH2 testmods
3 participants