Skip to content

bug in acf (PR#9360)

6 messages · aimcleod at uwo.ca, A.I. McLeod, Duncan Murdoch

#
Full_Name: Ian McLeod
Version: 2.3.1
OS: Windows
Submission from: (NULL) (129.100.76.136)
This is certainly a bug.

There are two problems:

(i) the error message is wrong since lag.max is set to 1.  Perhaps, if the
function acf can not be used for in this situaiton, a different error message
would be more appropriate.  I understand why this might be done but I don't
think it is the best approach.

(ii) Please look at the function GetB which is attached.  This is part a
computation for a fast algorithm for exact mle of mean.  Usually phi here are
the coefficients from a high order AR but when I tried for AR(1) I got the error
message.  So the workaround is given.  Notice that I use: 

p*as.vector(acf(phi,lag.max=p,type="covariance",demean=FALSE,plot=FALSE)$acf)

so what I expect to get when p=length(phi)=1 is just phi^2.  This is what
happens in Mathematica with ListCorrelate[{phi},{phi}].  When you have
acf="correlation" and demean=TRUE then one gets 0/0 which should be defined as 1
in this situation.

Probably if the R authors just want to use acf for data analysis they may simply
choose to require length(x)>1 in acf(x,...) although I don't see the harm in my
suggestion either.

Ian McLeod
#
On 11/13/2006 10:30 AM, aimcleod at uwo.ca wrote:
I'd say it's a documentation bug, rather than a code bug.
What happens is that lag.max is reduced to length(z)-1, which is zero in 
your case.  This change should be mentioned in the documentation.
I don't think that's a reasonable expectation.  You've got an empty sum 
in the formula for the lag 1 autocovariance:

sum_{i=1}^0 phi_i phi_{i+1}

R is assuming that's not what you meant and is reporting it as an error. 
  If it gave you any value, it should be zero, not phi^2.

Duncan Murdoch
#
********************************************
I agree the empty sum which is the lag 1 autocovariance should be zero but this is the SECOND term in $acf output.
For the first term,
1) if demean=F, it is variance which is phi^2 as I suggested
2) if demean=T, it is the variance/variance = 0/0 which I said should best be 1


----- Original Message ----- 
From: "Duncan Murdoch" <murdoch at stats.uwo.ca>
To: <aimcleod at uwo.ca>
Cc: <r-devel at stat.math.ethz.ch>; <R-bugs at biostat.ku.dk>
Sent: Monday, November 13, 2006 11:22 AM
Subject: Re: [Rd] bug in acf (PR#9360)
#
Here is the result from S-Plus V.8.
Call: acf(x = 1, lag.max = 1, type = "covariance", plot = F)

Autocovariances matrix:
  lag X2
1   0  0
2   1  0

Their function does not support the demean option, so the variance is zero but they set 0/0 to 0 instead of 1.  The empty sum is 
zero (that is a standard mathematical convention which S+ follows).  I still think *one* would be better for the correlation at lag 
*zero* since the autocorrelation function at lag *zero* is defined as this in every other case.  As I tried to explain before for 
some algorithms involving the computation of correlations and convolutions, it is more convenient to have this first term (lag zero) 
always set to *one*.  Otherwise it has be treated as a special case as in my GetB function which I showed you.

I guess it is not that important but I think it should be treated as more than a documentation bug!!  Knuth once said that after TeX 
reach Version 3.14 all bugs would just be declared "special features".  That would be another way to handle too.

For "canned data analysis" as opposed to "programming", it really doesn't matter at all how you handle this problem.

Ian McLeod


----- Original Message ----- 
From: "Duncan Murdoch" <murdoch at stats.uwo.ca>
To: <aimcleod at uwo.ca>
Cc: <r-devel at stat.math.ethz.ch>; <R-bugs at biostat.ku.dk>
Sent: Monday, November 13, 2006 11:22 AM
Subject: Re: [Rd] bug in acf (PR#9360)
#
On 11/13/2006 2:24 PM, A.I. McLeod wrote:
Okay, I see what you mean now.  Yes, I agree acf should return lag 0 
autocorrelations and autocovariances even for a series of length 1. 
I'll take a look at the code.

Duncan Murdoch
#
On 11/13/2006 2:24 PM, A.I. McLeod wrote:
I've put this change into R-devel and R-patched now.

Duncan Murdoch