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.

Sign up for GitHub

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

Jump to bottom

Initialize variables to avoid the warning message when compiling #753

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Initialize variables to avoid the warning message when compiling #753

wants to merge 2 commits into from

Conversation

ShunLiu-NOAA
Copy link
Contributor

@ShunLiu-NOAA ShunLiu-NOAA commented Jun 7, 2024 •
edited
Loading

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.

RussTreadon-NOAA
RussTreadon-NOAA approved these changes Jun 11, 2024
View reviewed changes
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 •
edited
Loading

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
GSI for RRFS does not build without warning prints #751
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RussTreadon-NOAA RussTreadon-NOAA RussTreadon-NOAA approved these changes

@TingLei-NOAA TingLei-NOAA TingLei-NOAA approved these changes

Assignees

@hu5970 hu5970

@ShunLiu-NOAA ShunLiu-NOAA

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

GSI for RRFS does not build without warning prints
4 participants
@ShunLiu-NOAA @RussTreadon-NOAA @hu5970 @TingLei-NOAA