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

Fix mosaic2 tests with strict debug flags #1597

Merged
merged 1 commit into from
Oct 11, 2024

Conversation

J-Lentz
Copy link
Contributor

@J-Lentz J-Lentz commented Oct 9, 2024

Description
This fixes two issues that occur in the mosaic2 tests when FMS is built using strict debug flags:

  1. The area argument of calc_mosaic_grid_area has been changed from intent(inout) to intent(out), to avoid an uninitialized value error.

  2. atan has been changed to atan2 in the unit tests, to avoid divide-by-zero errors.

How Has This Been Tested?
Fixes the mosaic2 unit tests when FMS is built with ifort 2021.13.0 on the AMD box with the following debug flags:
-check all -fpe0 -fp-stack-check -fstack-security-check -ftrapuv -init=arrays,minus_huge,snan

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

This fixes two issues that occur in the `mosaic2` tests when FMS is built using
strict debug flags:

1) The `area` argument of `calc_mosaic_grid_area` has been changed from
`intent(inout)` to `intent(out)`, to avoid an uninitialized value error.

2) `atan` has been changed to `atan2` in the unit tests, to avoid
divide-by-zero errors.
@laurenchilutti
Copy link
Contributor

@abrooks1085 Can you try this branch with ifx prod flags? See if it fixes the unit test crashes in mosaic2?

@@ -86,15 +86,13 @@
!> <br>Example usage:
!! call calc_mosaic_grid_area(lon, lat, area)
subroutine CALC_MOSAIC_GRID_AREA_(lon, lat, area)
real(kind=FMS_MOS_KIND_), dimension(:,:), intent(in) :: lon
real(kind=FMS_MOS_KIND_), dimension(:,:), intent(in) :: lat
real(kind=FMS_MOS_KIND_), dimension(:,:), intent(inout) :: area
Copy link
Contributor

Choose a reason for hiding this comment

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

I am asking purely for my own learning and do not know what is best: Why would you decide to change the intent instead of just initializing the output variable in the code before this subroutine is called?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CALC_MOSAIC_GRID_AREA_ is a wrapper around get_grid_area, which sets the area but doesn't read the value that's passed in. So intent(out) more clearly communicates to the user what the code is doing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Or in other words, there's no reason it should need to be initialized, because it gets set by get_grid_area.

@rem1776 rem1776 merged commit 95cd7b0 into NOAA-GFDL:main Oct 11, 2024
28 checks passed
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