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

[production/RRFS.v1] Fix improperly assigned fire emissions for ebb_dcycle==1 for retrospectives (NOT operational!) #191

Merged

Conversation

jordanschnell
Copy link

For ebb_dcycle==1 (retrospective fire emissions), the 3-d emissions were incorrectly assgined assigned at all levels using the surface data.

This address Issue #188

@jordanschnell
Copy link
Author

Hi, to make sure I understand the PR process correctly.

  1. I was told to create the PR using production/RRFSv1 as the base branch - is this correct?
  2. I need to duplicate this PR for the ufs-weather-model and FV3 code as well?

Also, I am in the process of running the regressson tests.

@grantfirl
Copy link
Collaborator

Hi, to make sure I understand the PR process correctly.

  1. I was told to create the PR using production/RRFSv1 as the base branch - is this correct?
  2. I need to duplicate this PR for the ufs-weather-model and FV3 code as well?

Also, I am in the process of running the regressson tests.

@jordanschnell If this PR is supposed to go into the RRFS release branch code, then, yes, production/RRFS.v1 should have been the starting point.

There are similar production/RRFS.v1 branches for fv3atm and ufs-weather-model, so, yes, repeat the same process. Start from the production/RRFS.v1 branches, add any changes (which may just be submodule hash updates and .gitmodules updates), commit, push, and open PRs into those repos. Let me know if you have any other specific questions.

IIRC, the regression tests for the RRFS release branch are a subset of all RTs, so that should make it easier/quicker to test. If answers are expected to change (which, given your changes, they should), hopefully there have been discussions with the RRFS release folks that this is OK with them.

@grantfirl
Copy link
Collaborator

@MatthewPyle-NOAA FYI

@jordanschnell
Copy link
Author

Hi, to make sure I understand the PR process correctly.

  1. I was told to create the PR using production/RRFSv1 as the base branch - is this correct?
  2. I need to duplicate this PR for the ufs-weather-model and FV3 code as well?

Also, I am in the process of running the regressson tests.

@jordanschnell If this PR is supposed to go into the RRFS release branch code, then, yes, production/RRFS.v1 should have been the starting point.

There are similar production/RRFS.v1 branches for fv3atm and ufs-weather-model, so, yes, repeat the same process. Start from the production/RRFS.v1 branches, add any changes (which may just be submodule hash updates and .gitmodules updates), commit, push, and open PRs into those repos. Let me know if you have any other specific questions.

IIRC, the regression tests for the RRFS release branch are a subset of all RTs, so that should make it easier/quicker to test. If answers are expected to change (which, given your changes, they should), hopefully there have been discussions with the RRFS release folks that this is OK with them.

Thanks @grantfirl, to be clear, this will NOT change answers in the operational configuration which uses a different namelist option (ebb_dcycle == 2), here the change only impacts ebb_dcycle==1.

And I apologize but I'm not quite clear on the submodule has and .gitmodule update procedure. Is that just something along the lines of:

cd ufs-weather-model
git submodule update --remote --merge

cd FV3
git submodule update --remote --merge

What files are modified?

@grantfirl
Copy link
Collaborator

grantfirl commented Mar 28, 2024

Hi, to make sure I understand the PR process correctly.

  1. I was told to create the PR using production/RRFSv1 as the base branch - is this correct?
  2. I need to duplicate this PR for the ufs-weather-model and FV3 code as well?

Also, I am in the process of running the regressson tests.

@jordanschnell If this PR is supposed to go into the RRFS release branch code, then, yes, production/RRFS.v1 should have been the starting point.
There are similar production/RRFS.v1 branches for fv3atm and ufs-weather-model, so, yes, repeat the same process. Start from the production/RRFS.v1 branches, add any changes (which may just be submodule hash updates and .gitmodules updates), commit, push, and open PRs into those repos. Let me know if you have any other specific questions.
IIRC, the regression tests for the RRFS release branch are a subset of all RTs, so that should make it easier/quicker to test. If answers are expected to change (which, given your changes, they should), hopefully there have been discussions with the RRFS release folks that this is OK with them.

Thanks @grantfirl, to be clear, this will NOT change answers in the operational configuration which uses a different namelist option (ebb_dcycle == 2), here the change only impacts ebb_dcycle==1.

And I apologize but I'm not quite clear on the submodule has and .gitmodule update procedure. Is that just something along the lines of:

cd ufs-weather-model git submodule update --remote --merge

cd FV3 git submodule update --remote --merge

What files are modified?

ebb_dcycle

Do you know if any of the regression tests use ebb_dcycle==1?

For the upstream repo PRs, it goes something like this:

  1. Check out the production/RRFS.v1 branch of fv3atm.
  2. git checkout -b ebb_dcycle_fix - create a new branch from the RRFS release branch
  3. Assuming that you already have the latest commit of ebb_dcycle_fix branch of ccpp-physics checked out locally, let's make the FV3 repo point to it with git add ccpp/physics.
  4. edit the .gitmodules file so that it is pointing to your fork and branch of ccpp-physics:
    [submodule "ccpp/physics"]
    path = ccpp/physics
    #url = https://github.com/ufs-community/ccpp-physics
    #branch = production/RRFS.v1
    url = https://github.com/jordanschnell/ccpp-physics
    branch = ebb_dcycle_fix
  5. git commit -m 'describe the changes
  6. git push
  7. Open a PR in fv3atm pointing to the production/RRFS.v1 branch

Repeat for ufs-weather-model. So, the only files that should be modified (assuming that there are no other changes to the repos) are the .gitmodules file and the submodule hash (ccpp-physics for the FV3 PR and FV3 for the ufs-weather-model PR).

@jordanschnell
Copy link
Author

Thank you so much, @grantfirl !
And yes, by default, the regression tests use ebb_dcycle = 1

@jordanschnell
Copy link
Author

Looks like I was successful for fv3atm, NOAA-EMC/fv3atm#812

@MatthewPyle-NOAA MatthewPyle-NOAA changed the title Fix improperly assigned fire emissions for ebb_dcycle==1 for retrospectives (NOT operational!) [production/RRFS.v1] Fix improperly assigned fire emissions for ebb_dcycle==1 for retrospectives (NOT operational!) Apr 3, 2024
@jkbk2004
Copy link

Tests are done at ufs-community/ufs-weather-model#2214. This PR can be merged. @grantfirl @Qingfu-Liu @dustinswales can you merge?

@grantfirl grantfirl merged commit d889bae into ufs-community:production/RRFS.v1 Apr 11, 2024
3 checks passed
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