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

[WIP] Please ignore this! An alternate fix to class(*) issues with FMS 2022-01 #68

Closed

Conversation

nikizadehgfdl
Copy link

  • This update is an alternate to PR#66 to fix the issues with passing
    read arguments to subroutines receiving class(*)
  • This tries to show that there are no inherent issues with passing a
    real and receiving it as class(*).
    Rather the root cause of the issues is some of these arguments are
    optional and are being passed to FMS even thought they are not present!
    Then they are trapped in FMS diag_manager inside a SELECT TYPE statement
    and the compiler is not smart enough to know that they are absent and bombs. 

- This update is an alternate to PR#66 to fix the issues with passing
read arguments to subroutines receiving class(*)
- This tries to show that there are no inherent issues with passing a
  real and receiving it as class(*).
  Rather the root cause of the issues is some of these arguments are
  optional and are being passed to FMS even thought they are not present!
  Then they are trapped in FMS diag_manager inside a SELECT TYPE statement
  and the compiler is not smart enough to know that they are absent and bombs. 
@codecov
Copy link

codecov bot commented Feb 10, 2022

Codecov Report

Merging #68 (b77de90) into dev/gfdl (df46be4) will increase coverage by 0.21%.
The diff coverage is 3.70%.

Impacted file tree graph

@@             Coverage Diff              @@
##           dev/gfdl      #68      +/-   ##
============================================
+ Coverage     28.82%   29.04%   +0.21%     
============================================
  Files           242      244       +2     
  Lines         71495    71928     +433     
============================================
+ Hits          20612    20893     +281     
- Misses        50883    51035     +152     
Impacted Files Coverage Δ
config_src/infra/FMS1/MOM_diag_manager_infra.F90 26.96% <3.70%> (-7.89%) ⬇️
src/framework/MOM_horizontal_regridding.F90 35.11% <0.00%> (-2.51%) ⬇️
src/parameterizations/lateral/MOM_hor_visc.F90 42.30% <0.00%> (-1.34%) ⬇️
src/framework/MOM_domains.F90 50.84% <0.00%> (-0.88%) ⬇️
src/tracer/ideal_age_example.F90 44.59% <0.00%> (-0.51%) ⬇️
src/core/MOM_checksum_packages.F90 30.82% <0.00%> (-0.48%) ⬇️
src/ALE/MOM_ALE.F90 34.44% <0.00%> (-0.24%) ⬇️
src/ALE/MOM_regridding.F90 21.36% <0.00%> (-0.10%) ⬇️
src/diagnostics/MOM_sum_output.F90 62.56% <0.00%> (-0.07%) ⬇️
src/core/MOM.F90 58.44% <0.00%> (ø)
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update df46be4...b77de90. Read the comment docs.

@marshallward
Copy link
Member

marshallward commented Feb 17, 2022

@nikizadehgfdl I tested the current dev/gfdl with FMS 2022.01-beta1 and there were no compile-time errors, or runtime errors in the .testing suite.

Is this a runtime error? Do I need to use a more complicated test, e.g. the production regression suite? Or a different FMS build?

(Forgot to add: I used GCC 11.2, have not yet tried Intel Fortran)

@jiandewang
Copy link

@marshallward @nikizadehgfdl Thanks for this information, we will do corresponding testing in UFS.
@nikizadehgfdl can you also make corresponding change in FMS2/infra code ?

@nikizadehgfdl
Copy link
Author

@marshallward, this is to avoid runtime errors and you need tests that have extensive diag_table, like OMIP,CM, ESM. If diag_table has no diagnostics that require mask or weight you won't run into any problem.

    - This update is an alternate to PR#66 to fix the issues with passing
    read arguments to subroutines receiving class(*)
    - This tries to show that there are no inherent issues with passing a
      real and receiving it as class(*).
      Rather the root cause of the issues is some of these arguments are
      optional and are being passed to FMS even thought they are not present!
      Then they are trapped in FMS diag_manager inside a SELECT TYPE statement
      and the compiler is not smart enough to know that they are absent and bombs. 
    - Added the same fix for infra/FMS2
- This update is an alternate to PR#66 to fix the issues with passing
  read arguments to subroutines receiving class(*)
- This tries to show that there are no inherent issues with passing a
  real and receiving it as class(*).
  Rather the root cause of the issues is some of these arguments are
  optional and are being passed to FMS even thought they are not present!
  Then they are trapped in FMS diag_manager inside a SELECT TYPE statement
  and the compiler is not smart enough to know that they are absent and bombs. 
- Added the same fix for infra/FMS2
- Fixed trailing spaces
@nikizadehgfdl
Copy link
Author

@jiandewang I updated infra/FMS2 the same way. My tests passed with FMS2022.01-beta1 (mixed-mode tag of FMS).

- This update is an alternate to PR#66 to fix the issues with passing
  read arguments to subroutines receiving class(*)
- This tries to show that there are no inherent issues with passing a
  real and receiving it as class(*).
  Rather the root cause of the issues is some of these arguments are
  optional and are being passed to FMS even thought they are not present!
  Then they are trapped in FMS diag_manager inside a SELECT TYPE statement
  and the compiler is not smart enough to know that they are absent and bombs. 
- Added the same fix for infra/FMS2
- Fixed trailing spaces
@MinsukJi-NOAA
Copy link

@jiandewang
I used your branch https://github.com/jiandewang/MOM6/tree/test/FMS-cherrypick-Niki to test the latest UFS weather model.
All regression tests passed on Hera and WCOSS Dell: ufs-community/ufs-weather-model#1036

@jiandewang
Copy link

@jiandewang I updated infra/FMS2 the same way. My tests passed with FMS2022.01-beta1 (mixed-mode tag of FMS).

@nikizadehgfdl thanks for the heads up and sorry for my late reply as I was out of town in the past week. We will do more testing and get back to you and Marshall.

@MinsukJi-NOAA
Copy link

@jiandewang
Regression tests on platforms Hera, WCOSS, and Gaea all passed using the latest changes in your MOM6 branch: https://github.com/jiandewang/MOM6/tree/test/FMS-cherrypick-Niki. These include intel, gnu, and debug tests.
Also, the RT's used the FMS2 infra by modifying the mom6_files.cmake file in the UFS weather model repository.

@jiandewang
Copy link

@jiandewang Regression tests on platforms Hera, WCOSS, and Gaea all passed using the latest changes in your MOM6 branch: https://github.com/jiandewang/MOM6/tree/test/FMS-cherrypick-Niki. These include intel, gnu, and debug tests. Also, the RT's used the FMS2 infra by modifying the mom6_files.cmake file in the UFS weather model repository.

@MinsukJi-NOAA can you provide information on which version of FMS library code being used in your test ?

@MinsukJi-NOAA
Copy link

@jiandewang Regression tests on platforms Hera, WCOSS, and Gaea all passed using the latest changes in your MOM6 branch: https://github.com/jiandewang/MOM6/tree/test/FMS-cherrypick-Niki. These include intel, gnu, and debug tests. Also, the RT's used the FMS2 infra by modifying the mom6_files.cmake file in the UFS weather model repository.

@MinsukJi-NOAA can you provide information on which version of FMS library code being used in your test ?

FMS2 is being used. See here

- This update is an alternate to PR#66 to fix the issues with passing
   read arguments to subroutines receiving class(*)
- This tries to show that there are no inherent issues with passing a
  real and receiving it as class(*).
  Rather the root cause of the issues is some of these arguments are
  optional and are being passed to FMS even thought they are not present!
  Then they are trapped in FMS diag_manager inside a SELECT TYPE statement
  and the compiler is not smart enough to know that they are absent and bombs. 
- Added the same fix for infra/FMS2
- Fixed trailing spaces and extra long lines
@marshallward
Copy link
Member

This PR will be sent directly to EMC, which is actively testing the FMS mixed precision support.

NOAA-EMC#88

We will eventually take this in via mom-ocean/MOM6 after EMC submits a PR in the future.

nikizadehgfdl added a commit to nikizadehgfdl/MOM6 that referenced this pull request Mar 7, 2023
- GNU executable crashes on send_data3d because it cannot handle
  optional input arguments that are declared as class(*) inside the
  callee interface, in this case rmask inside diag_manager::send_data_3d
  Such optional arguments have to be checked for and passed explicitly
  to send_data_3d
- This is similar to the issue in NOAA-GFDL#68
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants