CODE REVIEW ONLY: WRF-TEB integration #1052
Conversation
|
@dmey |
|
@dudhia @weiwangncar |
|
It's in a grey area like irrigation because it mostly couples through the
LSM.
Maybe it needs their approval first,
Jimy
…On Mon, Jan 13, 2020 at 1:43 PM Dave Gill ***@***.***> wrote:
@dudhia <https://github.com/dudhia> @weiwangncar
<https://github.com/weiwangncar>
Does this require the physics review board approval?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1052?email_source=notifications&email_token=AEIZ77DAFFYF4VIU4NNQNFLQ5SY6NA5CNFSM4KC4OBH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIZ2OXI#issuecomment-573810525>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEIZ77EKPKXWEZO4LD7T45TQ5SY6NANCNFSM4KC4OBHQ>
.
|
|
@davegill thanks for this -- I also wanted to clarify a couple of general points:
|
|
👋 @davegill -- I was just wondering if you had a chance to look at #1052 (comment). |
@dudhia is there anything that I need to do on my side? Is there still a standard procedure to follow? |
|
I think we need to still review the code changes at this end.
…On Thu, Jan 23, 2020 at 2:10 PM dmey ***@***.***> wrote:
It's in a grey area like irrigation because it mostly couples through the
LSM.
Maybe it needs their approval first,
Jimy
@dudhia <https://github.com/dudhia> is there anything that I need to do
on my side? Is there still a standard procedure to follow?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1052?email_source=notifications&email_token=AEIZ77ABRVBCNN77R4YBH7LQ7IBULA5CNFSM4KC4OBH2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJY3SSY#issuecomment-577878347>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AEIZ77BIK5PWYEF54HK3NXLQ7IBULANCNFSM4KC4OBHQ>
.
|
I think it is just fine assume that a user installs TEB first. However, if this appears to be a problem, say after a year or so and there are lots of users complaining, then you could put the source (and necessary build files) in the external directory.
We have provided a few comments that are easy to review on the github page with "Files changed". |
Sounds good. 👍
Thanks for your comments but I had a few questions about a couple of comments -- can you please let me know about: #1052 (comment) and #1052 (comment)? |
|
@davegill sounds good 👍 -- I will go ahead with those changes you suggested and create a PR for WRF-TEB targeting |
|
@dmey |
4 participants
Hi @davegill and @dudhia this is the code as implemented in the paper:
It includes all the files modified in WRF as well as the wrapper for TEB
phys/module_sf_teb.Fused to integrate TEB model (https://github.com/teb-model/teb) as an external library into WRF.The implementation here reflects what is in the paper (currently under review) and at https://github.com/WRF-CMake/wrf-teb. The current implementation is only available through the CMake project and is currently dependent on WRF-CMake as I found it much easier to do so when developing the code and port TEB into WRF however, given that we are currently awaiting to know more about the integration of WRF-CMake into WRF (#1010) I have created this draft PR to ask you if you could review the code (without CMake).
This PR is only meant for review, not for merging (this is why I picked the 4.1 branch). I will create a separate PR later if the contribution is accepted, targeted at the develop branch and with Makefiles. The idea is that depending on your specific requirements (e.g. whether we can depend on TEB as an external library) I could then write the Makefiles necessary to make this work also with the current WRF version.
The entry point for TEB is
phys/module_sf_teb.Fwhich callssrc/driver/modd_wrf_teb_driver.F90(currently linked in using CMake as an external project) from the TEB model repository https://github.com/teb-model/teb.