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

Fms mixedmode update #962

Merged
merged 27 commits into from
Jun 23, 2022
Merged

Conversation

MinsukJi-NOAA
Copy link
Contributor

Description
This PR includes code changes to allow a single FMS library to handle running 32-bit FV3 and 64-bit MOM6 for the coupled UFS weather model. The approach taken to minimize the code changes is to assume FMS is compiled in 64 bit, and to only modify the interfaces to FV3.

How Has This Been Tested?

  • Code changes pass the FMS CI tests (GNU) on GitHub Actions.
  • UFS weather model regression tests have been successfully done using code changes in the previous PR Fms mixedmode #857
    • UFS weather model regression tests will be performed using the updated code changes in this PR

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules
  • New check tests, if applicable, are included
  • make distcheck passes

MinsukJi-NOAA and others added 25 commits October 5, 2021 15:45
Code changes for files in the `diag_manager` directory
* Modify set_tracer_profile in tracer_manager.F90
* Modify real_to_time_type in time_manager.F90
* Modify set_tracer_profile in tracer_manager.F90
* Specify platform_mod variables used
* Modify real_to_time_type in time_manager.F90
* Modify files in the sat_vapor_pres directory
* Fix select type statement in sat_vapor_pres_k.F90
* Move write temp statement inside select type
* add constants4 directory for single precision constants
* modify CMakeLists.txt
* modify constants.F90 and fmsconstants.F90
* modify constants4/constants4.F90 and constants4/fmsconstants4.F90
* update constants4/fmsconstants4.F90
…3 for atmos_4xdaily* file NaN values (#7)

* Modify sat_vapor_pres_k.F90 to fix FV3 32 bit atmos_4xdaily* file NaN values
…t of triple do loops (#8)

* Move SELECT TYPE blocks out of triple do loops in diag_manager.F90
* Make 'unsupported kind' error messages more descriptive. Change author of constants4.F90

* Add doxygen comment to valid_types in sat_vapor_pres/sat_vapor_pres_k.F90
* Modify codes for r8-r4 conversion to remove compiler warnings
* Remove OVERLOAD_R8 directives regarding send_data_*d_r8 subroutines
* Add Doxygen comments to constants4.F90
* get_unit warning now only prints on root_pe (NOAA-GFDL#872)

Co-authored-by: Eric Stofferahn <Eric.Stofferahn@noaa.gov>

* Update changelog, version numbers and build info (NOAA-GFDL#873)

Co-authored-by: rem1776 <Ryan.Mulhall@lscamd50-d.gfdl.noaa.gov>

* chore: Change version number to next development version (NOAA-GFDL#874)

* CI: Update build action for yaml parser (NOAA-GFDL#871)

Update build actions
Change images to organization's repo and fix mixed mode parser test failure

* test(parser): Change real comparison value to double (NOAA-GFDL#886)

Co-authored-by: rem1776 <Ryan.Mulhall@lscamd50-d.gfdl.noaa.gov>

* Make 'unsupported kind' error messages more descriptive. Change author of constants4.F90

* Add doxygen comment to valid_types in sat_vapor_pres/sat_vapor_pres_k.F90

* Modify codes for r8-r4 conversion to remove compiler warnings

* Remove OVERLOAD_R8 directives regarding send_data_*d_r8 subroutines

* Add Doxygen comments to constants4.F90

* Updates AM4 regression test suite to run intel 21 only on PW (NOAA-GFDL#893)

* feat: emc changes for mixedmode (NOAA-GFDL#857) (NOAA-GFDL#898)

* Change send_data interface and any necessary routines for mixed real precision support
* Adds an r4 size constants file

BREAKING CHANGE: changes some argument types in diag_manager, tracer_manager, sat_vapor_pres, and time_manager from real to class(*)

Co-authored-by: Minsuk Ji <57227195+MinsukJi-NOAA@users.noreply.github.com>

* feat: adds option to override (ignore) checksum test when reading restarts (NOAA-GFDL#889)

* update to fms2_io to allow reads to ignore embedded checksums on a fileobj basis
* added fms2_io test cases for the ignore_checksum option to restart reads
tests the domain, domain_wrap, and bc restart options

* docs: add CI information file (NOAA-GFDL#899)

* test: Adds check_nml_error after reading a namelist, so it could crash only as expected (NOAA-GFDL#904)

* Revert "feat: emc changes for mixedmode (NOAA-GFDL#857) (NOAA-GFDL#898)" (NOAA-GFDL#914)

This reverts commit 516a5ef.

* fix: clean up unused/uninitialized variables and other warnings (NOAA-GFDL#859)

Remove unused variables throughout and changes/removals for other warnings such as uninitialized values and implicit casts

* docs: update function style and branch names

* Changes master to main in CONTRIBUTING.md

* Claifies function return documentation in CODE_STYLE.md

* feat: adds support for logical variables in the yaml parser (NOAA-GFDL#907)

* Adds support for logical variables in the yaml parser

* correctly determines if a string is true or false

* fix: add check for affinity to self-initialize and update test program (NOAA-GFDL#917)

* fix: adds casts to class(*) calls to match doubles in C routines (NOAA-GFDL#920)

Co-authored-by: rem1776 <Ryan.Mulhall@lscamd50-d.gfdl.noaa.gov>

* build: add intel code coverage build option to autotools (NOAA-GFDL#895)

* refactor: change to inclusive variable names (NOAA-GFDL#926)

* feat: Allow maximum number of restart variables to be set at build time (NOAA-GFDL#909)

* fix: Fixes for linter action and code style (NOAA-GFDL#869)

* test: Test script updates and input tests (NOAA-GFDL#800)

Rewrites test script to use added testing shell functions
Adds previously skipped or removed unit tests and support for testing with input files

Co-authored-by: Seth Underwood <Seth.Underwood@noaa.gov>
Co-authored-by: Uriel Ramirez <uriel.ramirez@noaa.gov>

* chore: 2022.01 release changes (NOAA-GFDL#941)

* chore: change version number to next development version (NOAA-GFDL#945)

* Add option for position independent code (NOAA-GFDL#930)

* fix: document and change parameter names for grid versions (NOAA-GFDL#918)

* fix: Moving empty and non-existent data table handling inside data_override (NOAA-GFDL#933)

Enables the processing of an empty or non-existent data_table or data_table.yaml from within data_override, so that any routine can call data_override_init without worry

* fix: Removal of internal FMS use of fms_io and mpp_io (NOAA-GFDL#928)

* fix: mpp changes to solve compile issues with serial builds (NOAA-GFDL#949)

* feat: adds build option for compiling with different sets of constants (NOAA-GFDL#929)

* feat: add and aggregate mpp_chksum unit tests (NOAA-GFDL#946)

* feat: add module for string utility routines (NOAA-GFDL#911)

Adds module and accompanying test for common string operations

* revert: reinstated the mpp_io routines and put them at the end for future elimination. (NOAA-GFDL#952)

* fix: add back in small_fac parameter to constants .h files (NOAA-GFDL#954)

* fix: root pe bug with fms2_io::flush_file (NOAA-GFDL#958)

* fix: clean up string routines for fms_string_utils_mod (NOAA-GFDL#953)

* Make changes to lines with more than 120 columns

Co-authored-by: Ryan Mulhall <35538242+rem1776@users.noreply.github.com>
Co-authored-by: Eric Stofferahn <Eric.Stofferahn@noaa.gov>
Co-authored-by: rem1776 <Ryan.Mulhall@lscamd50-d.gfdl.noaa.gov>
Co-authored-by: Tom Robinson <33458882+thomas-robinson@users.noreply.github.com>
Co-authored-by: Rusty Benson <6594772+bensonr@users.noreply.github.com>
Co-authored-by: uramirez8707 <49168881+uramirez8707@users.noreply.github.com>
Co-authored-by: Raffaele Montuoro <raffaele.montuoro@outlook.com>
Co-authored-by: Seth Underwood <Seth.Underwood@noaa.gov>
Co-authored-by: Uriel Ramirez <uriel.ramirez@noaa.gov>
Co-authored-by: Matthew Thompson <matthew.thompson@nasa.gov>
Co-authored-by: Eric <7784797+GFDL-Eric@users.noreply.github.com>
* get_unit warning now only prints on root_pe (NOAA-GFDL#872)

Co-authored-by: Eric Stofferahn <Eric.Stofferahn@noaa.gov>

* Update changelog, version numbers and build info (NOAA-GFDL#873)

Co-authored-by: rem1776 <Ryan.Mulhall@lscamd50-d.gfdl.noaa.gov>

* chore: Change version number to next development version (NOAA-GFDL#874)

* CI: Update build action for yaml parser (NOAA-GFDL#871)

Update build actions
Change images to organization's repo and fix mixed mode parser test failure

* test(parser): Change real comparison value to double (NOAA-GFDL#886)

Co-authored-by: rem1776 <Ryan.Mulhall@lscamd50-d.gfdl.noaa.gov>

* Make 'unsupported kind' error messages more descriptive. Change author of constants4.F90

* Add doxygen comment to valid_types in sat_vapor_pres/sat_vapor_pres_k.F90

* Modify codes for r8-r4 conversion to remove compiler warnings

* Remove OVERLOAD_R8 directives regarding send_data_*d_r8 subroutines

* Add Doxygen comments to constants4.F90

* Updates AM4 regression test suite to run intel 21 only on PW (NOAA-GFDL#893)

* feat: emc changes for mixedmode (NOAA-GFDL#857) (NOAA-GFDL#898)

* Change send_data interface and any necessary routines for mixed real precision support
* Adds an r4 size constants file

BREAKING CHANGE: changes some argument types in diag_manager, tracer_manager, sat_vapor_pres, and time_manager from real to class(*)

Co-authored-by: Minsuk Ji <57227195+MinsukJi-NOAA@users.noreply.github.com>

* feat: adds option to override (ignore) checksum test when reading restarts (NOAA-GFDL#889)

* update to fms2_io to allow reads to ignore embedded checksums on a fileobj basis
* added fms2_io test cases for the ignore_checksum option to restart reads
tests the domain, domain_wrap, and bc restart options

* docs: add CI information file (NOAA-GFDL#899)

* test: Adds check_nml_error after reading a namelist, so it could crash only as expected (NOAA-GFDL#904)

* Revert "feat: emc changes for mixedmode (NOAA-GFDL#857) (NOAA-GFDL#898)" (NOAA-GFDL#914)

This reverts commit 516a5ef.

* fix: clean up unused/uninitialized variables and other warnings (NOAA-GFDL#859)

Remove unused variables throughout and changes/removals for other warnings such as uninitialized values and implicit casts

* docs: update function style and branch names

* Changes master to main in CONTRIBUTING.md

* Claifies function return documentation in CODE_STYLE.md

* feat: adds support for logical variables in the yaml parser (NOAA-GFDL#907)

* Adds support for logical variables in the yaml parser

* correctly determines if a string is true or false

* fix: add check for affinity to self-initialize and update test program (NOAA-GFDL#917)

* fix: adds casts to class(*) calls to match doubles in C routines (NOAA-GFDL#920)

Co-authored-by: rem1776 <Ryan.Mulhall@lscamd50-d.gfdl.noaa.gov>

* build: add intel code coverage build option to autotools (NOAA-GFDL#895)

* refactor: change to inclusive variable names (NOAA-GFDL#926)

* feat: Allow maximum number of restart variables to be set at build time (NOAA-GFDL#909)

* fix: Fixes for linter action and code style (NOAA-GFDL#869)

* test: Test script updates and input tests (NOAA-GFDL#800)

Rewrites test script to use added testing shell functions
Adds previously skipped or removed unit tests and support for testing with input files

Co-authored-by: Seth Underwood <Seth.Underwood@noaa.gov>
Co-authored-by: Uriel Ramirez <uriel.ramirez@noaa.gov>

* chore: 2022.01 release changes (NOAA-GFDL#941)

* chore: change version number to next development version (NOAA-GFDL#945)

* Add option for position independent code (NOAA-GFDL#930)

* fix: document and change parameter names for grid versions (NOAA-GFDL#918)

* fix: Moving empty and non-existent data table handling inside data_override (NOAA-GFDL#933)

Enables the processing of an empty or non-existent data_table or data_table.yaml from within data_override, so that any routine can call data_override_init without worry

* fix: Removal of internal FMS use of fms_io and mpp_io (NOAA-GFDL#928)

* fix: mpp changes to solve compile issues with serial builds (NOAA-GFDL#949)

* feat: adds build option for compiling with different sets of constants (NOAA-GFDL#929)

* feat: add and aggregate mpp_chksum unit tests (NOAA-GFDL#946)

* feat: add module for string utility routines (NOAA-GFDL#911)

Adds module and accompanying test for common string operations

* revert: reinstated the mpp_io routines and put them at the end for future elimination. (NOAA-GFDL#952)

* fix: add back in small_fac parameter to constants .h files (NOAA-GFDL#954)

* fix: root pe bug with fms2_io::flush_file (NOAA-GFDL#958)

* fix: clean up string routines for fms_string_utils_mod (NOAA-GFDL#953)

* chore: update libFMS module with new routines (NOAA-GFDL#912)

* fix: removes fms_c.c and fms_c.h (NOAA-GFDL#961)

* Make changes to lines with more than 120 columns

Co-authored-by: Ryan Mulhall <35538242+rem1776@users.noreply.github.com>
Co-authored-by: Eric Stofferahn <Eric.Stofferahn@noaa.gov>
Co-authored-by: rem1776 <Ryan.Mulhall@lscamd50-d.gfdl.noaa.gov>
Co-authored-by: Tom Robinson <33458882+thomas-robinson@users.noreply.github.com>
Co-authored-by: Rusty Benson <6594772+bensonr@users.noreply.github.com>
Co-authored-by: uramirez8707 <49168881+uramirez8707@users.noreply.github.com>
Co-authored-by: Raffaele Montuoro <raffaele.montuoro@outlook.com>
Co-authored-by: Seth Underwood <Seth.Underwood@noaa.gov>
Co-authored-by: Uriel Ramirez <uriel.ramirez@noaa.gov>
Co-authored-by: Matthew Thompson <matthew.thompson@nasa.gov>
Co-authored-by: Eric <7784797+GFDL-Eric@users.noreply.github.com>
@thomas-robinson
Copy link
Member

@ngs333 (and to some extent @uramirez8707 ), this is going to conflict with your development, especially in send_data. Do you think:

  1. These updates can coexist with your updates? or
  2. These updates will be replaced by your updates?
  3. @ngs333 you deleted a lot of what this code updates. Do you foresee adding that code back in to maintain/support old behavior, or do you think what you've done will supersede these updates?

@thomas-robinson
Copy link
Member

There are two merge conflicts with the current dmUpdate specifically in diag_manager.F90

git remote add gfdl https://github.com/NOAA-GFDL/FMS.git
git remote add emc https://github.com/NOAA-EMC/FMS.git
git fetch gfdl
git fetch emc
git checkout gfdl/dmUpdate
git merge emc/fms_mixedmode

There will be more conflicts when the send_data work is merged in
#979

@MinsukJi-NOAA
Copy link
Contributor Author

There are two merge conflicts with the current dmUpdate specifically in diag_manager.F90

git remote add gfdl https://github.com/NOAA-GFDL/FMS.git
git remote add emc https://github.com/NOAA-EMC/FMS.git
git fetch gfdl
git fetch emc
git checkout gfdl/dmUpdate
git merge emc/fms_mixedmode

There will be more conflicts when the send_data work is merged in #979

Conflicts are in register_diag_field_scalar and register_diag_field_array functions. It seems that new functions register_diag_field_scalar_modern and register_diag_field_array_modern are being used. Maybe one way to move forward is to have the register_diag_field_scalar_old and register_diag_field_array_old in sync with emc fms_mixedmode register_diag_field_scalar and register_diag_field_array, respectively?

What are GFDL's plans are for *_modern and *_old functions down the road?

@thomas-robinson
Copy link
Member

@MinsukJi-NOAA we should be able to resolve these conflicts because they seem relatively straight forward. The "old" and "modern" routines are going to co-exist for some time. We will move forward with our development based off of your updates.

@thomas-robinson
Copy link
Member

@rem1776 We will merge in main and then merge this PR. We will tag this 2022.03-alpha1 and run testing.

@MinsukJi-NOAA
Copy link
Contributor Author

MinsukJi-NOAA commented Jun 15, 2022

@rem1776 @thomas-robinson I verified that functions register_diag_field_scalar_old and register_diag_field_array_old are the same as emc mixedmode register_diag_field_scalar and register_difag_field_array, respectively.

It also looks like send_data_3d is identical to the emc mixedmode send_data_3d.

@thomas-robinson
Copy link
Member

@MinsukJi-NOAA We had a few more updates, but they should be minor. The check for range being size 2 was moved and there were some typos. Otherwise I think we are good

Copy link
Member

@thomas-robinson thomas-robinson left a comment

Choose a reason for hiding this comment

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

The changes to the diag manager are now in line with our diag manager development, and I think we can merge this.

Copy link
Contributor

@bensonr bensonr left a comment

Choose a reason for hiding this comment

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

@binli2337 @MinsukJi-NOAA @junwang-noaa - as part of the 2022.02 FMS release, constants handling was rewritten to isolate the individual constants groups by laboratory/use. Please look at the new structure and propose a way to introduce the desired constant behavior/handling.

@MinsukJi-NOAA
Copy link
Contributor Author

@binli2337 @MinsukJi-NOAA @junwang-noaa - as part of the 2022.02 FMS release, constants handling was rewritten to isolate the individual constants groups by laboratory/use. Please look at the new structure and propose a way to introduce the desired constant behavior/handling.

@bensonr Could you point out the hash where these changes to constants were made?

@bensonr
Copy link
Contributor

bensonr commented Jun 16, 2022

@MinsukJi-NOAA - 16003bb

@junwang-noaa
Copy link

@bensonr I want to double check with you, my understanding is that in GFDL applications FMS will be compiled along with model code with corresponding compile options. So the mixed mode changes for constantsR4 have no impact on GFDL/GOES applications.

The constantR4 will only be added for the prebuilt FMS library in UFS. When running in mixed mode, dycore compiled with 32BIT=Y can still use the r4 constants. Will GFDL applications run in mixed mode coupled applications? What are the compile options for FMS you are going to use in this mixed mode runs?

@bensonr
Copy link
Contributor

bensonr commented Jun 16, 2022

@junwang-noaa - the bottom line is it doesn't conform to the current programming and macros for constants within FMS. Being that FMS is a community library, it is not in our best interest to have conflicting approaches which could cause confusion and future issues. In my opinion, leaving all the constants in full precision and casting them to the appropriate precision in the various components would be the correct solution.

GEOS does have the desire (and I believe the capability) of running mixed-mode. GFDL's SHiELD model has always run with mixed-mode and the climate researchers have expressed interest in exploring the capability, but have concerns about conservation that need to be satisfied.

@junwang-noaa
Copy link

@bensonr Thanks for the explanation. Bin will update the constantR4 to follow the same structure. But in general when mixed model FMS is used in the mixed model coupled runs (fms is compiled in R8, dycore runs in 32BIT/mom6 runs on 64BIT), we will need the hard coded R4 constantsR4 module used in dycore. Constants module can be used in all other configurations (fms compiled in r4 (r8) and dycore runs in r4 (r8), or fms compiled in r8 and mom6/dycore run in r8).

Copy link
Contributor

@bensonr bensonr left a comment

Choose a reason for hiding this comment

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

@binli2337 - you're still overcomplicated things here. You do not need to make copies of *_constants.h and hardcode them to use r4_kind. All you needed to do was set the definition of RKIND in fms_constants4.F90 and when it included *_constants.h, it would have the correct typing of the constants.

#define RKIND r4_kind

@rem1776 rem1776 merged commit 9d8542e into NOAA-GFDL:emc_mixedmode Jun 23, 2022
GFDL-Eric pushed a commit to GFDL-Eric/FMS that referenced this pull request Aug 8, 2022
adds mixed precison support for multiple areas of the code to support a single FMS library to handle running 32-bit FV3 and 64-bit MOM6 for the coupled UFS weather model

BREAKING CHANGE: Replaces real arguments with class(*) for routines in diag_manager, sat_vapor_pres, time_manager and tracer_manager
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants