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

History variable FATES_TRANSITION_MATRIX_LULU contains garbage for FatesColdDryDepSatPhen and FatesColdMeganSatPhen #2656

Closed
slevis-lmwg opened this issue Jul 22, 2024 · 9 comments
Assignees
Labels
bug something is working incorrectly external issue needs to be addressed elsewhere (submodule); issue here for the sake of project tracking FATES API update Changes to the FATES version that also REQUIRE an API change in CTSM

Comments

@slevis-lmwg
Copy link
Contributor

slevis-lmwg commented Jul 22, 2024

Yep, those cases are definitely garbage, but they've been that way for these two tests since that variable was introduced in ctsm5.2.013. Those values should be zero'd out. I'm not sure why the FatesColdSatPhen correctly zeros FATES_TRANSITION_MATRIX_LULU, but FatesColdDryDepSatPhen and FatesColdMeganSatPhen does not.

Originally posted by @glemieux in #2605 (comment)

@slevis-lmwg slevis-lmwg added the bug something is working incorrectly label Jul 22, 2024
@slevis-lmwg
Copy link
Contributor Author

I originally spotted the problem in #2605 as a strange difference from baseline in these two tests:

SMS_D.1x1_brazil.I2000Clm60FatesSpCruRsGs.derecho_gnu.clm-FatesColdDryDepSatPhen.GC.0719-175606de_gnu/SMS_D.1x1_brazil.I2000Clm60FatesSpCruRsGs.derecho_gnu.clm-FatesColdDryDepSatPhen.GC.0719-175606de_gnu.clm2.h0.2000-01-01-00000.nc.cprnc.out: RMS FATES_TRANSITION_MATRIX_LULU       Infinity            NORMALIZED    Infinity
SMS_D.1x1_brazil.I2000Clm60FatesSpCruRsGs.derecho_gnu.clm-FatesColdMeganSatPhen.GC.0719-175606de_gnu/SMS_D.1x1_brazil.I2000Clm60FatesSpCruRsGs.derecho_gnu.clm-FatesColdMeganSatPhen.GC.0719-175606de_gnu.clm2.h0.2000-01-01-00000.nc.cprnc.out: RMS FATES_TRANSITION_MATRIX_LULU       Infinity            NORMALIZED    Infinity

@glemieux glemieux changed the title History variable FATES_TRANSITION_MATRIX_LULU contains garbage History variable FATES_TRANSITION_MATRIX_LULU contains garbage for FatesColdDryDepSatPhen and FatesColdMeganSatPhen Jul 22, 2024
@glemieux glemieux changed the title History variable FATES_TRANSITION_MATRIX_LULU contains garbage for FatesColdDryDepSatPhen and FatesColdMeganSatPhen History variable FATES_TRANSITION_MATRIX_LULU contains garbage for FatesColdDryDepSatPhen and FatesColdMeganSatPhen Jul 22, 2024
@glemieux
Copy link
Collaborator

glemieux commented Jul 22, 2024

Checking the difference between FatesColdSatPhen testmod, which appropriately zeros FATES_TRANSITION_MATRIX_LULU, and these failing testmods, the only difference are the expected settings in shell_commands (i.e. CLM_BLDNML_OPTS set to '--drydep' or '--megan').

@glemieux
Copy link
Collaborator

glemieux commented Jul 22, 2024

This might simply be an initialization error on the fates side that is only showing up with gnu compiler tests. I noticed that the instances of FatesColdSatPhen that I've been checking are either intel or nvhpc. Testing.

@glemieux
Copy link
Collaborator

This might simply be an initialization error on the fates side that is only showing up with gnu compiler tests. I noticed that the instances of FatesColdSatPhen that I've been checking are either intel or nvhpc. Testing.

It looks like this is actually the case. Running gnu compiled version of the FatesColdSatPhen test results in the transition matrix reporting junk. I'll open an issue on the fates-side and cross reference here.

@glemieux
Copy link
Collaborator

This probably is being missed in more tests that are based on the Fates testmod as history output is being cleared and this output is not added.

@glemieux
Copy link
Collaborator

fates issue: NGEET/fates#1230

glemieux added a commit to rgknox/ctsm that referenced this issue Jul 24, 2024
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.
glemieux added a commit to rgknox/ctsm that referenced this issue Jul 24, 2024
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

Note that this will be resolved via NGEET/fates#1231, which will likely be fates tag sci.1.77.2_api.36.0.0. This tag could come in with #2594.

ekluzek added a commit to ekluzek/CTSM that referenced this issue Jul 24, 2024
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.

 Conflicts:
	bld/unit_testers/build-namelist_test.pl
slevis-lmwg added a commit to slevis-lmwg/ctsm that referenced this issue Jul 24, 2024
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.
@ekluzek ekluzek added the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Jul 25, 2024
samsrabin added a commit to samsrabin/CTSM that referenced this issue Jul 25, 2024
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.
adrifoster pushed a commit to adrifoster/CTSM that referenced this issue Jul 25, 2024
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.
adrifoster pushed a commit to adrifoster/CTSM that referenced this issue Jul 25, 2024
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.
glemieux added a commit to rgknox/ctsm that referenced this issue Jul 26, 2024
@glemieux
Copy link
Collaborator

This is fixed on fates main via sci.1.77.2_api.36.0.0.

@samsrabin samsrabin removed the next this should get some attention in the next week or two. Normally each Thursday SE meeting. label Aug 8, 2024
@samsrabin samsrabin added FATES API update Changes to the FATES version that also REQUIRE an API change in CTSM external issue needs to be addressed elsewhere (submodule); issue here for the sake of project tracking labels Aug 8, 2024
@glemieux
Copy link
Collaborator

glemieux commented Sep 5, 2024

This should be fixed by #2624

@glemieux glemieux closed this as completed Sep 5, 2024
samsrabin pushed a commit to samsrabin/CTSM that referenced this issue Sep 17, 2024
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.
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug something is working incorrectly external issue needs to be addressed elsewhere (submodule); issue here for the sake of project tracking FATES API update Changes to the FATES version that also REQUIRE an API change in CTSM
Projects
None yet
Development

No branches or pull requests

4 participants