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

Post update #2113

Merged
merged 16 commits into from
Feb 8, 2024
Merged

Post update #2113

merged 16 commits into from
Feb 8, 2024

Conversation

JiliDong-NOAA
Copy link
Contributor

@JiliDong-NOAA JiliDong-NOAA commented Jan 23, 2024

Commit Queue Requirements:

  • Fill out all sections of this template.
  • All sub component pull requests have been reviewed by their code managers.
  • Run the full RT suite (compared to current baselines) on either Hera/Derecho/Hercules AND have committed the log to my PR branch.
  • Add list of all failed regression tests in "Regression Tests" section.

PR Information

Description

This PR will update UPP to the latest commit. With the updated UPP, this PR will fix the missing reflectivity bug when using NSSL MP and inline post together by adding nssl mp to microphysics options when assigning model reflectivity.

Commit Message

UPP will be brought to the commit to fix the missing reflectivity bug for NSSL MP cases.

Priority

  • Critical Bugfix (This PR contains a critical bug fix and should be prioritized.)
  • High (This PR contains a feature or fix needed for a time-sensitive project (eg, retrospectives, implementations))
  • Normal

Blocking Dependencies

Git Issues Fixed By This PR

Changes

Subcomponent (with links)

Input data

  • No changes are expected to input data.
  • Changes are expected to input data:
    • New input data.
    • Updated input data.

Regression Tests:

  • No changes are expected to any regression test.
  • Changes are expected to the following tests:
FAILED REGRESSION TESTS

regional_wofs_intel 063

rrfs_v1nssl_intel 077

rrfs_v1nssl_nohailnoccn_intel 078

grib2 output will be changed by this PR for tests with NSSL microphysics and inline post combined together

regional_wofs_intel 063

rrfs_v1nssl_intel 077

rrfs_v1nssl_nohailnoccn_intel 078

Libraries

  • Not Needed
  • Needed
    • Create separate issue in JCSDA/spack-stack asking for update to library. Include library name, library version.
    • Add issue link from JCSDA/spack-stack following this item

Testing Log:

  • RDHPCS
    • Hera
    • Orion
    • Hercules
    • Jet
    • Gaea
    • Derecho
  • WCOSS2
    • Dogwood/Cactus
    • Acorn
  • CI
    • Completed
  • opnReqTest
    • N/A
    • Log attached to comment

@JiliDong-NOAA
Copy link
Contributor Author

RegressionTests_hera.log

@JiliDong-NOAA JiliDong-NOAA marked this pull request as draft January 24, 2024 20:33
@JiliDong-NOAA
Copy link
Contributor Author

need more time to investigate the failed RT

@JiliDong-NOAA JiliDong-NOAA marked this pull request as ready for review January 29, 2024 17:31
@JiliDong-NOAA
Copy link
Contributor Author

After further investigation, the failed RTs are expected with the baseline changes from:

  1. REFZI and REFZR are switched to missing values as expected, which is similar to what Thompson microphysics scheme does
  2. missing value are handled differently for reflectivity when switching to the correct block: previousy missing values will be reset to dbzmin and they are now kept as is.

@jkbk2004
Copy link
Collaborator

jkbk2004 commented Feb 6, 2024

@JiliDong-NOAA What is your timeline to commit this PR? If there is any urgency waiting for this PR, we will give a priority.

@JiliDong-NOAA
Copy link
Contributor Author

@JiliDong-NOAA What is your timeline to commit this PR? If there is any urgency waiting for this PR, we will give a priority.

I don't have a timeline for this PR, but it will still be good to get it committed soon as this PR is essentially a bug fix.

@BrianCurtis-NOAA
Copy link
Collaborator

There are still requests waiting in your NOAA-EMC/fv3atm#774 PR. It would be helpful to know why this is a critical bugfix.

@JiliDong-NOAA
Copy link
Contributor Author

There are still requests waiting in your NOAA-EMC/fv3atm#774 PR. It would be helpful to know why this is a critical bugfix.

I just:

  1. synced the PR with develop
  2. updated FV3 request by adding explanation and slides to PR template
  3. created issues for ufs-weather-model and FV3 and referred in PR

Here is the slides showing reflectivity before and after the fix.

missing_refl_forPR.pptx

Please let me know what else I need to do.

@BrianCurtis-NOAA
Copy link
Collaborator

Thanks Jili! The Hera log you have attached to the PR is just an xml formatted file and the Hera log you committed to the PR is still in a merge conflict state.

The commited log shows the following:

FAILED TESTS: 
063 regional_wofs_intel failed in check_result
regional_wofs_intel 063 failed in run_test
077 rrfs_v1nssl_intel failed in check_result
rrfs_v1nssl_intel 077 failed in run_test
078 rrfs_v1nssl_nohailnoccn_intel failed in check_result
rrfs_v1nssl_nohailnoccn_intel 078 failed in run_test

Could you update the top post with the names of the tests and not the numbers of them? I can accept the committed log as an example of what tests fail for now.

@JiliDong-NOAA
Copy link
Contributor Author

Thanks Jili! The Hera log you have attached to the PR is just an xml formatted file and the Hera log you committed to the PR is still in a merge conflict state.

The commited log shows the following:

FAILED TESTS: 
063 regional_wofs_intel failed in check_result
regional_wofs_intel 063 failed in run_test
077 rrfs_v1nssl_intel failed in check_result
rrfs_v1nssl_intel 077 failed in run_test
078 rrfs_v1nssl_nohailnoccn_intel failed in check_result
rrfs_v1nssl_nohailnoccn_intel 078 failed in run_test

Could you update the top post with the names of the tests and not the numbers of them? I can accept the committed log as an example of what tests fail for now.

PR template updated with failed RT tests name added.

@FernandoAndrade-NOAA FernandoAndrade-NOAA added the Baseline Updates Current baselines will be updated. label Feb 7, 2024
@zach1221
Copy link
Collaborator

zach1221 commented Feb 7, 2024

@JiliDong-NOAA We're ready to begin testing this PR. Can you please sync up your branch?

@JiliDong-NOAA
Copy link
Contributor Author

@JiliDong-NOAA We're ready to begin testing this PR. Can you please sync up your branch?

Done

@zach1221 zach1221 added the Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked. label Feb 8, 2024
@zach1221
Copy link
Collaborator

zach1221 commented Feb 8, 2024

Testing is complete. Let's move to merge the fv3atm sub-pr.

@zach1221
Copy link
Collaborator

zach1221 commented Feb 8, 2024

@JiliDong-NOAA fv3atm sub-pr is merged. Please update submodule hash and revert .gitmodule url.
NOAA-EMC/fv3atm@9dec0fa

@JiliDong-NOAA
Copy link
Contributor Author

@JiliDong-NOAA fv3atm sub-pr is merged. Please update submodule hash and revert .gitmodule url. NOAA-EMC/fv3atm@9dec0fa

.gitmodules reverted and FV3 updated

@zach1221 zach1221 merged commit b4c59bb into ufs-community:develop Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Baseline Updates Current baselines will be updated. Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

missing reflectivity when using NSSL microphysics, Cu and inline post
6 participants