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

Initialize variables to avoid the warning message when compiling #753

Merged
merged 4 commits into from
Oct 1, 2024

Conversation

ShunLiu-NOAA
Copy link
Contributor

@ShunLiu-NOAA ShunLiu-NOAA commented Jun 7, 2024

Description

This PR is to initialize variables in observer_fv3reg.f90 to avoid the warning message when compiling.
The change will update regional EnKF excutable. However, the current regression test can't identify the changes in regional EnKF mode. The difference between before and after this update is that no warning message appears when compiling regional EnKF.

This PR is to address #751

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

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
  • New and existing tests pass with my changes
  • Any dependent changes have been merged and published

@RussTreadon-NOAA
Copy link
Contributor

We can use the regional enkf ctest in PR #750 to test the changes in this PR, right?

@hu5970
Copy link
Collaborator

hu5970 commented Jun 10, 2024

@RussTreadon-NOAA Yes, we can use the regional EnKF case from PR #750. But need to turn on the case manually.

@RussTreadon-NOAA
Copy link
Contributor

@RussTreadon-NOAA Yes, we can use the regional EnKF case from PR #750. But need to turn on the case manually.

Good. So we can test if initializing variables to zero has any impact ... at least in the context of the regional enkf ctest.

Copy link
Contributor

@RussTreadon-NOAA RussTreadon-NOAA left a comment

Choose a reason for hiding this comment

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

Thank you @ShunLiu-NOAA for making these changes. Looks good to me. Initializing variables to zero should not alter results, but it would be good to confirm this via the regional enkf ctest in PR #750 or an offline test.

Approve pending no impact result from ctest or offline test.

@ShunLiu-NOAA
Copy link
Contributor Author

@RussTreadon-NOAA Thank you for reviewing the changes. I will start an offline test.

@TingLei-NOAA
Copy link
Contributor

TingLei-NOAA commented Jun 12, 2024

Those added initialization are in "dummy " subroutines ( as shown by those lines

write(6,*)'this is a dummy subroutine, running this means something wrong ,stop'
   call stop2(555)

So, in my opinion, no need to do extra test to see if the will have impacts on the results if we are sure there are no changes outside of those dummy subroutines ( while the whole piece (observer_fv3reg.f90) is for those dummy subroutine)

@RussTreadon-NOAA RussTreadon-NOAA linked an issue Jun 13, 2024 that may be closed by this pull request
@RussTreadon-NOAA
Copy link
Contributor

@ShunLiu-NOAA , what is the status of this PR?

@RussTreadon-NOAA
Copy link
Contributor

@ShunLiu-NOAA , what is the status of this PR?

@ShunLiu-NOAA
Copy link
Contributor Author

Test project /scratch1/NCEPDEV/da/Shun.Liu/gsi_regression/GSI/build
Start 1: global_4denvar
Start 2: rtma
Start 3: rrfs_3denvar_rdasens
Start 4: hafs_4denvar_glbens
Start 5: hafs_3denvar_hybens
Start 6: global_enkf
1/6 Test #3: rrfs_3denvar_rdasens ............. Passed 859.46 sec
2/6 Test #6: global_enkf ...................... Passed 1298.20 sec
3/6 Test #5: hafs_3denvar_hybens .............. Passed 1403.31 sec
4/6 Test #2: rtma ............................. Passed 2170.92 sec
5/6 Test #4: hafs_4denvar_glbens .............. Passed 2186.12 sec
6/6 Test #1: global_4denvar ................... Passed 2886.83 sec

100% tests passed, 0 tests failed out of 6

image

@ShunLiu-NOAA
Copy link
Contributor Author

@hu5970 ctest on WCOSS and HERA passed. The PR is ready to merge into develop.

@RussTreadon-NOAA
Copy link
Contributor

Thank you @ShunLiu-NOAA for running ctests. My previous approval of this PR remains. These changes address a WCOSS Implementation Standard

  1. Code must compile without errors or warnings. Errors and warnings may not be suppressed, and the compiler warning level ("-W" options) must be at least the default one.

@hu5970 , is it possible for you to run the regional enkf ctest for the sake of completeness? Initializing variable to izero or zero should have no impact on regional enkf results. It would be nice to confirm this.

@ShunLiu-NOAA
Copy link
Contributor Author

@RussTreadon-NOAA Thank you. I will coordinate with @hu5970 for a regional enkf test.

@hu5970
Copy link
Collaborator

hu5970 commented Oct 1, 2024

I run a regression test suite with the regional EnKF case on and there are the results:

Test project /scratch1/BMC/wrfruc/mhu/code/gsi/GSI/build
    Start 1: global_4denvar
    Start 2: rtma
    Start 3: rrfs_3denvar_rdasens
    Start 4: hafs_4denvar_glbens
    Start 5: hafs_3denvar_hybens
    Start 6: rrfs_enkf_conv

1/6 Test #6: rrfs_enkf_conv ...................   Passed  303.82 sec
2/6 Test #3: rrfs_3denvar_rdasens .............   Passed  492.80 sec
3/6 Test #2: rtma .............................   Passed  1093.14 sec
4/6 Test #5: hafs_3denvar_hybens ..............   Passed  1109.30 sec
5/6 Test #4: hafs_4denvar_glbens ..............   Passed  1349.62 sec
6/6 Test #1: global_4denvar ...................   Passed  2028.61 sec

100% tests passed, 0 tests failed out of 6

Total Test time (real) = 2028.63 sec

@hu5970
Copy link
Collaborator

hu5970 commented Oct 1, 2024

I also updated the ctest instruction on how to run ctest with regional enkf case.

@hu5970 hu5970 merged commit 665f9b4 into NOAA-EMC:develop Oct 1, 2024
4 checks passed
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.

GSI for RRFS does not build without warning prints
4 participants