Mixed precision random_numbers_mod #1191

J-Lentz · 2023-04-12T15:38:42Z

Initial implementation of mixed precision random_numbers_mod and unit 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

Initial implementation of mixed precision random_numbers_mod.

Rather than requiring that sample moments be within 5% of their expected values, calculate the standard deviations of sample moments and require that each sample moment be within one standard deviation of its expected value.

Re-order the subroutines to make the CI pass.

Re-order the unit test subroutines (again) in an effort to make the github CI pass.

Rather than re-ordering the subroutines, use an abstract interface to specify the dummy argument by which either test_sample_1d or test_sample_2d is passed.

mlee03 · 2023-05-24T17:30:01Z

random_numbers/include/random_numbers.inc

    type(randomNumberStream), intent(inout) :: stream
-    real,                     intent(  out) :: number
+    real(RN_KIND_),           intent(  out) :: number


could RN_KIND_ be changed to FMS_RN_KIND_? :)

Changed in fc57559

mlee03 · 2023-05-24T17:31:35Z

random_numbers/mersennetwister.F90

@@ -92,6 +92,8 @@
 !> @{
 module MersenneTwister_mod
 ! -------------------------------------------------------------
+  use platform_mod, only: r8_kind
+


line 106, could -2147483648_8 be changed to UMASK = -2147483648_i8_kind?

It could... but the value gets assigned to a 4-byte integer, so I'm pretty confused what the original author had in mind here.

I've removed the _8 in fc57559... but I'm feeling a bit uneasy about it, since someone presumably put it there for a reason that's not clear to me.

It seems that the _8 was a workaround to make it compile with GNU.

I've replaced this constant, as well as several others, with hexadecimal representations in 1d3f321.

Trying to make the hexadecimal constants work with older versions of gfortran brought me down a rabbit hole that I don't wish to explore any further. So I'm just going to revert those constants back to decimal values, leave an _i8_kind where the _8 was, and stop asking questions... some stones are better left unturned......

mlee03 · 2023-05-24T17:32:44Z

test_fms/random_numbers/Makefile.am

+test_random_numbers_r4_SOURCES = test_random_numbers.F90
+test_random_numbers_r8_SOURCES = test_random_numbers.F90
+
+test_random_numbers_r4_CPPFLAGS = $(AM_CPPFLAGS) -DKIND_=r4_kind


-DTEST_RN_KIND_ (just for consistency with the other unit tests)

Changed in fc57559

mlee03 · 2023-05-24T17:39:08Z

test_fms/random_numbers/test_random_numbers.F90

+
+  do i=1, n_moments
+    moment_mu(i) = CALC_MOMENT_(i)
+    moment_sigma(i) = sqrt((CALC_MOMENT_(2*i) - moment_mu(i)**2) / n)


Changed in fc57559

mlee03 · 2023-05-31T16:21:09Z

test_fms/random_numbers/test_random_numbers.F90

+integer, parameter :: n0_1d = 2500 !> Initial length of 1D random sample vector
+integer, parameter :: n0_2d = 50 !> Initial dimensions of 2D random sample array
+
+integer, parameter :: seeds(0:*) = [0, -5, 3] !> Seed constants


You're a Fortran guru, this is the first time I'm seeing something like this lol. Does this notation work with all compilers?

I've tested it with Intel, GNU, and Cray and it works with all of them. It seems that assumed-size arrays are actually older than the dinosaurs (they're a Fortran 77 feature), while square bracket array constructors are an F2003 feature.

can you confirm that this works with the nvdia compilers?

Nope, it doesn't work in nvfortran 23.3. It gets the lower index wrong.

Changed to a 1-indexed array in b978a99.

mlee03 · 2023-05-31T16:23:18Z

test_fms/random_numbers/test_random_numbers.F90

+call fms_init
+
+if (mpp_npes() .ne. 4) then
+  call mpp_error(fatal, "The random_numbers unit test requires four PEs")


Should it be FATAL or should it be a NOTE/WARNING and then exit the program? Or maybe check at the script level and if 4 pes are not available, skip the test...

What's the problem with using FATAL? I believe it should end up with four PEs any time it's run through the script, so this is really just a sanity check that shouldn't fail under normal circumstances.

mlee03 · 2023-05-31T16:53:53Z

test_fms/random_numbers/test_random_numbers.F90

+end subroutine test_getRandomNumbers
+
+! Expression for the expectation value of the i-th raw moment
+#define CALC_MOMENT_(i) (1._k / real(i + 1, k))


where did this equation come from?

The m-th raw moment of a distribution, by definition, is the integral of x^m times f(x), where f(x) is the distribution's probability density function. For the uniform distribution, f(x)=1 for 0<x<1, and f(x)=0 outside of that range. So the m-th moment is the integral from 0 to 1 of x^m, which is 1/(m + 1).

It's been a while since I thought about this stuff. Why is it a uniform distribution?

The comments in random_number_mod don't seem to explicitly state that the distribution is uniform. But, I think the term "random number generator" usually tends to mean "uniform random number generator", unless some other distribution is explicitly stated. The [0, 1] output range is also a hint that it's intended to output uniform random numbers.

mlee03 · 2023-05-31T16:56:18Z

test_fms/random_numbers/test_random_numbers.F90

+end function test_sample_2d
+
+!> Check that the first 1,000 moments of a sample are within one standard
+!> deviation of their expected values


why one standard deviation? do we know what the distribution function looks like?

One standard deviation was an empirical choice, based on the observation that the test passes within a couple seconds. But if we need the test to run faster, we could use a more lenient condition like 1.5 or 2 standard deviations.

I believe the central limit theorem implies that the distribution of sample moments should be approximately Gaussian, at least in the limit as the sample size approaches infinity.

Several constants in mersennetwister.F90 have been replaced with hexadecimal representations.

uramirez8707 · 2023-06-01T17:31:30Z

test_fms/random_numbers/test_random_numbers.F90

+integer, parameter :: n0_1d = 2500 !> Initial length of 1D random sample vector
+integer, parameter :: n0_2d = 50 !> Initial dimensions of 2D random sample array
+
+integer, parameter :: seeds(0:*) = [0, -5, 3] !> Seed constants


can you confirm that this works with the nvdia compilers?

Avoid using a 0-indexed array to work around an nvfortran bug.

ganganoaa

Looks fine to me.

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 <Ryan.Mulhall@noaa.gov> Co-authored-by: MiKyung Lee <58964324+mlee03@users.noreply.github.com> Co-authored-by: mlee03 <Mikyung.Lee@lscamd50-d.gfdl.noaa.gov> Co-authored-by: Caitlyn McAllister <65364559+mcallic2@users.noreply.github.com> Co-authored-by: Jesse Lentz <42011922+J-Lentz@users.noreply.github.com>

Mixed precision random_numbers_mod

6f2d9e6

Initial implementation of mixed precision random_numbers_mod.

J-Lentz requested review from rem1776, thomas-robinson and bensonr as code owners April 12, 2023 15:38

Jesse Lentz added 3 commits May 19, 2023 13:53

Add unit tests for random_numbers_mod

cbda086

Merge branch 'mixedmode' into random_numbers

aa57517

Rename kind macro to RN_KIND_

23bf25f

J-Lentz requested review from mlee03 and mcallic2 May 19, 2023 18:22

Jesse Lentz added 6 commits May 22, 2023 12:42

Use standard deviations of sample moments

d9e7cb0

Rather than requiring that sample moments be within 5% of their expected values, calculate the standard deviations of sample moments and require that each sample moment be within one standard deviation of its expected value.

Re-order subroutines

716a007

Re-order the subroutines to make the CI pass.

Re-order subroutines in unit test

60ac3e8

Re-order the unit test subroutines (again) in an effort to make the github CI pass.

Use abstract interface for procedure dummy argument

f9f35ef

Rather than re-ordering the subroutines, use an abstract interface to specify the dummy argument by which either test_sample_1d or test_sample_2d is passed.

Improve comments in unit tests

c3857ca

Fix typo in import statement

ef7e8c6

mcallic2 approved these changes May 24, 2023

View reviewed changes

mlee03 reviewed May 31, 2023

View reviewed changes

J-Lentz requested a review from uramirez8707 May 31, 2023 17:15

Jesse Lentz added 4 commits May 31, 2023 17:40

Review comments addressed

fc57559

Express constants as hexadecimal values

1d3f321

Several constants in mersennetwister.F90 have been replaced with hexadecimal representations.

Add int() typecast to hexadecimal constants

7623796

Mersenne Twister: Revert constants to decimal values

953cab6

uramirez8707 approved these changes Jun 1, 2023

View reviewed changes

Avoid using a 0-indexed array

b978a99

Avoid using a 0-indexed array to work around an nvfortran bug.

mcallic2 approved these changes Jun 2, 2023

View reviewed changes

J-Lentz requested a review from ganganoaa June 2, 2023 16:26

ganganoaa reviewed Jun 2, 2023

View reviewed changes

mlee03 approved these changes Jun 5, 2023

View reviewed changes

Merge branch 'mixedmode' into random_numbers

6c64b74

rem1776 merged commit 9f3c791 into NOAA-GFDL:mixedmode Jun 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mixed precision random_numbers_mod #1191

Mixed precision random_numbers_mod #1191

J-Lentz commented Apr 12, 2023 •

edited

Loading

mlee03 May 24, 2023

J-Lentz May 31, 2023

mlee03 May 24, 2023

J-Lentz May 31, 2023

J-Lentz May 31, 2023

J-Lentz May 31, 2023

J-Lentz Jun 1, 2023

mlee03 May 24, 2023

J-Lentz May 31, 2023

mlee03 May 24, 2023

J-Lentz May 31, 2023

mlee03 May 31, 2023

J-Lentz Jun 1, 2023

uramirez8707 Jun 1, 2023

J-Lentz Jun 1, 2023

J-Lentz Jun 1, 2023

mlee03 May 31, 2023 •

edited

Loading

J-Lentz May 31, 2023

mlee03 May 31, 2023

J-Lentz May 31, 2023

mlee03 Jun 2, 2023

J-Lentz Jun 2, 2023

mlee03 May 31, 2023

J-Lentz May 31, 2023

uramirez8707 Jun 1, 2023

ganganoaa left a comment

Mixed precision random_numbers_mod #1191

Mixed precision random_numbers_mod #1191

Conversation

J-Lentz commented Apr 12, 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

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

Choose a reason for hiding this comment

mlee03 May 31, 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

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

ganganoaa left a comment

Choose a reason for hiding this comment

J-Lentz commented Apr 12, 2023 •

edited

Loading

mlee03 May 31, 2023 •

edited

Loading