Skip to content

Add MMM-physics as a git submodule#2282

Merged
islas merged 3 commits intowrf-model:developfrom
islas:convert_mmm_phys_git_submodule
Feb 10, 2026
Merged

Add MMM-physics as a git submodule#2282
islas merged 3 commits intowrf-model:developfrom
islas:convert_mmm_phys_git_submodule

Conversation

@islas
Copy link
Collaborator

@islas islas commented Feb 6, 2026

TYPE: no impact

KEYWORDS: git, submodule, manage_externals

SOURCE: internal

DESCRIPTION OF CHANGES:
Problem:
MMM-physics (under phys/physics_mmm) is currently the only external repository tracked with only manage_externals. This makes manage_externals and git two failure points for a proper clone of WRF to succeed.

Solution:
Add MMM-physics as a git submodule using the original path of phys/physics_mmm). To avoid confusion in managing two locations of the submodule (one in .gitmodules and the other in arch/Externals.cfg) the MMM-physics repo is removed from manage_externals. From a user perspective, no change to the build process is evident.

TESTS CONDUCTED:

  1. Build proceeds as normal from a fresh clone.

@islas islas requested review from a team as code owners February 6, 2026 00:54
[submodule "phys/physics_mmm"]
path = phys/physics_mmm
url = https://github.com/NCAR/MMM-physics.git
[submodule "phys/MYNN-SFC"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we put this above MYNN-EDMF in order?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could, but I don't think I understand why we would want to do that. Is there any particular reason to change the order?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Currently it is between two MYNN packages, and also other new ones may be put after MYNN PBL. I already resolved one conflict for TEMPO being put there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't see that fix for TEMPO as it still between the two MYNN packages. Regardless, this is a metadata file for git to use where order should not matter.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it comes down to a desire to keep the MYNN schemes together in the list of submodules (and similarly in the Makefile logic, I would assume). This seems reasonable enough to me.

Copy link
Collaborator

Choose a reason for hiding this comment

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

TEMPO is not merged yet, but I resolved that conflict. It was added to older code that didn't have MYNNSFC yet.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@dudhia For #2270, even after the merge conflict being resolved it looks like it will place the submodule between the two MYNN submodules.

And very well, I'll make the changes to the .gitmodules and Makefile to ensure MYNN colocality.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, the conflict resolution didn't allow me to swap them just allow both.

@mgduda
Copy link
Collaborator

mgduda commented Feb 6, 2026

@islas Is this PR meant to address the build failures that we were seeing in the WRF "coop" container?

@islas
Copy link
Collaborator Author

islas commented Feb 6, 2026

Yes. This should resolve the issues as this will take the same logic path that our remaining external repos use.

@weiwangncar
Copy link
Collaborator

The regression test results:

Test Type              | Expected  | Received |  Failed
= = = = = = = = = = = = = = = = = = = = = = = =  = = = =
Number of Tests        : 23           24
Number of Builds       : 60           57
Number of Simulations  : 158           150        0
Number of Comparisons  : 95           86        0

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

@mgduda
Copy link
Collaborator

mgduda commented Feb 6, 2026

Will there be any problems if a contributor updates only the git submodule or the Externals.cfg file (i.e., one, but not both)?

@islas
Copy link
Collaborator Author

islas commented Feb 6, 2026

@mgduda Yes, sort of. Since manage_externals runs last of two, it will have priority when working. Although we don't often update this submodule, this could become confusing in the future.

@mgduda
Copy link
Collaborator

mgduda commented Feb 6, 2026

Perhaps it would be better overall if there were just one way of incorporating external repositories.

@islas
Copy link
Collaborator Author

islas commented Feb 6, 2026

Agreed. This proposed change was based on the assumption that we wanted to keep manage_externals in some capacity and avoid wholly switching the repo reference over. However, maybe the potential headache it could cause down the road is worth just addressing now.

@islas islas requested a review from a team as a code owner February 7, 2026 00:22
@islas islas merged commit aa6ad5b into wrf-model:develop Feb 10, 2026
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