R devel - Aug 2004 - bus error /segmentation fault from 'approx' (PR#7177)

Full_Name: joerg  van den hoff
Version: 1.9.1
OS: MacOS  and SunOS 
Submission from: (NULL) (149.220.4.88)


follow up to ID 7166. something like

approx(c(1,2),c(NA,NA),1.5,rule=2)

crashes 1.9.1 on both systems (MacOS 10.3.5.: bus error,  SunOS 5.9:
segmentation fault) even if xout is within given x range (as in example above)
where rule=2 seems not be relevant anyway.

j.van_den_hoff@fz-rossendorf.de writes:
> follow up to ID 7166. something like
> 
> approx(c(1,2),c(NA,NA),1.5,rule=2)
> 
> crashes 1.9.1 on both systems (MacOS 10.3.5.: bus error,  SunOS 5.9:
> segmentation fault) even if xout is within given x range (as in example
above)
> where rule=2 seems not be relevant anyway.
Yes, this is a silly bug in the R driver routine:


    if (nx < 2 && method == "linear")
        stop("approx requires at least two values to interpolate")
    if (any(na <- is.na(x) | is.na(y))) {
        ok <- !na
        x <- x[ok]
        y <- y[ok]
        nx <- length(x)
    }

You want to do the check after removing NAs! 

Also, we should probably have a check for (nx == 0 && method !=
"linear")

-- 
   O__  ---- Peter Dalgaard             Blegdamsvej 3  
  c/ /'_ --- Dept. of Biostatistics     2200 Cph. N   
 (*) \(*) -- University of Copenhagen   Denmark      Ph: (+45) 35327918
~~~~~~~~~~ - (p.dalgaard@biostat.ku.dk)             FAX: (+45) 35327907

>>>>> "PD" == Peter Dalgaard
<p.dalgaard@biostat.ku.dk>
>>>>>     on 16 Aug 2004 12:01:20 +0200 writes:
    PD> j.van_den_hoff@fz-rossendorf.de writes:
    >> follow up to ID 7166. something like
    >> 
    >> approx(c(1,2),c(NA,NA),1.5,rule=2)
    >> 
    >> crashes 1.9.1 on both systems (MacOS 10.3.5.: bus error,  SunOS
5.9:
    >> segmentation fault) even if xout is within given x range (as in
example above)
    >> where rule=2 seems not be relevant anyway.

    PD> Yes, this is a silly bug in the R driver routine:


    PD> if (nx < 2 && method == "linear")
    PD> stop("approx requires at least two values to interpolate")
    PD> if (any(na <- is.na(x) | is.na(y))) {
    PD> ok <- !na
    PD> x <- x[ok]
    PD> y <- y[ok]
    PD> nx <- length(x)
    PD> }

    PD> You want to do the check after removing NAs! 

    PD> Also, we should probably have a check for (nx == 0 && method
!= "linear")
yes (2 times)  *and*  'method' is not a string anymore at that
moment because it has been pmatch()ed.

I'm going to match.arg() it instead.
And I see there's more cleanup possible, since,
xy.coords() already makes sure to return 2 equal length numeric
components.

When I do this now, it breaks back compatibility since things
like (PR#6809)
	approx(list(x=rep(NaN, 9), y=1:9), xout=NaN)
would give an error instead of (NaN, NaN).

OTOH, the stop("....") message you cite above was obviously
*intended* to be triggered for such a case.
I presume we rather intend to be back compatible?

Martin

>>>>> "MM" == Martin Maechler
<maechler@stat.math.ethz.ch>
>>>>>     on Mon, 16 Aug 2004 14:19:16 +0200 (CEST) writes:
>>>>> "PD" == Peter Dalgaard
<p.dalgaard@biostat.ku.dk>
>>>>>     on 16 Aug 2004 12:01:20 +0200 writes:
    PD> j.van_den_hoff@fz-rossendorf.de writes:
    >>> follow up to ID 7166. something like
    >>> 
    >>> approx(c(1,2),c(NA,NA),1.5,rule=2)
    >>> 
    >>> crashes 1.9.1 on both systems (MacOS 10.3.5.: bus error,
    >>> SunOS 5.9: segmentation fault) even if xout is within
    >>> given x range (as in example above) where rule=2 seems
    >>> not be relevant anyway.

      PD> Yes, this is a silly bug in the R driver routine:

      PD> if (nx < 2 && method == "linear")
      PD>    stop("approx requires at least two values to
interpolate")
      PD> if (any(na <- is.na(x) | is.na(y))) {
      PD>    ok <- !na
      PD>    x <- x[ok]
      PD>    y <- y[ok]
      PD>    nx <- length(x)
      PD> }

      PD> You want to do the check after removing NAs! 

      PD> Also, we should probably have a check for (nx == 0 &&
method != "linear")
    MM> yes (2 times)  *and*  'method' is not a string anymore at
that
    MM> moment because it has been pmatch()ed.

    MM> I'm going to match.arg() it instead.

(no, I didn't. It's not worth it, since the C code needs the
 integer version anyway.)

    MM> And I see there's more cleanup possible, since,
    MM> xy.coords() already makes sure to return 2 equal length numeric
    MM> components.

    MM> When I do this now, it breaks back compatibility since things
    MM> like (PR#6809)
    MM> approx(list(x=rep(NaN, 9), y=1:9), xout=NaN)
    MM> would give an error instead of (NaN, NaN).

    MM> OTOH, the stop("....") message you cite above was obviously
    MM> *intended* to be triggered for such a case.
    MM> I presume we rather intend to be back compatible?

I'm not so sure anymore:  For approxfun() {which must return a
function() in non-error cases} this would need extra code, just
for a somewhat non-sensical case.
And then, approx() should stay entirely "parallel" to approxfun().

S-plus also simply gives an error in these cases  {but AFAIR, R's
approx has tried to be better here...}

So, I propose to change behavior for all these cases and
produce an error,  for approx() from the following code part :

    if (nx <= 1) {
	if(method == 1)# linear
	    stop("need at least two non-NA values to interpolate")
	if(nx == 0) stop("zero non-NA points")
    }

Martin