search

TEOS-10 / GSW-C

C implementation of the Thermodynamic Equation Of Seawater - 2010 (TEOS-10)
Other
18 stars 17 forks source link

NaN value for latitude or longitude causes segfault in gsw_saar #7

Closed petercable closed 7 years ago

petercable commented 7 years ago

What is the expected behavior? Should I replace NaN values in the input data with something else?

hylandg commented 7 years ago

Crap In, Crap Out ...

I'm not surprised with this behaviour - the lat/long values in gsw_saarare used to calculate an index into an array so a NaN value will give an out-of-bounds segfault. There is code to check

if (lat < -86.0 || lat > +90.0) ...

but a NaN will not satisfy this test. You will need to check for the presence of NaNs before calling gsw_saar. Use the usual trick ...

if (lat != lat) lat = 91.0;

to weed them out.

petercable commented 7 years ago

Understood. I can't imagine I'm the only one who will hit this issue. Would you be interested in a patch with something like this?

if (isnan(lat) | isnan(lon))
    return (return_value)
hylandg commented 7 years ago

A thorny (and philosophical) issue ... to what extent should routines check input arguments?

The Matlab code checks some arguments quite extensively, but even gsw_SAAR.m doesn't check if lat/lon are NaN. The Fortran version was coded with ocean models in mind. So efficiency was the primary aim, where parameters would be checked at a high level (and once per model iteration), rather than each time a low-level function was called. The C code currently follows the Fortran code, but only for expediency and historical reasons. It doesn't have to continue that way - depends on the user community and what they want (just a matter of making a case ...).

I won't be changing the Fortran code, but for the C-code it's not my call.

I suggest for the moment you just include your suggested code into your own calling routine.

Cheers, Glenn.

petercable commented 7 years ago

Works for me... Thanks!