Mixed precision topography_mod #1250

mcallic2 · 2023-06-07T14:57:41Z

Description
Adds mixed precision capabilities to topography_mod

Fixes # (issue)

How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Please also note
any relevant details for your test configuration (e.g. compiler, OS). Include
enough information so someone can reproduce your tests.

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

topography/include/topography.inc

topography/include/gaussian_topog.inc

mlee03 · 2023-08-29T17:04:44Z

topography/gaussian_topog.F90

@@ -105,9 +110,9 @@ module gaussian_topog_mod
 !! by variables in the namelist.
 subroutine gaussian_topog_init ( lon, lat, zsurf )


Is there a reason why this subroutine wasn't moved to the inc file?

mlee03 · 2023-08-29T17:07:35Z

topography/include/gaussian_topog.inc

-  wlat = 15.*dtr;  if (present(wlatd)) wlat=wlatd*dtr
-  rlon =  0.    ;  if (present(rlond)) rlon=rlond*dtr
-  rlat =  0.    ;  if (present(rlatd)) rlat=rlatd*dtr
+  olon = 90.0_lkind*dtr;  if (present(olond)) olon=olond*dtr


olon=real(olond*dtr, lkind) and for below too

mlee03 · 2023-08-29T17:08:13Z

topography/include/gaussian_topog.inc


 ! compute gaussian-shaped mountain
    do j=1,size(lat(:))
      dy = abs(lat(j) - olat)   ! dist from y origin
-      yy = max(0., dy-rlat)/wlat
+      yy = max(0.0_lkind, dy-rlat)/wlat


all module levels variables are declared as r8_kind. you need to convert them to lkind

mlee03 · 2023-08-29T17:11:58Z

topography/topography.F90

- integer :: namelen
+function open_topog_file ( )
+logical :: open_topog_file
+real(kind=r8_kind)    :: r_ipts, r_jpts


Im wondering if these could be r4_kind if the whole purpose of this subroutine is to open the topog_file and get ipts and jpts

mlee03 · 2023-08-29T17:41:27Z

@mcallic2 i made a PR to your branch

Mm topography

topography/include/gaussian_topog.inc

rem1776 · 2023-08-30T15:09:10Z

topography/include/gaussian_topog.inc

-!     The variables in this namelist are only used when routine
-!     <TT>gaussian_topog_init</TT> is called.  The namelist variables
-!     are dimensioned (by 10), so that multiple mountains can be generated.
+!> @brief Returns a simple surface height field that consists of a single


can you move this comment block to right above the function its documenting (get_gaussian_topog). If its not right above the routine it won't get parsed correctly.

fixed in b55e5da

You only moved the first 3 lines (the brief description), the rest ofit needs to be moved (lines 35-59)

I'm still lost as to why the comments for get_gaussian_topog is moved here above gaussian_topog_init...

Do lines 34-35 also need to be moved down?

I think they are fine, it'll just read the first !> as the brief descriptions and the second part as the expanded.

topography/include/topography.inc

mlee03 · 2023-08-30T17:29:33Z

topography/include/gaussian_topog.inc

-!     The variables in this namelist are only used when routine
-!     <TT>gaussian_topog_init</TT> is called.  The namelist variables
-!     are dimensioned (by 10), so that multiple mountains can be generated.
+!> The height, position, width, and elongation of the mountain


I think this comment block has been misplaced?

@rem1776 told me to move it for documentation, it's right above get_gaussian_topog now

it seems to be above gaussian_topog_init?

its below end subroutine gaussian_topog_init

#1250 (comment)

mcallic2 · 2023-08-31T14:03:45Z

@mlee03 @rem1776 friends, I went back and read through the comments. I understand what both of you are saying. I "reorganized" the comment blocks (in be19229) to coincide with their respective routines. Let me know if ya'll agree.

mlee03 · 2023-08-31T14:32:04Z

topography/include/gaussian_topog.inc

-  olat = 45.0_r8_kind*dtr;  if (present(olatd)) olat=real(olatd*dtr,r8_kind)
-  wlon = 15.0_r8_kind*dtr;  if (present(wlond)) wlon=real(wlond*dtr,r8_kind)
-  wlat = 15.0_r8_kind*dtr;  if (present(wlatd)) wlat=real(wlatd*dtr,r8_kind)
+  olon = 90.0_r8_kind*real(dtr, r8_kind);  if (present(olond)) olon=real(olond*dtr,r8_kind)


real(dtr,r8_kind) isn't necessary but ok.

I ran gcc with all the warnings and there were conversion errors, I can send you the path to it if you wanna look

BREAKING CHANGE: In coupler_types.F90, `coupler_nd_field_type` and `coupler_nd_values_type` have been renamed to indicate real kind value: `coupler_nd_real4/8_field_type` and `coupler_nd_real4/8_values_type`. The `bc` field within `coupler_nd_bc_type` was modified to use r8_kind within the value and field types, and an additional field added `bc_r4` to use r4_kind values. Includes: * feat: eliminate use of real numbers for mixed precision in `block_control` (#1195) * feat: mixed precision field_manager (#1205) * feat: mixed precision random_numbers_mod (#1191) * feat: mixed precision time_manager reals to r8 and clean up (#1196) * feat: mixed Precision tracer_manager (#1212) * Mixed precision monin_obukhov (#1116) * Mixed precision: `monin_obukhov` unit tests (#1272) * mixed-precision diag_integral_mod (#1217) * mixed precision time_interp (#1252) * mixed precision interpolator_mod (#1305) * Mixed precision astronomy (#1092) * Mixed precision `data_override_mod` (#1323) * mixed precision exchange (#1341) * coupler mixed precision (#1353) * Mixed precision topography_mod (#1250) --------- Co-authored-by: rem1776 <[email protected]> Co-authored-by: MiKyung Lee <[email protected]> Co-authored-by: mlee03 <[email protected]> Co-authored-by: Caitlyn McAllister <[email protected]> Co-authored-by: Jesse Lentz <[email protected]>

mcallic2 and others added 17 commits December 16, 2022 16:05

Seperate routines/functions

8a8f0ce

Create macros

ea2f24a

Adds include statements

cfe8491

Adds include and header files

75de129

Adds fms kind to real declarations

4cc88fc

Adds fms kind around real numbers

f3a29c2

Adds r8_kind to real variables and numbers in .F90 scripts

6003889

Adds interfaces, changes module procedures

cd1c409

Spelling mistake

b02be55

Fix include statement

3c496e4

Merge branch 'NOAA-GFDL:main' into topography

6434674

Merge mixedmode 2023.01_mm

8d62b7b

merge mixedmode

4b26098

add precision to module variables

e5dba69

fix spelling mistake

a355b8b

add kind parameter

bbc3911

merge

d6f4fb1

mcallic2 requested review from rem1776, thomas-robinson and bensonr as code owners June 7, 2023 14:57

mcallic2 marked this pull request as draft June 7, 2023 14:57

mcallic2 added 9 commits July 17, 2023 15:56

trim trailing whitespace

8ee9de3

add topography/include to cmake

a132932

change to flag path

08abb9c

begin adding unit tests for module

3b3e1fd

merge mixed mode

3b38ab5

forgetting include path

bb4d1a7

trim trailing whitespace

08c1ed7

that should stay an integer

6068731

need to stay integers

9195444

mlee03 suggested changes Aug 23, 2023

View reviewed changes

topography/include/topography.inc Outdated Show resolved Hide resolved

topography/include/topography.inc Outdated Show resolved Hide resolved

mcallic2 and others added 2 commits August 28, 2023 09:41

these are indices

b7400af

Merge branch 'mixedmode' into topography

7c63c01

mlee03 reviewed Aug 28, 2023

View reviewed changes

topography/include/gaussian_topog.inc Outdated Show resolved Hide resolved

mcallic2 added 3 commits August 29, 2023 09:21

removes unecessary precision

85b0345

merge mixedmode

37ddf2f

merge changes

6fde163

mlee03 suggested changes Aug 29, 2023

View reviewed changes

mlee03 reviewed Aug 29, 2023

View reviewed changes

mlee03 added 2 commits August 29, 2023 13:32

update topography

58cd3d1

change to r4_kind in open_file

9b2d811

Merge pull request #5 from mlee03/mm_topography

9dbd326

Mm topography

mlee03 reviewed Aug 30, 2023

View reviewed changes

topography/include/gaussian_topog.inc Outdated Show resolved Hide resolved

rem1776 requested changes Aug 30, 2023

View reviewed changes

mcallic2 added 4 commits August 30, 2023 13:16

change precision to r8_kind for mod variables

7be92cd

change comment block for documentation

b55e5da

fixed indentation

d9ed145

removes repetative random_numbers

f39b7c4

mlee03 reviewed Aug 30, 2023

View reviewed changes

mcallic2 added 2 commits August 30, 2023 14:06

remove print statement

6c7b785

moves comments

be19229

rem1776 approved these changes Aug 31, 2023

View reviewed changes

fixes conversion error

23548a3

mlee03 reviewed Aug 31, 2023

View reviewed changes

mlee03 approved these changes Aug 31, 2023

View reviewed changes

J-Lentz approved these changes Aug 31, 2023

View reviewed changes

rem1776 merged commit 497ab46 into NOAA-GFDL:mixedmode Aug 31, 2023
19 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mixed precision topography_mod #1250

Mixed precision topography_mod #1250

mcallic2 commented Jun 7, 2023 •

edited

Loading

mlee03 Aug 29, 2023

mlee03 Aug 29, 2023

mlee03 Aug 29, 2023

mlee03 Aug 29, 2023

mlee03 commented Aug 29, 2023

rem1776 Aug 30, 2023

mcallic2 Aug 30, 2023

rem1776 Aug 30, 2023

mlee03 Aug 31, 2023

mlee03 Aug 31, 2023

rem1776 Aug 31, 2023

mlee03 Aug 30, 2023

mcallic2 Aug 30, 2023

mlee03 Aug 30, 2023

mcallic2 Aug 30, 2023

rem1776 Aug 30, 2023

mcallic2 commented Aug 31, 2023

mlee03 Aug 31, 2023

mcallic2 Aug 31, 2023

		@@ -105,9 +110,9 @@ module gaussian_topog_mod
		!! by variables in the namelist.
		subroutine gaussian_topog_init ( lon, lat, zsurf )

Mixed precision topography_mod #1250

Mixed precision topography_mod #1250

Conversation

mcallic2 commented Jun 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mlee03 commented Aug 29, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcallic2 commented Aug 31, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mcallic2 commented Jun 7, 2023 •

edited

Loading