Conversation
TYPE: enhancement KEYWORDS: NoahMP, submodule SOURCE: Cenlin He (RAL/NCAR), internal DESCRIPTION OF CHANGES: This is an initial effort to include an external stand-alone repository into the WRF system use git "submodules". Here is the recipe, should we be interested in doing this again. Also this serves as a "perhaps not that way again" warning. Time will tell. \### Part 1: Out with the old, in with the new The code does not build after this initial step. 1. Remove the existing WRF-versions of the NoahMP source code ``` cd WRF/phys git rm module_sf_noahmp_glacier.F git rm module_sf_noahmp_groundwater.F git rm module_sf_noahmpdrv.F git rm module_sf_noahmplsm.F ``` ... and the tables ``` cd WRF/run git rm GENPARM.TBL git rm MPTABLE.TBL git rm SOILPARM.TBL ``` 2. Add the NoahMP repository as a submodule ``` cd WRF/phys git submodule add https://github.com/NCAR/noahmp noahmp ``` This generates the new file WRF/.gitmodules 3. Along with the rest of the staged files, include the SHA1 of the NoahMP repository as part of the what the WRF model keeps permanently. ``` cd WRF/phys git add noahmp ``` LIST OF MODIFIED FILES: A: .gitmodules D: phys/module_sf_noahmp_glacier.F D: phys/module_sf_noahmp_groundwater.F D: phys/module_sf_noahmpdrv.F D: phys/module_sf_noahmplsm.F A: phys/noahmp D: run/GENPARM.TBL D: run/MPTABLE.TBL D: run/SOILPARM.TBL TESTS CONDUCTED: 1. Tests? 2. Jenkins? RELEASE NOTE: The NoahMP external repository is now a submodule for WRF.
In the physics Makefile, before any files source files are processed, check to see if the NoahMP source files exist (the *.F files). If so, then proceed as normally (they must already be linked in). If the files do not exist, then `git submodule update --init --recursive` to bring in the NoahMP repository, and then manually link the files into the original WRF locations. If the source files did not exist, assume that the NoahMP table files were also missing, so put those in the run directory. Since these files are not really part of the WRF repo anymore, we remove them when asked for a super-duper cleaning: `clean -aa`. M clean M phys/Makefile
An "if" test in the phys/Makefile detects if the NoahMP source files exist. If not, then the git submodule command is used. The important item is that the top-level Makefile has a submodules target. M Makefile M clean M phys/Makefile
|
@davegill @Cenlin_He GENPARM.TBL and SOILPARM.TBL can not be removed. Almost all other LSMs (Noah, RUC, and possible PX) use them. |
|
@weiwangncar @cenlinhe |
|
I was able to compile this from the clone branch and it is quite
transparent to do.
…On Thu, Jan 20, 2022 at 2:03 PM Dave Gill ***@***.***> wrote:
@weiwangncar <https://github.com/weiwangncar> @cenlinhe
<https://github.com/cenlinhe>
Wei,
OK, I'll put those files back where I found them. :)
—
Reply to this email directly, view it on GitHub
<#1646 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEIZ77FI35ZLIVNAIXN4VPLUXB2AXANCNFSM5MNWIRQQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
|
@weiwangncar @davegill hmm... for the GENPARM.TBL and SOILPARM.TBL, if we do not remove them, then it may be difficult to connect these two tables to the NoahMP github repo? Currently, these two tables are maintained and updated through NoahMP repo. I thought these two tables are only used by NoahMP but it may not be the case as Wei mentioned. How should we solve this? |
|
We have to decide how we're going to allow the noahmp group to have control
over these.
…On Thu, Jan 20, 2022 at 2:03 PM Dave Gill ***@***.***> wrote:
@weiwangncar <https://github.com/weiwangncar> @cenlinhe
<https://github.com/cenlinhe>
Wei,
OK, I'll put those files back where I found them. :)
—
Reply to this email directly, view it on GitHub
<#1646 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEIZ77FI35ZLIVNAIXN4VPLUXB2AXANCNFSM5MNWIRQQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
|
The tables are available after compile, just less easy to get to for others.
…On Thu, Jan 20, 2022 at 2:16 PM Cenlin_He ***@***.***> wrote:
@weiwangncar <https://github.com/weiwangncar> @davegill
<https://github.com/davegill> hmm... for the GENPARM.TBL and
SOILPARM.TBL, if we do not remove them, then it may be difficult to connect
these two tables to the NoahMP github repo? Currently, these two tables are
maintained and updated through NoahMP repo. I thought these two tables are
only used by NoahMP but it may not be the case as Wei mentioned. How should
we solve this?
—
Reply to this email directly, view it on GitHub
<#1646 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEIZ77HYSGEQWZ7574LWSKLUXB3RPANCNFSM5MNWIRQQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because your review was requested.Message ID:
***@***.***>
|
|
If the tables are only available under run/ directory after compiling, then could we make the compile process allow it to obtain the three tables from the NoahMP submodule directory during compiling so that the 3 tables from NoahMP submodule will be available under run/ after compiling the code? |
I was also pleasantly surprised at how easy it was. |
|
@dudhia @weiwangncar @cenlinhe If we always assume that a user will build the code FIRST, then the we can get all three files from NoahMP, if we let them choose to host these table files. If a user needs access to the files BEFORE a build, then there is a not-quite-memorable command to get the NoahMP directory populated: Basically whatever is decided, there are really no negative impacts for developing this option - we just need to know which way we are going. |
|
I think it is fine to ask people to build before editing those files. They
are linked in run so are easy to find.
…On Thu, Jan 20, 2022 at 2:41 PM Dave Gill ***@***.***> wrote:
@dudhia <https://github.com/dudhia> @weiwangncar
<https://github.com/weiwangncar> @cenlinhe <https://github.com/cenlinhe>
Regarding the data tables, I have a second PR that only uses the
MPTABLE.TBL file. We can choose which PR we want to offer.
If we always assume that a user will build the code FIRST, then the we can
get all three files from NoahMP, if we let them choose to host these table
files.
If a user needs access to the files BEFORE a build, then there is a
not-quite-memorable command to get the NoahMP directory populated:
git submodule update --init --recursive
Which would then need to be used in tandem with some Unix ln -sf to put
those table files where they belong.
We could put those commands in a README in the run directory, but I doubt
anyone remembers that helper README is there (other than us).
Basically whatever is decided, there are really no negative impacts for
developing this option - we just need to know which way we are going.
—
Reply to this email directly, view it on GitHub
<#1646 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEIZ77FJW7WHJ47SI4HQHDDUXB6O5ANCNFSM5MNWIRQQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
I agree with Jimy. Users always look for those tables under run/ directory (which implies they will build the model first). |
|
RUC has its own section in SOILPARM.TBL so there is a case for
separately having a RUC one.
GENPARM.TBL seems common to all.
…On Thu, Jan 20, 2022 at 2:46 PM Cenlin_He ***@***.***> wrote:
I agree with Jimy. Users always look for those tables under run/ directory
(which implies they will build the model first).
—
Reply to this email directly, view it on GitHub
<#1646 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEIZ77B3KXATXTOBZYZ3H4TUXB7E7ANCNFSM5MNWIRQQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
modified: phys/Makefile
|
@weiwangncar @dudhia I talked to Fei and our plan is to create a copy of these tables only for Noah-MP by renaming them to Noah-MP specific ones in the near future (including some code changes within NoahMP code to read the new table names). Then, we will manage these new Noah-MP tables from the NoahMP github repo. For the current two tables, we will leave them as they are within WRF. If you all think this would be a good way to go, we could do it. However, I have to say that I do not have time currently to do the proposed changes in NoahMP. If you can wait until after AMS week (e.g., early Feb), then I can make the changes so that they can go into the upcoming v4.4 release. If the release timeline does not allow this, we will have to do this after the v4.4 release. What do you think? |
|
jenkins is OK |
|
@cenlinhe I think it is a good approach. We can accommodate this at a later time, since this affects NoahMP only (code-wise). |
|
@weiwangncar @dudhia @cenlinhe |
|
@weiwangncar @dudhia @cenlinhe |
|
@davegill @weiwangncar @dudhia OK, let's do the new table names for NoahMP later after v4.4 release. So for now, what do we do with the tables in NoahMP submodule? Do we still use those from NoahMP subdirectory under phys/ or we simply use them to replace the current WRF ones and not link to NoahMP subdirectory? (Note that there are updates for these three tables, so we need to use the version in NoahMP repo for this release no matter how we do it). |
|
@cenlinhe Not after 4.4 release, but after 4.4 cut-over. This means we will be creating a 4.4 release branch next week (maybe). Hopefully you will be making that change before our release to the public in April or May. |
|
@weiwangncar Oh, I see. I misunderstood it. Sure, I will try to do that. |
|
So is the plan to have a separate NoahMP version of these tables with a new
name in the submodule?
…On Thu, Jan 20, 2022 at 6:40 PM Cenlin_He ***@***.***> wrote:
@weiwangncar <https://github.com/weiwangncar> Oh, I see. I misunderstood
it. Sure, I will try to do that.
—
Reply to this email directly, view it on GitHub
<#1646 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEIZ77G3RRBB2PD4PZDP2FLUXC2QXANCNFSM5MNWIRQQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@dudhia Currently, yes, but let me discuss with Fei again since this change of table names would also affect other host models using NoahMP. I will get back to the WRF team later. |
|
@weiwangncar @dudhia @cenlinhe From a strategic position, if we remove these files from WRF and then try to bring them back - we lose all of their history. This PR currently removes those three table files. It seems that we have a choice (excluding MPTABLE.TBL) as to the version control of some of the land surface
The group that does not control VEGPARM.TBL and SOILPARM.TBL will need to make a renamed copy of the files. This is trivial to do. In WRF, if we do a |
|
VEGPARM.TBL may only be for the old Noah, not NoahMP (Cenlin can confirm).
…On Sun, Jan 23, 2022 at 11:53 PM Dave Gill ***@***.***> wrote:
@weiwangncar <https://github.com/weiwangncar> @dudhia
<https://github.com/dudhia> @cenlinhe <https://github.com/cenlinhe>
Folks,
Cenlin committed changes to the WRF repo with PR #1641
<#1641>. Those included changes to
the MPTABLE.TBL and SOILPARM.TBL files. I do understand that there is a
proposed change to the VEGPARM.TBL file that is now on the table. There was
no concern before about modifying these a table file in the previous NoahMP
PR.
From a strategic position, if we remove these files from WRF and then try
to bring them back - we lose all of their history. This PR currently
removes those three table files.
It seems that we have a choice (excluding MPTABLE.TBL) as to the version
control of some of the land surface
table files:
1. NoahMP controls VEGPARM.TBL and SOILPARM.TBL, or
2. WRF controls VEGPARM.TBL and SOILPARM.TBL.
The group that does not control VEGPARM.TBL and SOILPARM.TBL will need to
make a renamed copy of the files. This is trivial to do. In WRF, if we do a git
mv command, we maintain all of the history of VEGPARM.TBL and
SOILPARM.TBL. Excluding NoahMP, currently the VEGPARM.TBL and SOILPARM.TBL
files are only input in the RUC and Noah LSMs. This would be a pretty small
PR to accommodate renaming these files (for either NoahMP or WRF). The
NoahMP group probably has more oversight and stakeholders, whereas WRF
could unilaterally change the files without much ado.
—
Reply to this email directly, view it on GitHub
<#1646 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEIZ77HAQYA56WKMSL7TXZLUXTZONANCNFSM5MNWIRQQ>
.
Triage notifications on the go with GitHub Mobile for iOS
<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675>
or Android
<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Yes, VEGPARM.TBL is only for old Noah, not NoahMP. We did not update VEGPARM.TBL in my previous PR. The tables updated are GENPARM.TBL, SOILPARM.TBL, and MPTABLE.TBL. Among these, the MPTABLE.TBL is only for NoahMP, while the GENPARM.TBL and SOILPARM.TBL are also used by old Noah and RUC. |
|
@cenlinhe Can you send me your modified SOILPAR.TBL to take a look? |
|
@weiwangncar |
|
@weiwangncar If you want to take a look at what changes have been made to SOILPARM. TBL, please see here and navigate to the SOILPARM.TBL file in "File changes": cenlinhe@fcb9faa |
|
As it stands now, there is no difference in GENPARM.TBL in WRF and noahmp repository. And the only change in SOILPARM.TBL is the new 7 additions, and it doesn't break WRF (I tested). |
|
@weiwangncar @dudhia @cenlinhe
Wei, |
|
Choosing NoahMP as a submodule in WRF (3) #1653 |
TYPE: enhancement
KEYWORDS: NoahMP, submodule
SOURCE: Cenlin He (RAL/NCAR), internal
DESCRIPTION OF CHANGES:
This is an initial effort to include an external stand-alone repository into the
WRF system using git "submodules".
Here is the recipe, should we be interested in doing this again. Also this serves as
a "perhaps not that way again" warning. Time will tell.
Part 1: Out with the old, in with the new
The code does not build after this initial step. We are just preparing the soil.
... and the tables
This generates the new file WRF/.gitmodules
as part of the what the WRF model keeps permanently.
LIST OF MODIFIED FILES:
A .gitmodules
D phys/module_sf_noahmp_glacier.F
D phys/module_sf_noahmp_groundwater.F
D phys/module_sf_noahmpdrv.F
D phys/module_sf_noahmplsm.F
A phys/noahmp
D run/GENPARM.TBL
D run/MPTABLE.TBL
D run/SOILPARM.TBL
Part 2: Make it work
The code builds after this step, which includes the mods to the build system (the
variously impacted Makefiles).
LIST OF MODIFIED FILES:
M Makefile
M clean
M phys/Makefile
General Logic
In the top-level WRF Makefile, the
physicstarget now includes a pre-stepto first build the NoahMP submodule files. If the NoahMP source files exist (the *.F files)
nothing happens. If the files do not exist, then the rule is
git submodule update --init --recursiveto bring in the NoahMP repository, and then manually link the files into the
original WRF locations. If the source files did not exist, assume that the
NoahMP table files were also missing, so put those in the run directory.
Since these files are not really part of the WRF repo anymore, we remove
them when asked for a super-duper cleaning:
clean -aa.The NoahMP files are linked into the physics directory. This emulates the existing paradigm
of putting all of the physics source into a single location. This minimizes the infrastructure
modifications, and becomes almost transparent to a casual user. Care will need to be taken
so that no files associated with NoahMP are accidentally re-incorporated back into WRF.
TESTS CONDUCTED:
command, and subsequent linking.
step is bypassed.
RELEASE NOTE: The NoahMP external repository is now a submodule for WRF.