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

Combination CCPP-related PR for ozone diagnostics, metadata intent bugfixes, sfcsub.F landmask bugfix, and canopy resistance output #2264

Merged
merged 27 commits into from
May 20, 2024

Conversation

grantfirl
Copy link
Collaborator

@grantfirl grantfirl commented May 3, 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 Intel+GNU RT suite (compared to current baselines) on either Hera/Derecho/Hercules
  • Commit 'test_changes.list' from previous step

Description:

This PR contains ccpp-physics changes to fix bugs:

and for more diagnostic output:

All of the above PRs were combined into one:

Replaces: #2253

Commit Message:

* UFSWM, CMEPS, FV3/ccpp_physics - CCPP Physics bugfixes for metadata intents and sfcsub.f landmask and output changes for ozone diagnostics and canopy resistance

Priority:

  • Normal

Git Tracking

UFSWM:

Sub component Pull Requests:

UFSWM Blocking Dependencies:

  • None

Changes

Regression Test Changes (Please commit test_changes.list):

  • PR Updates/Changes Baselines.

The changes to the ozone production/loss and mixing diagnostics will cause result changes for tests that output these terms. Any tests that use diag_additional_control_dtend or diag_additional_rap_dtend will be affected. See the test_changes.list file for specific tests.

Input data Changes:

  • None.

Library Changes/Upgrades:

  • No Updates

Testing Log:

  • RDHPCS
    • Hera
    • Orion
    • Hercules
    • Jet
    • Gaea
    • Derecho
  • WCOSS2
    • Dogwood/Cactus
    • Acorn
  • CI
  • opnReqTest (complete task if unnecessary)

@grantfirl grantfirl changed the title CPP combo_20240503 CCPP combo_20240503 May 6, 2024
@grantfirl grantfirl changed the title CCPP combo_20240503 Combination CCPP-related PR for ozone diagnostics, metadata intent bugfixes, sfcsub.F landmask bugfix, and canopy resistance output May 7, 2024
@grantfirl grantfirl marked this pull request as ready for review May 7, 2024 19:42
@grantfirl
Copy link
Collaborator Author

@jkbk2004 Apologies for the delay on this one. We ran into some RT failures related to NOAHMP as a component land model that we needed to fix. We're all good to go now.

@jkbk2004
Copy link
Collaborator

@jkbk2004 Apologies for the delay on this one. We ran into some RT failures related to NOAHMP as a component land model that we needed to fix. We're all good to go now.

@grantfirl sounds good!

@jkbk2004
Copy link
Collaborator

@grantfirl Can you sync up branches? We like to commit this pr today.

@grantfirl
Copy link
Collaborator Author

@grantfirl Can you sync up branches? We like to commit this pr today.

@jkbk2004 Finished

@BrianCurtis-NOAA
Copy link
Collaborator

Waiting on labels and a new bl_date I believe.

@BrianCurtis-NOAA
Copy link
Collaborator

There's still a review needed in ufs-community/ccpp-physics#205

@zach1221 zach1221 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 May 16, 2024
@grantfirl
Copy link
Collaborator Author

There's still a review needed in ufs-community/ccpp-physics#205

@BrianCurtis-NOAA We only need 1 for ccpp/physics PRs going into ufs/dev, so we should be good to go. However, this PR was assembled from 4 smaller PRs that were all reviewed and approved independently. I've reviewed the independent PRs, but cannot review my own combination.

@BrianCurtis-NOAA
Copy link
Collaborator

BrianCurtis-NOAA commented May 16, 2024

There's still a review needed in ufs-community/ccpp-physics#205

@BrianCurtis-NOAA We only need 1 for ccpp/physics PRs going into ufs/dev, so we should be good to go. However, this PR was assembled from 4 smaller PRs that were all reviewed and approved independently. I've reviewed the independent PRs, but cannot review my own combination.

There's still a review needed in ufs-community/ccpp-physics#205

@BrianCurtis-NOAA We only need 1 for ccpp/physics PRs going into ufs/dev, so we should be good to go. However, this PR was assembled from 4 smaller PRs that were all reviewed and approved independently. I've reviewed the independent PRs, but cannot review my own combination.

I agree, 1 is fine, but it seems there are revisions that are not from keeping up-to-date with auth repo (past the previous code review), and I would prefer all code is reviewed if possible prior to testing.

@zach1221 zach1221 removed the Ready for Commit Queue The PR is ready for the Commit Queue. All checkboxes in PR template have been checked. label May 16, 2024
@grantfirl
Copy link
Collaborator Author

There's still a review needed in ufs-community/ccpp-physics#205

@BrianCurtis-NOAA We only need 1 for ccpp/physics PRs going into ufs/dev, so we should be good to go. However, this PR was assembled from 4 smaller PRs that were all reviewed and approved independently. I've reviewed the independent PRs, but cannot review my own combination.

There's still a review needed in ufs-community/ccpp-physics#205

@BrianCurtis-NOAA We only need 1 for ccpp/physics PRs going into ufs/dev, so we should be good to go. However, this PR was assembled from 4 smaller PRs that were all reviewed and approved independently. I've reviewed the independent PRs, but cannot review my own combination.

I agree, 1 is fine, but it seems there are revisions that are not from keeping up-to-date with auth repo (past the previous code review), and I would prefer all code is reviewed if possible prior to testing.

Understood, and you're correct. I'll request re-review.

@grantfirl
Copy link
Collaborator Author

There's still a review needed in ufs-community/ccpp-physics#205

@BrianCurtis-NOAA We only need 1 for ccpp/physics PRs going into ufs/dev, so we should be good to go. However, this PR was assembled from 4 smaller PRs that were all reviewed and approved independently. I've reviewed the independent PRs, but cannot review my own combination.

There's still a review needed in ufs-community/ccpp-physics#205

@BrianCurtis-NOAA We only need 1 for ccpp/physics PRs going into ufs/dev, so we should be good to go. However, this PR was assembled from 4 smaller PRs that were all reviewed and approved independently. I've reviewed the independent PRs, but cannot review my own combination.

I agree, 1 is fine, but it seems there are revisions that are not from keeping up-to-date with auth repo (past the previous code review), and I would prefer all code is reviewed if possible prior to testing.

@BrianCurtis-NOAA Approval for ufs-community/ccpp-physics#205 has been achieved.

@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 May 16, 2024
@zach1221
Copy link
Collaborator

Testing is now complete. We can move to begin the merge process starting with ccpp-physics.

@zach1221
Copy link
Collaborator

@grantfirl fv3atm and noah-mp are merged. Please update submodule hashes and .gitmodule urls.
NOAA-EMC/fv3atm@10271c9
NOAA-EMC/noahmp@ec38ea3

@grantfirl
Copy link
Collaborator Author

Sure, although we still need NOAA-EMC/CMEPS#119 to be merged, right?

@zach1221
Copy link
Collaborator

Sure, although we still need NOAA-EMC/CMEPS#119 to be merged, right?

Correct. We've requested it. Feel free to wait for that one to be merged, so you can do them all at once.

@BrianCurtis-NOAA
Copy link
Collaborator

@grantfirl @zach1221 CMEPS has been merged: NOAA-EMC/CMEPS@2d837b1

@grantfirl
Copy link
Collaborator Author

@BrianCurtis-NOAA @zach1221 This is ready to go.

@BrianCurtis-NOAA
Copy link
Collaborator

I've cleaned up the description/commit message a bit.

@FernandoAndrade-NOAA FernandoAndrade-NOAA merged commit d55f1b8 into ufs-community:develop May 20, 2024
3 checks passed
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.

5 participants