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

Use prebuilt FMS library #534

Merged
merged 31 commits into from
May 14, 2021
Merged

Conversation

DusanJovic-NOAA
Copy link
Collaborator

@DusanJovic-NOAA DusanJovic-NOAA commented Apr 19, 2021

PR 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.

  • If new or updated input data is required by this PR, it is clearly stated in the text of the PR.

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

This PR removes FMS git submodule, and updates module files and top-level CMakeLists.txt to use fms module (library) built by hpc-stack.

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)

  • hera.intel
  • hera.gnu
  • orion.intel
  • cheyenne.intel
  • cheyenne.gnu
  • gaea.intel
  • jet.intel
  • wcoss_cray
  • wcoss_dell_p3

Requires new baselines.

Copy link
Collaborator

@climbfuji climbfuji left a comment

Choose a reason for hiding this comment

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

Looks good. You will need to update cmake in several other modulfiles. For all cheyenne modules, please use module load cmake/3.18.2. Didn't check the other modulefiles/platforms.

CMakeLists.txt Show resolved Hide resolved
Copy link
Contributor

@aerorahul aerorahul left a comment

Choose a reason for hiding this comment

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

Looks good.
Just one comment on the unnecessary printing of cmake version.

CMakeLists.txt Outdated Show resolved Hide resolved
Copy link
Contributor

@aerorahul aerorahul left a comment

Choose a reason for hiding this comment

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

looks good.

@BrianCurtis-NOAA
Copy link
Collaborator

BrianCurtis-NOAA commented Apr 19, 2021

Should you also do a git rm FMS ?

@DusanJovic-NOAA
Copy link
Collaborator Author

Should you also do a git rm FMS ?

Yes. Thanks.

@@ -1,12 +0,0 @@
--- FMS/mpp/include/mpp_util_mpi.inc 2021-02-08 08:24:21.000000000 -0700
+++ FMS/mpp/include/mpp_util_mpi.inc 2021-02-08 08:24:15.000000000 -0700
Copy link
Collaborator

Choose a reason for hiding this comment

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

So this patch is no longer needed or is moved to fms library?

Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't know until we try. If it is still needed, it will have to go to hpc-stack so that we have it under our own control.

Copy link
Contributor

Choose a reason for hiding this comment

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

It will be difficult to apply inside of hpc-stack build_fms.sh.
There are no machine specific conditionals in any of these build scripts.

@climbfuji
Copy link
Collaborator

climbfuji commented Apr 20, 2021 via email

@aerorahul
Copy link
Contributor

I was thinking of doing that inside config/config_cheyenne_gnu-9.sh if needed.

On Apr 20, 2021, at 7:16 AM, Rahul Mahajan @.***> wrote:

@aerorahul commented on this pull request.

In cheyenne_gnu_fms_mpp_util_mpi_inc.patch #534 (comment):

@@ -1,12 +0,0 @@
---- FMS/mpp/include/mpp_util_mpi.inc 2021-02-08 08:24:21.000000000 -0700
-+++ FMS/mpp/include/mpp_util_mpi.inc 2021-02-08 08:24:15.000000000 -0700
It will be difficult to apply inside of hpc-stack build_fms.sh.
There are no machine specific conditionals in any of these build scripts.


You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub #534 (comment), or unsubscribe https://github.com/notifications/unsubscribe-auth/AB5C2RIWBTPKV6JJCIAVDBLTJV5ETANCNFSM43GMRZXA.

If you can do that, that would be great.

1 similar comment
@aerorahul
Copy link
Contributor

I was thinking of doing that inside config/config_cheyenne_gnu-9.sh if needed.

On Apr 20, 2021, at 7:16 AM, Rahul Mahajan @.***> wrote:

@aerorahul commented on this pull request.

In cheyenne_gnu_fms_mpp_util_mpi_inc.patch #534 (comment):

@@ -1,12 +0,0 @@
---- FMS/mpp/include/mpp_util_mpi.inc 2021-02-08 08:24:21.000000000 -0700
-+++ FMS/mpp/include/mpp_util_mpi.inc 2021-02-08 08:24:15.000000000 -0700
It will be difficult to apply inside of hpc-stack build_fms.sh.
There are no machine specific conditionals in any of these build scripts.


You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub #534 (comment), or unsubscribe https://github.com/notifications/unsubscribe-auth/AB5C2RIWBTPKV6JJCIAVDBLTJV5ETANCNFSM43GMRZXA.

If you can do that, that would be great.

@climbfuji
Copy link
Collaborator

climbfuji commented Apr 20, 2021 via email

@DusanJovic-NOAA DusanJovic-NOAA added the Baseline Updates Current baselines will be updated. label Apr 20, 2021
@DusanJovic-NOAA DusanJovic-NOAA self-assigned this Apr 30, 2021
@MinsukJi-NOAA
Copy link
Contributor

@DusanJovic-NOAA, I made a PR to your libfms branch for CI update that is needed (#575)

MinsukJi-NOAA and others added 3 commits May 13, 2021 16:39
* Remove fms submodule check
* Use an updated ubuntu-hpc container for fms library
@BrianCurtis-NOAA
Copy link
Collaborator

Machine: orion
Compiler: intel
Job: BL
Repo location: /work/noaa/nems/emc.nemspara/autort/pr/618299995/20210513134518/ufs-weather-model
Please manually delete: /work/noaa/stmp/bcurtis/stmp/bcurtis/FV3_RT/rt_142045
Repo location: /work/noaa/nems/emc.nemspara/autort/pr/618299995/20210513150953/ufs-weather-model
Please manually delete: /work/noaa/stmp/bcurtis/stmp/bcurtis/FV3_RT/rt_374650
Test fv3_wrtGlatlon_netcdf 038 failed failed
Test fv3_wrtGlatlon_netcdf 038 failed in run_test failed
Please make changes and add the following label back:
orion-intel-BL

@BrianCurtis-NOAA
Copy link
Collaborator

Machine: jet
Compiler: intel
Job: BL
Repo location: /lfs4/HFIP/h-nems/emc.nemspara/autort/pr/618299995/20210513184519/ufs-weather-model
Please manually delete: /lfs4/HFIP/h-nems/emc.nemspara/RT_RUNDIRS/emc.nemspara/FV3_RT/rt_180393
Repo location: /lfs4/HFIP/h-nems/emc.nemspara/autort/pr/618299995/20210513213615/ufs-weather-model
Please manually delete: /lfs4/HFIP/h-nems/emc.nemspara/RT_RUNDIRS/emc.nemspara/FV3_RT/rt_188552
Test fv3_gfsv16_csawmg 071 failed in run_test failed
Please make changes and add the following label back:
jet-intel-BL

@BrianCurtis-NOAA
Copy link
Collaborator

Machine: orion
Compiler: intel
Job: RT
Repo location: /work/noaa/nems/emc.nemspara/autort/pr/618299995/20210513164511/ufs-weather-model
Please manually delete: /work/noaa/stmp/bcurtis/stmp/bcurtis/FV3_RT/rt_256028
Test cpld_bmarkfrac_v16_nsst 022 failed failed
Test cpld_bmarkfrac_v16_nsst 022 failed in run_test failed
Please make changes and add the following label back:
orion-intel-RT

@DusanJovic-NOAA
Copy link
Collaborator Author

Ready for merge.

CMakeLists.txt Show resolved Hide resolved
Copy link
Collaborator

@junwang-noaa junwang-noaa left a comment

Choose a reason for hiding this comment

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

I see the compile time is dropped significantly, thanks for making this change!

@DusanJovic-NOAA DusanJovic-NOAA merged commit 9350745 into ufs-community:develop May 14, 2021
@DusanJovic-NOAA DusanJovic-NOAA deleted the libfms branch May 14, 2021 14:11
pjpegion pushed a commit to NOAA-PSL/ufs-weather-model that referenced this pull request Apr 4, 2023
This adds support for 32-bit physics to the FV3, based on prior work on the Neptune model.

Co-authored-by: Dusan Jovic <dusan.jovic@noaa.gov>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Baseline Updates Current baselines will be updated.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants