Skip to content

fix issue with using compset longname in cesm testing#1

Merged
alperaltuntas merged 3 commits into
ESCOMP:dev/ncarfrom
jedwards4b:fix/string_lengths
Aug 6, 2024
Merged

fix issue with using compset longname in cesm testing#1
alperaltuntas merged 3 commits into
ESCOMP:dev/ncarfrom
jedwards4b:fix/string_lengths

Conversation

@jedwards4b
Copy link
Copy Markdown
Collaborator

Description
This PR adjusts string lengths to be longer than 128 characters and
modifies the get_time_string function to allow for the use of % signs in
cesm test names. This allows us to run tests with MOM using the cesm
testing mechanism with long compset names.

Fixes # (issue)

How Has This Been Tested?
I ran test ERS.ne30pg3_t232.1850_CAM60_CLM50%BGC-CROP_CICE_MOM6_MOSART_DGLC%NOEVOLVE_SWAV.derecho_intel.allactive-defaultio

This test will fail in mom - first because the filename string length was hardcoded to 128 characters and second because fms assumes that the % sign will not be used in filenames.

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • New check tests, if applicable, are included
  • make distcheck passes

Comment thread diag_manager/diag_util.F90 Outdated
CALL get_date(current_time, yr1, mo1, dy1, hr1, mi1, sc1)
len = LEN_TRIM(filename)
first_percent = INDEX(filename, '%')
do i=1,9
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

In this loop instead of just looking for a % sign we look for %\d where \d is a digit 1-9. This allows other uses of the % sign in the filename.

@jedwards4b jedwards4b requested a review from alperaltuntas July 30, 2024 19:21
Comment thread diag_manager/diag_util.F90 Outdated
Comment on lines +2180 to +2185
write(first_fms_percent,"('%',i1)") i
first_percent = INDEX(filename, first_fms_percent)
if (first_percent > 0) then
exit
endif
enddo
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't think this will find the index of first percent of the string xyz%8f%2g%1300%400 correctly; could you do something like

first_percent = INDEX(filename, '%1')
do i=2,9
  write(first_fms_percent,"('%',i1)") i 
  first_percent_loc = INDEX(filename, first_fms_percent)
  if (first_percent_loc > 0) then
    if (first_percent == 0) then
      first_percent = first_percent_loc
    else
      first_percent = min(first_percent, first_percent_loc)
  endif
enddo

Also, are you sure you don't need to capture %0?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Oh, you pushed changes while I was typing that :) I think this is what you want in find_first_fms_percent()

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I don't understand your loop - the one I wrote appears to work correctly now.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I see now, you are correct - I will update the PR. Thanks

Copy link
Copy Markdown

@mnlevy1981 mnlevy1981 left a comment

Choose a reason for hiding this comment

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

This looks good to me; I'd like to see test suite output before approving it (I assume it'll be bit-for-bit with aux_mom / pr_mom, but it would be nice to verify). I should have time to run those tests in the next day or two

Copy link
Copy Markdown

@mnlevy1981 mnlevy1981 left a comment

Choose a reason for hiding this comment

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

I ran aux_mom and the baseline comparisons all passed (except we still have a B1850MOM compset in the test list and for some reason create_newcase failed on the ERS.TL319_t061_wt061.GMOM_JRA_WD.derecho_intel). The test cases are all in /glade/derecho/scratch/mlevy/tests/aux_mom/cime6.1.0-cesm_cice6_5_0_12-mi_240705_newfms.intel; I'm seeing a bunch of NLCOMP failures that point to diag_table, which makes me wonder if there's a problem with NLCOMP because nothing changed in the generation of diag_table:

14	FAIL
	7	NLCOMP (FAIL)
      1 DIMCS_Ld1.TL319_t232.GMOM_JRA.derecho_intel.mom-cfc_mods--mom-debug
      1 ERI.TL319_t232.GMOM_JRA.derecho_intel.mom-debug
      1 ERS.T62_t061.GMOM_JRA.derecho_intel.mom-debug
      1 PET.TL319_t232.CMOM_JRA.derecho_intel
      1 SMS_D.TL319_t232.GMOM_JRA_RYF.derecho_intel
      1 SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel
      1 SMS.T62_t025.CMOM.derecho_intel
	2	COMPARE (FAIL)
      1 ERI.TL319_t232.GMOM_JRA.derecho_intel.mom-debug
      1 PET.TL319_t232.CMOM_JRA.derecho_intel
	1	MEMCOMP (FAIL)
      1 ERI.TL319_t232.GMOM_JRA.derecho_intel.mom-debug
	1	RUN (FAIL)
      1 DIMCS_Ld1.TL319_t232.GMOM_JRA.derecho_intel.mom-cfc_mods--mom-debug
	1	BASELINE (FAIL)
      1 PET.TL319_t232.CMOM_JRA.derecho_intel
	2	CREATE_NEWCASE (FAIL)
      1 ERS.TL319_t061_wt061.GMOM_JRA_WD.derecho_intel
      1 SMS_Ld2.f09_t232.B1850MOM.derecho_intel.mom-bcompset

(the DIMCS_Ld1 test ran out of walltime, but something like 10 or 11 of the scaling tests finished and all looked good)

From the SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel TestStatus.log file:

2024-07-31 11:34:04: NLCOMP
Comparison failed between '/glade/derecho/scratch/mlevy/tests/aux_mom/cime6.1.0-cesm_cice6_5_0_12-mi_240705_newfms.intel/SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_113353_yhxrk0/CaseDocs/diag_table' with '/glade/derecho/scratch/mlevy/baselines/aux_mom/cime6.1.0-cesm_cice6_5_0_12-mi_240705/SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel/CaseDocs/diag_table'
Inequivalent lines 'MOM6 diagnostic fields table for CESM case: SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_070336_6vi8sd' != 'MOM6 diagnostic fields table for CESM case: SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_113353_yhxrk0'
  NORMALIZED: 'MOM6 diagnostic fields table for CESM case: SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_070336_6vi8sd' != 'MOM6 diagnostic fields table for CESM case: SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_113353_yhxrk0'
Inequivalent lines 'SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_070336_6vi8sd.mom6.h.rho2%4yr-%2mo',                        1,  'days',   1, 'days',   'time', 1, 'days' != 'SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_113353_yhxrk0.mom6.h.rho2%4yr-%2mo',                        1,  'days',   1, 'days',   'time', 1, 'days'
  NORMALIZED: 'SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_070336_6vi8sd.mom6.h.rho2%4yr-%2mo',                        1,  'days',   1, 'days',   'time', 1, 'days' != 'SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_113353_yhxrk0.mom6.h.rho2%4yr-%2mo',                        1,  'days',   1, 'days',   'time', 1, 'days'
Inequivalent lines 'SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_070336_6vi8sd.mom6.h.native%4yr-%2mo',                      1,  'days',   1, 'days',   'time', 1, 'days' != 'SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_113353_yhxrk0.mom6.h.native%4yr-%2mo',                      1,  'days',   1, 'days',   'time', 1, 'days'
  NORMALIZED: 'SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_070336_6vi8sd.mom6.h.native%4yr-%2mo',                      1,  'days',   1, 'days',   'time', 1, 'days' != 'SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_113353_yhxrk0.mom6.h.native%4yr-%2mo',                      1,  'days',   1, 'days',   'time', 1, 'days'
Inequivalent lines 'SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_070336_6vi8sd.mom6.h.z%4yr-%2mo',                           1,  'days',   1, 'days',   'time', 1, 'days' != 'SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_113353_yhxrk0.mom6.h.z%4yr-%2mo',                           1,  'days',   1, 'days',   'time', 1, 'days'
  NORMALIZED: 'SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_070336_6vi8sd.mom6.h.z%4yr-%2mo',                           1,  'days',   1, 'days',   'time', 1, 'days' != 'SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_113353_yhxrk0.mom6.h.z%4yr-%2mo',                           1,  'days',   1, 'days',   'time', 1, 'days'
Inequivalent lines 'SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_070336_6vi8sd.mom6.h.static',                               -1, 'days',   1, 'days',   'time' != 'SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_113353_yhxrk0.mom6.h.static',                               -1, 'days',   1, 'days',   'time'
  NORMALIZED: 'SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_070336_6vi8sd.mom6.h.static',                               -1, 'days',   1, 'days',   'time' != 'SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_113353_yhxrk0.mom6.h.static',                               -1, 'days',   1, 'days',   'time'
Inequivalent lines 'ocean_model_rho2', 'volcello','volcello','SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_070336_6vi8sd.mom6.h.rho2%4yr-%2mo', 'all', 'mean', 'none', 1 != 'ocean_model_rho2', 'volcello','volcello','SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_113353_yhxrk0.mom6.h.rho2%4yr-%2mo', 'all', 'mean', 'none', 1
  NORMALIZED: 'ocean_model_rho2', 'volcello','volcello','SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_070336_6vi8sd.mom6.h.rho2%4yr-%2mo', 'all', 'mean', 'none', 1 != 'ocean_model_rho2', 'volcello','volcello','SMS_Ld40.TL319_t232.CMOM_JRA.derecho_intel.GC.20240731_113353_yhxrk0.mom6.h.rho2%4yr-%2mo', 'all', 'mean', 'none', 1

So I think we have some work to do before aux_mom is useful again, but all in all this PR looks fine. Should someone from the CAM side run some tests as well?

@jedwards4b
Copy link
Copy Markdown
Collaborator Author

It looks like the normalize mechanism should strip the GC.20240731_070336_6vi8sd part of the file name but isn't doing it.

@alperaltuntas alperaltuntas merged commit 5a030fc into ESCOMP:dev/ncar Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants