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

Fix code stability issue #1173 + Fix build 'remarks' PR #1201 #1210

Merged

Conversation

sbanihash
Copy link
Collaborator

@sbanihash sbanihash commented Mar 27, 2024

Pull Request Summary

This PR fixes issue #1173 reported by NCO during HAFS.v1 implementation regarding ww3_outp debug build failure as well as the addition of PR #1201 to address the fix for build remarks

Description

With these changes the debug build will not encounter a failure while running ww3_outp in addition to the elimination of some build remarks for WW3.

Please also include the following information:

  • Add any suggestions for a reviewer: @BijuThomas-NOAA @BinLiu-NOAA
  • Mention any labels that should be added: bug
  • Are answer changes expected from this PR? Please describe the changes and the reason why in addition to which of the following labels would apply: none

Issue(s) addressed

Commit Message

Please provide a short summary of this PR, which will be used during Squash and Merge and will be shown as a git log message. Be sure to add any co-authors here.

Check list

Testing

  • How were these changes tested?
    Four tests were conducted, with and without the fixes, and with Release and Debug build flags. The runs with and without the fixes are reproducible with the Release flag, which is expected. The run with the fix and debug flag fixes the issue with code instability while running ww3_outp. Runs without the fix and with the Debug flag reproduce the error that NCO had reported.
  • Are the changes covered by regression tests? (If not, why? Do new tests need to be added?)
    ww3_ufs1.1 was tested. ww3_outp was executed to reproduce the error and check if the bug fix has been effective.
  • Have the matrix regression tests been run (if yes, please note HPC and compiler)?
    parts of Matrix 12 , on Hera with intel compiler

@MatthewMasarik-NOAA
Copy link
Collaborator

Thanks for this fix PR, @sbanihash. @JessicaMeixner-NOAA or I will be back in touch this afternoon.

@JessicaMeixner-NOAA
Copy link
Collaborator

@BinLiu-NOAA and @BijuThomas-NOAA can one of you approve that this has been tested in HAFS and is good to be merged?

@sbanihash - @MatthewMasarik-NOAA will be making a PR to develop with the fixes that are not already there.

Copy link
Collaborator

@MatthewMasarik-NOAA MatthewMasarik-NOAA left a comment

Choose a reason for hiding this comment

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

Code review
Pass - the changes cleanly address the issues.

Testing
Pass - the updates from #1201 were tested in our NCEP regtest matrix. @sbanihash and @BijuThomas-NOAA has confirmed the testing for the fix addressing #1173.

Approved.

@MatthewMasarik-NOAA
Copy link
Collaborator

@BijuThomas-NOAA, thank you for bringing this to our attention. Thanks @sbanihash, @BijuThomas-NOAA for providing a nice fix.

@MatthewMasarik-NOAA MatthewMasarik-NOAA merged commit 68456de into NOAA-EMC:production/hafs.v2 Mar 27, 2024
11 of 12 checks passed
@MatthewMasarik-NOAA
Copy link
Collaborator

Congratulations 🎉 @sbanihash on your first commit to noaa-emc/ww3!

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.

4 participants