earth-system-radiation / rte-rrtmgp

RTE+RRTMGP is a set of codes for computing radiative fluxes in planetary atmospheres.
BSD 3-Clause "New" or "Revised" License
74 stars 65 forks source link

Adjust tolerances, fix error in accelerator source function kernel #258

Closed RobertPincus closed 7 months ago

RobertPincus commented 7 months ago

In the course of adjusting tolerances to match @skosukhin's values I uncovered a bug in the LW source function. This PR includes the bug fix and the adjusted tolerances, so more tests pass (nvfortran 23.9 is still failing with a missing pointer)

RobertPincus commented 7 months ago

Also in this PR is changing the testing and example scripts to fail when individual tests fail (as in #256)

skosukhin commented 7 months ago

It looks like this makes Cray 16.0.1 happy as well.

Regarding the failing NVHPC 23.9 tests, the following quick and dirty workaround fixes the issue:

--- a/rrtmgp-frontend/mo_gas_optics_rrtmgp.F90
+++ b/rrtmgp-frontend/mo_gas_optics_rrtmgp.F90
@@ -306,12 +306,20 @@ contains
     ! Interpolate source function
     ! present status of optional argument is passed to source()
     !
+    if(present(tlev)) then
     error_msg = source(this,                               &
                        ncol, nlay, nband, ngpt,            &
                        play, plev, tlay, tsfc,             &
                        jtemp, jpress, jeta, tropo, fmajor, &
                        sources,                            &
                        tlev)
+    else
+    error_msg = source(this,                               &
+                       ncol, nlay, nband, ngpt,            &
+                       play, plev, tlay, tsfc,             &
+                       jtemp, jpress, jeta, tropo, fmajor, &
+                       sources)
+    end if
     if(present(tlev)) then
       !$acc        exit data      delete(tlev)
       !$omp target exit data map(release:tlev)

@alexeedm looks like a compiler bug after all

RobertPincus commented 7 months ago

@skosukhin Ack! The failing tests made me think the change in 5ac4d0a was safe, but it was not I guess...