Skip to content

Conversation

@NickSzapiro-NOAA
Copy link

Address remaining CCPP physics warnings in ufs-community/ufs-weather-model#2703 of

.../ufs-weather-model/FV3/ccpp/physics/physics/Interstitials/UFS_SCM_NEPTUNE/sfcsub.F:3097:72:
 3097 |               data(imax-i+1,jj) = work(i,j)
Warning: 'jj' may be used uninitialized [-Wmaybe-uninitialized]

.../ufs-weather-model/FV3/ccpp/physics/physics/GWD/cires_ugwpv1_triggers.F90(301): warning #6717: This name has not been given an explicit type.   [TLIM_FGF]
          if (dtot >= tlim_fgf ) kex = kex+1

.../ufs-weather-model/FV3/ccpp/physics/physics/GWD/cires_ugwpv1_oro.F90(14): warning #6843: A dummy argument with an explicit INTENT(OUT) declaration is not given an explicit value.   [ZLWB]
                      zobl, zlwb, zogw, tau_ogw, dudt_ogw, dvdt_ogw,      &

ufs-weather-model regression tests pass bit-for-bit on hera. As for the changes:

  1. I'm mostly guessing at the "fix" for jj based on neighboring logic. I don't think any RTs exercise dlat > 0.0, dlon <= 0.0 for sfcsub.F
  2. tlim_fgf is defined above this in get_spectra_tau_nstgw but not in get_spectra_tau_okw
  3. Setting zlwb=0 is as in code and Issue 1984 fix: reduce warnings for ccpp-physics #149

tlim_fgf is defined above this in get_spectra_tau_nstgw but not in get_spectra_tau_okw
Fix warning of:
A dummy argument with an explicit INTENT(OUT) declaration is not given an explicit value.   [ZLWB]
Fix Warning: 'jj' may be used uninitialized [-Wmaybe-uninitialized]
@grantfirl
Copy link
Collaborator

@NickSzapiro-NOAA To speed up the UFS merge queue, I'm testing this combined with #295 on Hera. If successful, I'm going to combine this into that PR.

Copy link
Collaborator

@grantfirl grantfirl left a comment

Choose a reason for hiding this comment

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

I'm not an expert in the sfcsub.F code, but I would have done the same fix, so perhaps two people guessing are correct?

@grantfirl
Copy link
Collaborator

It's possible that @GeorgeGayno-NOAA might have a better idea if the changes to sfcsub.F are correct.

@grantfirl
Copy link
Collaborator

OK, the combined Hera RTs were successful except for a timeout for hafs_regional_atm_wav_intel, so I'm going to combine into #295 .

@GeorgeGayno-NOAA
Copy link

It's possible that @GeorgeGayno-NOAA might have a better idea if the changes to sfcsub.F are correct.

I am not too familiar with that routine, but your changes look ok. That appears to be a big error. What is the impact of the code fix on your tests?

@NickSzapiro-NOAA
Copy link
Author

There's no change from this in ufs-weather-model RTs (presumably as we don't get into this dlat > 0.0, dlon <= 0.0 logic). I don't know if UFS_UTILS has tests/datasets that do that

@GeorgeGayno-NOAA
Copy link

The sfcsub.F module is used by the UFS_UTILS global_cycle program. I made the code fix, recompiled global_cycle and reran the regression tests. One of the tests mimics how global_cycle is used in GFS v16 OPS. All tests passed. And that particular 'if' branch was never accessed. That routine appears to remap the input data so it has a consistent N/S E/W orientation. Apparently, we don't have any input data that requires that type of remapping. Otherwise, we would have discovered this bug sooner.

@rhaesung rhaesung merged commit ef272c1 into ufs-community:ufs/dev Jul 11, 2025
@grantfirl
Copy link
Collaborator

The sfcsub.F module is used by the UFS_UTILS global_cycle program. I made the code fix, recompiled global_cycle and reran the regression tests. One of the tests mimics how global_cycle is used in GFS v16 OPS. All tests passed. And that particular 'if' branch was never accessed. That routine appears to remap the input data so it has a consistent N/S E/W orientation. Apparently, we don't have any input data that requires that type of remapping. Otherwise, we would have discovered this bug sooner.

@GeorgeGayno-NOAA Thanks for your input/help!

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.

5 participants