-
Notifications
You must be signed in to change notification settings - Fork 18
2018 06 01 test efiring simplify branch in GSW R
Incorporating GSW-C ‘efiring-simplify’ branch within GSW-R
Dan Kelley, Dalhousie University
2018-06-01
Abstract. The GSW-R branch "newc" incorporates the GSW-C branch
"efiring-simplify" cleanly, after adding two new arguments to the R version of
the gsw_geo_strf_dyn_height_1 function. All existing check values (covering
217 functions) continue to work properly, on multiple machine architectures and
both the release and development versions of R.
cd ~/git/GSW-C
git checkout master
git checkout -b efiring-simplify master
git pull https://github.com/efiring/GSW-C.git simplify
I think these are the relevant files. Perhaps E Firing can comment?
EF: I would expect gsw_saar.c and gsw_saar_data.c to be needed as well.
cd ~/git/GSW-R
cp ~/git/GSW-C/gsw_internal_const.h .
cp ~/git/GSW-C/gsw_oceanographic_toolbox.c .
cp ~/git/GSW-C/gswteos-10.h .
Since this is built up from the gsw_data_v3_0.nc file from GSW-Fortran, we merely
need to check that this file has not changed, and
md5 ~/git/GSW-C/gsw_data_v3_0.nc
md5 ~/git/GSW-Fortran/test/gsw_data_v3_0.nc
reveals that to be the case, both values being
dacb3f981e8e710ac2e83477701b39051. Thus, there is no need to rebuild the
data/saar.rda file used by GSW-R.
I had to modify gsw_geo_strf_dyn_height_1 to accept new args p_ref and
interp_method. This involved minor changes, as below (focusing only on files
I altered):
git diff HEAD^1 --stat
R/gsw.R | 9 +-
src/wrappers.c | 4 +-
The updated GSW-R builds and tests cleanly on the author's MacOS R-release,
Apple LLVM 8.0.0 (clang-800.0.42.1) system, as well as on the following,
as checked by rhub::check_for_cran():
(1) Ubuntu Linux 16.04 LTS, R-release, GCC;
(2) Debian Linux, R-devel, GCC ASAN/UBSAN;
(3) Fedora Linux, R-devel, clang, gfortran;
and
(4) Windows Server 2008 R2 SP1, R-devel, 32/64 bit.
Note that these tests involve (a) doc-code alignment on parameter names and order and (b) numerical tests of check values from the Matlab webpages. The tests are quite extensive, e.g.
git grep expect_|wc
217 1079 19936
indicates that several hundred variables are checked. The number of check values is likely about 3 to 5 times that; it depends on how many check values are given for each variable on the Matlab webpages.
Updating GSW-R to accept the new GSW-C code from the "efiring-simplify" branch
was a simple process. Fairly extensive build-tests (with various compilers,
OSs, and R versions) suggest that GSW-R users will not be affected by the
switch. The only exception is for users who employ gsw_geo_strf_dyn_height,
but since this is broken (with memory leaks and segfaults in some cases), it seems
reasonable to assume that its users will welcome the recommended replacement,
gsw_geo_strf_dyn_height_1).
PS. GSW-R users would probably like to know whether they get the same results
for gsw_geo_strf_dyn_height_1 as GSW-Python users get. Perhaps E Firing can
supply such values, so I could incorporate them in the docs (and thus,
automatically, in the test suite).
EF: Agreed, we will work that out.