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

Add manage externals to access MMM-physics repo #2126

Merged
merged 8 commits into from
Jan 9, 2025

Conversation

weiwangncar
Copy link
Collaborator

TYPE: enhancement, no impact

KEYWORDS: manage_externals, some physics for tropical suite

SOURCE: internal

DESCRIPTION OF CHANGES:
Add the use of manage_externals tool to access physics in MMM-physics repository. The physics we access are part of the 'tropical' suite: YSU PBL, revised MM5 surface layer, WSM6 microphysics, and new Tiedtke scheme. It also accesses GWDO option 1 routine. These modules have been residing in phys/physics_mmm/ in 4.6. Instead of copied files in this directory, we now use manage_externals to access these modules from MMM-physics repository.

Will need to update the tags specified in Externals.cfg before final release.

LIST OF MODIFIED FILES:
A arch/Externals.cfg
M phys/Makefile
D phys/physics_mmm/bl_gwdo.F90
D phys/physics_mmm/bl_ysu.F90
D phys/physics_mmm/cu_ntiedtke.F90
D phys/physics_mmm/module_libmassv.F90
D phys/physics_mmm/mp_radar.F90
D phys/physics_mmm/mp_wsm6.F90
D phys/physics_mmm/mp_wsm6_effectRad.F90
D phys/physics_mmm/sf_sfclayrev.F90
A tools/manage_externals/

TESTS CONDUCTED:

  1. Successfully compiled the code on Derecho.
  2. Are the Jenkins tests all passing?

RELEASE NOTE: Add manage_externals tool to access physics modules in MMM-physics git repository.

@weiwangncar weiwangncar requested review from a team as code owners October 30, 2024 19:32
@weiwangncar
Copy link
Collaborator Author

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

@islas
Copy link
Collaborator

islas commented Dec 16, 2024

@weiwangncar After talking it over a bit with @mgduda, I think it would be best in this case to use manage externals from the top-level directory (changing the local_path to ./phys/physics_mmm) or have the Externals.cfg file in the location it is meant to be run.

This would allow future usage of manage_externals with less of an implicit path to run a config file located in arch/ inside the phys/ directory. I'm in favor of running manage_externals from the project root directory and always passing relative paths from said project root.

@weiwangncar
Copy link
Collaborator Author

@islas What does this mean? '... or have the Externals.cfg file in the location it is meant to be run.' Which location does this refer to?

@islas
Copy link
Collaborator

islas commented Dec 18, 2024

In that case the config file would be directly in the location the command checkout_externals is ran, so the Externals.cfg would be in phys/

@weiwangncar
Copy link
Collaborator Author

@islas @mgduda I moved Externals.cfg to phys/ - it is to avoid clusters in the top directory.

@islas
Copy link
Collaborator

islas commented Dec 22, 2024

@weiwangncar Thanks! Though, just to clarify, my original recommendation should not have cluttered the top directory as the command would have looked like:
./tools/manage_externals/checkout_externals arch/Externals.cfg with local_path = ./phys/physics_mmm so the MMM shared physics would still be checked out to ./phys/physics_mmm.

@weiwangncar
Copy link
Collaborator Author

@islas Are you saying instread of running tools/manage_externals/checkout_externals from phys/Makefile, I should add it and run it in the top-level Makefile? I'm still missing the point, sorry.

@islas
Copy link
Collaborator

islas commented Dec 23, 2024

I think the simplest way might just be in the $(LIBTARGET) recipe line that calls the checkout to a subshell that will cd to the top and call it there.

So like:

$(LIBTARGET):
  (cd .. && ./tools/manage_externals/checkout_externals --externals ./arch/Externals.cfg)

You can also see an example of changing directories within a single subshell call in make in frame/Makefile for the rule to run the registry.

The above with the change to the local_path in ./arch/Externals.cfg to be local_path = ./phys/physics_mmm makes the reference point of the checkout_externals command the project root directory. This will be helpful if say, for instance, another external repo is added to be in ./frame/some_common_frame - now all externals in the file have a common root directory.

@weiwangncar
Copy link
Collaborator Author

@islas If I have (cd .. && .....), do I need to have a 'cd phys' to come back to phys directory to execute the following 'make' command?

@islas
Copy link
Collaborator

islas commented Dec 24, 2024

I don't think you will need to as it will be executed in a subshell that then exits at the end of the parentheses

Use the execute_process function to checkout
externals always before adding sources.

Run the command from the top level project
root directory.
@islas islas requested a review from a team as a code owner December 30, 2024 20:27
Copy link
Collaborator

@dudhia dudhia left a comment

Choose a reason for hiding this comment

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

I will approve this based on not seeing anything to question.

@islas islas merged commit 7195dc2 into wrf-model:develop Jan 9, 2025
1 check was pending
@weiwangncar
Copy link
Collaborator Author

@islas @dudhia I may change the tag later if one for WRF release can be created in the shared physics repo.

@dudhia
Copy link
Collaborator

dudhia commented Jan 10, 2025

@weiwangncar Good point. We need to remember to do that. It may already be newer than the MPAS tag.

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.

5 participants