Skip to content

Bring local Noah-MP snowmelt to output & add accumulated snowmelt output#1707

Merged
weiwangncar merged 2 commits intowrf-model:release-v4.4from
cenlinhe:add_QMELT
Mar 29, 2022
Merged

Bring local Noah-MP snowmelt to output & add accumulated snowmelt output#1707
weiwangncar merged 2 commits intowrf-model:release-v4.4from
cenlinhe:add_QMELT

Conversation

@cenlinhe
Copy link
Contributor

@cenlinhe cenlinhe commented Mar 23, 2022

Bring local snowmelt variable in Noah-MP to output variable and add accumulated snowmelt output in diagnostics

TYPE: enhancement

KEYWORDS: snowmelt, Noah-MP, output

SOURCE: Cenlin He (NCAR)

DESCRIPTION OF CHANGES:
Add two new output variables for Noah-MP package for snowmelt

ASSOCIATED REPOSITORY CHANGE:
NCAR/noahmp#42

LIST OF MODIFIED FILES: list of changed files (use git diff --name-status master to get formatted list)
M Registry/Registry.EM_COMMON
M Registry/registry.noahmp
M dyn_em/module_first_rk_step_part1.F
M phys/module_diag_misc.F
M phys/module_diagnostics_driver.F
M phys/module_surface_driver.F
M phys/noahmp

@cenlinhe cenlinhe requested review from a team as code owners March 23, 2022 22:49
@cenlinhe
Copy link
Contributor Author

This version seems working. I closed the previous PR.

@weiwangncar
Copy link
Collaborator

Jenkins tests have passed:

Test Type              | Expected  | Received |  Failed
= = = = = = = = = = = = = = = = = = = = = = = =  = = = =
Number of Tests        : 23           24
Number of Builds       : 60           58
Number of Simulations  : 158           156        0
Number of Comparisons  : 95           92        0

Failed Simulations are: 
None
Which comparisons are not bit-for-bit: 
None

@weiwangncar
Copy link
Collaborator

@cenlinhe This is great! Did you do anything differently?

@cenlinhe
Copy link
Contributor Author

I separate the update of the link to NoahMP submodule commit and the change in WRF files into two separate commits. But not sure why I cannot put all the changes (WRF files + NoahMP submodule commit link update) in one single commit.

@dudhia
Copy link
Collaborator

dudhia commented Mar 24, 2022

Is the link change something you do in the noahmp repository because it doesn't show up here? Not familiar with how this operates yet.

@cenlinhe
Copy link
Contributor Author

@dudhia Sorry for the confusion. The link change I meant is the link to an updated commit in the same Noah-MP branch "release-v4.4-WRF", not a new link to a new branch. It is the normal update for syncing with noahmp submodule based on Dave Gill's method.

@dudhia
Copy link
Collaborator

dudhia commented Mar 24, 2022 via email

@cenlinhe
Copy link
Contributor Author

@dudhia This is what Dave suggested to sync with updated Noah-MP submodule commit: #1660
If it is a bug fix, we will update the NoahMP github release-v4.4-WRF branch, and in this case, we will need to do this WRF sync with NoahMP updated branch commit every time we have a bug fix. If there is some major feature enhancements, we will not directly update the release-v4.4-WRF branch linked to WRF, we will wait before the next major WRF release and create a new release branch in NoahMP branch so that WRF can be linked to this new major release branch.
What do you think?

@dudhia
Copy link
Collaborator

dudhia commented Mar 24, 2022 via email

@cenlinhe
Copy link
Contributor Author

@dudhia Here is the technical steps:

In your own WRF branch planned for PR submission:

  1. Get the NoahMP repo available and in the WRF source code. Go to the
    top-most level of the WRF directory structure to issue the following command:
    git submodule update --init --recursive

  2. Get the correct NoahMP branch (currently it is the release-v4.4-WRF branch from NoahMP)
    cd phys/noahmp
    git checkout release-v4.4-WRF
    cd ..
    (Note that "git checkout release-v4.4-WRF" will update the link to the latest commit in the release-v4.4-WRF in NoahMP Github)

  3. Stage this modification:
    git add noahmp

  4. add the commit and submit the PR:
    git commit -m 'sync with NoahMP recent update'

@dudhia
Copy link
Collaborator

dudhia commented Mar 24, 2022 via email

@cenlinhe
Copy link
Contributor Author

What confused me though is that if I do the above NoahMP commit sync update along with some WRF file changes (e.g., Registry, surface driver) together in one single WRF commit, the issue in the previous PR I ran into occurred saying something "not able to pull Noah-MP" during compilation. But when I do the NoahMP sync as a single commit first and then add WRF file changes as a second commit (as I did here for this PR), the issue is gone. I did not see this issue in our HRLDAS offline NoahMP host model update, which also puts the NoahMP github as a submodule. Anyway, I think in the future, I will just do this two-step way for WRF PR to prevent potential issue.

@cenlinhe
Copy link
Contributor Author

I also have another PR with the NoahMP glacier bug fix that was just identified by Kevin Manning and Jordan yesterday. But I will wait until this current PR to be merged before I submit that bug fix PR, since last time Wei seems not able to merge two PRs with NoahMP commit link updates together.

@dudhia
Copy link
Collaborator

dudhia commented Mar 24, 2022 via email

Copy link
Collaborator

@weiwangncar weiwangncar left a comment

Choose a reason for hiding this comment

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

Changes are mostly passing two variables around..

@weiwangncar weiwangncar merged commit 407e9fc into wrf-model:release-v4.4 Mar 29, 2022
vlakshmanan-scala pushed a commit to scala-computing/WRF that referenced this pull request Apr 4, 2024
…put (wrf-model#1707)

Bring local snowmelt variable in Noah-MP to output variable and add accumulated snowmelt output in diagnostics

TYPE: enhancement

KEYWORDS: snowmelt, Noah-MP, output

SOURCE: Cenlin He (NCAR)

DESCRIPTION OF CHANGES:
Add two new output variables for Noah-MP package for snowmelt

ASSOCIATED REPOSITORY CHANGE:
[NCAR/noahmp#42](NCAR/noahmp#42)

LIST OF MODIFIED FILES: list of changed files (use git diff --name-status master to get formatted list)
M Registry/Registry.EM_COMMON
M Registry/registry.noahmp
M dyn_em/module_first_rk_step_part1.F
M phys/module_diag_misc.F
M phys/module_diagnostics_driver.F
M phys/module_surface_driver.F
M phys/noahmp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants