-
Notifications
You must be signed in to change notification settings - Fork 253
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 CDEPS to UFS-weather-model for Data component needs #524
Conversation
remove FoX dependency building and use ESMF Config instead. Co-authored-by: Jun Wang <junwang-noaa@users.noreply.github.com>
@uturuncoglu |
@aerorahul The problem is that this was not planned and not discussed with NCAR. There must be some kind of discussion before introducing major changes like this. |
Agreed. |
Do you want to set-up a call or I could do it. |
@ufuk Turuncoglu ***@***.***>
please feel free to set up a call. On the EMC side, we don't want to have
extra FoX and genf90 dependencies when integrating CDEPS. We already have
17 external repositories in UFS, we would like to have dedicated
application support, we would like to avoid extra dependencies unless it's
required. It's good that we have the flexibility not to include extra
dependency at EMC fork. Let's discuss this at the meeting.
By the way, @ binli2337 <https://github.com/binli2337> is setting up the
esmf configuration files instead of xml files. Please let us know if you
have questions on setting up the ESMF configuration file, or if it takes
you too long to set up the xml file, let us know.
…On Fri, Apr 16, 2021 at 5:45 PM Ufuk Turunçoğlu ***@***.***> wrote:
Do you want to set-up a call or I could do it.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#524 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AI7D6TO3MLSC7MFFDH4E56DTJCVYZANCNFSM427VUEVQ>
.
|
@junwang-noaa I could totally understand your concerns about the external dependencies. My point is that you could still achieve your goal by keeping fox submodule. Since you are controlling the build with CDEPS-Interface, you could build CDEPS without fox and you could use ESMF configuration file to control the namelist options. The fox capability could be there and ready to run once it is complied with fox but it won't be a not part of your dependency because you are not building it and it could be still used by other applications such as CESM. This will allow you to push your UFS related customizations such as bringing new data atmosphere etc. to ESCOMP without any issue and also allow CDEPS community to use those additional features. Having PR in ESCOMP level requires testing branch against CESM, HAFS and S2S and all regression tests needs to be passed at that level. At least, CESM tests will fail without Fox unless this issue is resolved. Of course if you don't have any intention to push those new features to ESCOMP, there is no need to be compatible across the CDEPS applications. Anyway, it would be hard to find time slot that works for everyone for the call and I propose to use call on Friday. In the mean time, if you send me an example ESMF configuration file for namelist that would be great because once I sync my CDEPS branch in HAFS level, I also need to fix the HAFS specific RTs to use ESMF configuration file rather than xml files. |
@uturuncoglu my understanding is that we need support for all the repositories in UFS since it will go to operational. So far the original FoX is from "https://github.com/andreww/fox", this is an external personal repository, which is not allowed to check out on operational platform for operational implementation. So what if the original repository is deleted? Is there agreement with the developer to keep/support this repository during the time frame when the code needs to be checked out in operational models? Is there any license issue? This is part of code dependency we are talking about and we don't know the answer. So we hope the CDEPS code gives us flexibility not to checkout for external repositories because of the operational code constraints. If CDEPS has to keep these external repositories, we may have to create new branches at emc fork when merging/pushing code to the official repo. This is what we would like to discuss with the CDEPS developer group. The ESMF configuration file will look like this as shown in dshr_stream_mod.F90 :
The corresponding xml file is:
|
You should use Fox as an optional external library just like pio or netcdf, without making it submodule or checking it out at the build time. And you should add USE_Fox=OFF option as a default, and then in CESM you can load fox module, set USE_Fox to ON, while we can leave it to OFF and never use it. |
@uturuncoglu @mvertens It looks to me that in Jun's implementation she abstracted reading in the config so that when you call NOAA-EMC/CDEPS@69a3a61#diff-c8dcbaa4f1e1fe3c77cdad46c05e297227205a83bd880a5e224bd6fcdbb22087R179 So, from this perspective, everything should work as before within CESM. So, is the real question here about how to handle the sub-module dependency to Fox? NCAR wants to have it as a submodule, and EMC does not want it as a submodule? I guess the reason that it could be a problem for EMC is if the repo goes away, then the clone command would fail? How can inclusion of the submodule be handled conditionally? Should |
@rsdunlapiv If we plan to go with this implementation, CESM needs to checkout Fox externally since Fox submodule is removed. If you see the recent comments of @junwang-noaa even without building, operational side does not want those external libraries. |
@uturuncoglu @mvertens @jedwards4b I am trying to lay out the solution space here. Solutions I can think of are:
|
@rsdunlapiv I think these are reasonable options that we could discuss further. |
@rsdunlapiv UFS does have several layers of submodules, we have to checkout UFS recursively. for 2), we then need to find a way to just block checking out submodules under CDEPS, I am not sure if there is a way to do that. Also this includes genf90, which is a submodule in the ESCOMF/CDEPS, but we don't need it in emc dork. Another option is that if CESM does not absolutely require this FoX, can we switch to esmf configure file to remove the FoX dependency in CESM too? My understanding is that CESM uses esmf configuration file already. |
@rsdunlapiv - thanks for your summary of the choices. That is very helpful.
@jedwards4b - what is your preference for 1. 2. or 3.?
…On Mon, Apr 19, 2021 at 2:00 PM Ufuk Turunçoğlu ***@***.***> wrote:
@rsdunlapiv <https://github.com/rsdunlapiv> I think these are reasonable
options that we could discuss further.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#524 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB4XCEYHUTDYEU2DHMKXA4DTJSDXRANCNFSM427VUEVQ>
.
--
Mariana Vertenstein
CESM Software Engineering Group Head
National Center for Atmospheric Research
Boulder, Colorado
Office 303-497-1349
Email: ***@***.***
|
@junwang-noaa - FOX is an integral part of how CESM uses CDEPS. It was always views as part of CDEPS. So CESM absolutely requires FOX. I think the options that @rsdunalpiv outlines are reasonable. |
@jedwards4b The issue is that there are some constraints with external repos for operational code. We'd like to use CDEPS, but we then have to face these external repositories in CDEPS. We can't use the code as is. We hope there is a way that both of us can use CDEPS in CESM and UFS. We are not creating any problem against your recommendation, we just want to find a way to integrate the CDEPS in UFS for future operational implementations. Thanks for your understanding. |
@junwang-noaa - thanks for your comments. In terms of the suggestions from @rsdunlapiv - which one do you think is optimal. I think it is great that UFS wants to use CDEPS and collaborate on its future development. We just need to figure out how UFS can continue to contribute to this community software in a way that does not adversely impact CESM. |
@rsdunlapiv I don't understand how option 1 solves the problem - it still leaves the fox code in cdeps. @junwang-noaa The external repo of fox has the same base repo that cdeps does so I don't understand what constraints you are facing? |
@jedwards4b For option 1 I was thinking that it would avoid any clone failures looking for the recursive submodule. It might be a technicality, but I suspect that when transitioning the code to operations, the review would show a link to an external repository that is not support/maintained, so it would probably be flagged. By including the code as a git subtree, it is always there, so there is no need for any external link. |
@jedwards4b in our version of emc/develop, there is no FoX/genf90 code in CDEPS. |
@mvertens so the FoX is also used in other conpoments in CESM besides CDEPS? |
@junwang-noaa - no FoX is used only by CDEPS - but it is needed when CESM uses CDEPS. |
@rsdunlapiv - that would make FoX be used the way we use ESMF - which is an interesting idea. |
Option 4 could be that you maintain a branch or fork of CDEPS without fox and this doesn't come back to our development branch. |
@rsdunlapiv |
@jedwards4b We can still merge/push code changes by creating a new branch to deal with the submodule separately. |
@aerorahul - @uturuncoglu has scheduled a call for 4/22 at 11:30MT. |
@jedwards4b with respect to option 4, it could create a lot of extra work to make sure that FoX is not "accidentally" merged in. In this case EMC CDEPS would become a "fork" in the classic sense of the word, and might start to diverge from the authoritative repository. |
…ommunity#524) Combined PROD and REPRO build modes into 'Release' build type. Now only 'Debug' and 'Release' build type are supported. Both build types must produce bit-for-bit reproducible outputs using different number of threads, mpi tasks, different domain decomposition, have reproducible restarts etc.
Description
This PR
genf90
to create Fortran 90 files. These files are pre-generated and committed to the ufs-weather-modelPR Checklist
Ths PR is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR. Please consult the ufs-weather-model wiki if you are unsure how to do this.
This PR has been tested using a branch which is up-to-date with the top of all sub-component repositories except for those sub-components which are the subject of this PR
An Issue describing the work contained in this PR has been created either in the subcomponent(s) or in the ufs-weather-model. The Issue should be created in the repository that is most relevant to the changes in contained in the PR. The Issue and the dependent sub-component PR
are specified below.
Instructions: All subsequent sections of text should be filled in as appropriate.
The information provided below allows the code managers to understand the changes relevant to this PR, whether those changes are in the ufs-weather-model repository or in a subcomponent repository. Ufs-weather-model code managers will use the information provided to add any applicable labels, assign reviewers and place it in the Commit Queue. Once the PR is in the Commit Queue, it is the PR owner's responsiblity to keep the PR up-to-date with the develop branch of ufs-weather-model.
Description
Provide a detailed description of what this PR does. What bug does it fix, or what feature does it add? Is a change of answers expected from this PR? Are any library updates included in this PR (modulefiles etc.)?
Issue(s) addressed
In preparation of #433
Testing
How were these changes tested? What compilers / HPCs was it tested with? Are the changes covered by regression tests? (If not, why? Do new tests need to be added?) Have regression tests and unit tests (utests) been run? On which platforms and with which compilers? (Note that unit tests can only be run on tier-1 platforms)
This PR adds CDEPS as a component. The component is not being linked or used in any of the applications that are currently supported within the UFS.
A
COMPILE
test forDATM
with CDEPS is added in this PR.Dependencies
NOAA-EMC/CDEPS repository has been updated to satisfy the requirements of this PR.