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

A MERRA2 aerosol interpolation bug was found by RRFS #2082

Merged
merged 18 commits into from
Jan 16, 2024
Merged

A MERRA2 aerosol interpolation bug was found by RRFS #2082

merged 18 commits into from
Jan 16, 2024

Conversation

AnningCheng-NOAA
Copy link
Contributor

@AnningCheng-NOAA AnningCheng-NOAA commented Jan 9, 2024 •
edited by BrianCurtis-NOAA
Loading

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

Commit Message

-fix array out of bounds issue in physics aerinterpol.F90 (ufs-community/ccpp-physics#152)

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)

  • AQM
  • CDEPS
  • CICE
  • CMEPS
  • CMakeModules
  • FV3
  • GOCART
  • HYCOM
  • MOM6
  • NOAHMP
  • WW3
  • stochastic_physics
  • none

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:
    New baselines are needed for all conus_13km related tests. Namely:
    conus13km_2threads conus13km_debug conus13km_debug_decomp conus13km_decomp conus13km_restart
    conus13km_control conus13km_debug_2threads conus13km_debug_qr conus13km_radar_tten_debug conus13km_restart_mismatch
FAILED REGRESSION TESTS

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

@AnningCheng-NOAA
Copy link
Contributor Author

Here is the log file for the regression tests. All test with use_Merra2=T and iaer=1011 passed. A few tests related conus13k failed with use_Merra2=F and iaer=1011 options, which does not make sense, because iaer asks for reading MERRA2 data, but use_Merra2 turn off the data access.

RegressionTests_hera.log

@AnningCheng-NOAA
Copy link
Contributor Author

use_merra2 is overwritten by the regional*.IN and rrfs*IN files in directory fv3_conf. use_merra2=T was dded in conus13km_control to avoid confusion.

The issue of the failed tests is related to the bug fixed. A simple tests is done by adding the following line in /scratch1/NCEPDEV/global/Anning.Cheng/ufs-weather-model_wk/FV3/ccpp/physics/physics/MP/Morrison_Gettelman/aerinterp.F90
429 if(prsl(j,L) == aerpres(j,k)) write(,)"RRFS prsl=aerpres"
and the "RRFS prsl=aerpres" is shown in /scratch1/NCEPDEV/stmp2/Anning.Cheng/FV3_RT/rt_176886/conus13km_control_intel/out.
So the bug fixed does modify the baseline results.

On the whole, The regression tests in Hera are successful and the code is in good shape to be merged into the trunk . New baselines are needed in all conus_13km related RT tests.

@AnningCheng-NOAA AnningCheng-NOAA mentioned this pull request Jan 10, 2024
Merged
@jkbk2004
Copy link
Collaborator

@AnningCheng-NOAA can you sync up the branch? we will start working on this pr once the branches are synced up.

@AnningCheng-NOAA
Copy link
Contributor Author

AnningCheng-NOAA commented Jan 12, 2024 via email

@jkbk2004
Copy link
Collaborator

@AnningCheng-NOAA I synced up CICE/MOM6/Stochastic-physics hashes. Files changed look ok now. Can you confirm?

@AnningCheng-NOAA
Copy link
Contributor Author

AnningCheng-NOAA commented Jan 12, 2024 via email

@jkbk2004 jkbk2004 added 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. labels Jan 12, 2024
@jkbk2004
Copy link
Collaborator

@BrianCurtis-NOAA @zach1221 @FernandoAndrade-NOAA This pr is ready. conus 13km cases change.

@jkbk2004
Copy link
Collaborator

@AnningCheng-NOAA I updated commit messages above. Please, feel free to add or change the commit messages.

@DeniseWorthen
Copy link
Collaborator

@AnningCheng-NOAA You should update the FV3 hash in your UWM PR branch now that you merged the CI change.

@AnningCheng-NOAA
Copy link
Contributor Author

AnningCheng-NOAA commented Jan 12, 2024 via email

@DeniseWorthen
Copy link
Collaborator

In your UWM branch, you just need to pull in the update to FV3 and then add it.

You should see (git status) something like this

modified:   FV3 (new commits, untracked content)

so git add then git commit.

Make sure the FV3 hash in your UWM branch is 1042921

@AnningCheng-NOAA
Copy link
Contributor Author

AnningCheng-NOAA commented Jan 12, 2024 via email

@jkbk2004
Copy link
Collaborator

All tests are done. We can start merging process.

@jkbk2004 jkbk2004 mentioned this pull request Jan 16, 2024
Merged
@jkbk2004
Copy link
Collaborator

@AnningCheng-NOAA can you update fv3 hash? correct one is NOAA-EMC/fv3atm@095a1d5

@AnningCheng-NOAA
Copy link
Contributor Author

AnningCheng-NOAA commented Jan 16, 2024 via email

@jkbk2004
Copy link
Collaborator

jkbk2004 commented Jan 16, 2024 •
edited
Loading

@DusanJovic-NOAA we have one thing to clear on fv3 develop commit. @AlexanderRichert-NOAA committed NOAA-EMC/fv3atm@138aab1 and @AnningCheng-NOAA synced against that hash. But My question is if it's ok to bring in the change. @BrianCurtis-NOAA @zach1221 @FernandoAndrade-NOAA FYI

@DusanJovic-NOAA
Copy link
Collaborator

@DusanJovic-NOAA we have one thing to clear on fv3 develop commit. @AlexanderRichert-NOAA committed NOAA-EMC/fv3atm@138aab1 and @AnningCheng-NOAA synced against that hash. But My question is if it's ok to bring in the change. @BrianCurtis-NOAA @zach1221 @FernandoAndrade-NOAA FYI

Sorry, what exactly is the issue with NOAA-EMC/fv3atm@138aab1, what do we need to clear?

@jkbk2004
Copy link
Collaborator

@DusanJovic-NOAA we have one thing to clear on fv3 develop commit. @AlexanderRichert-NOAA committed NOAA-EMC/fv3atm@138aab1 and @AnningCheng-NOAA synced against that hash. But My question is if it's ok to bring in the change. @BrianCurtis-NOAA @zach1221 @FernandoAndrade-NOAA FYI

Sorry, what exactly is the issue with NOAA-EMC/fv3atm@138aab1, what do we need to clear?

No issue but to make sure we are synching with the hash.

@jkbk2004 jkbk2004 merged commit 1492141 into ufs-community:develop Jan 16, 2024
@grantfirl grantfirl mentioned this pull request Mar 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zach1221 zach1221 zach1221 approved these changes

@DusanJovic-NOAA DusanJovic-NOAA DusanJovic-NOAA approved these changes

@SadeghTabas-NOAA SadeghTabas-NOAA Awaiting requested review from SadeghTabas-NOAA

@BrianCurtis-NOAA BrianCurtis-NOAA Awaiting requested review from BrianCurtis-NOAA

@FernandoAndrade-NOAA FernandoAndrade-NOAA Awaiting requested review from FernandoAndrade-NOAA

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug and Proposed Solution in MERRA Aerosol Code
7 participants
@AnningCheng-NOAA @jkbk2004 @DeniseWorthen @DusanJovic-NOAA @zach1221 @FernandoAndrade-NOAA @BrianCurtis-NOAA