From henrik.bengtsson at ucsf.edu  Wed May 13 18:20:43 2015
From: henrik.bengtsson at ucsf.edu (Henrik Bengtsson)
Date: Wed, 13 May 2015 09:20:43 -0700
Subject: [Rd] Why is the diag function so slow (for extraction)?
In-Reply-To: <CAFDcVCQKCpJxkXq-gKcKg0UkAWS16mp8UJEQec83=42bQuFy2g@mail.gmail.com>
References: <1430762350040-4706780.post@n4.nabble.com>
	<06F749E7-5096-438D-A299-667C7A77B477@gmail.com>
	<alpine.DEB.2.10.1505050844470.2460@luke-Latitude>
	<CAAVP=akUU10FPvkYhNBVQzi9jsMkCma-VVGgSqjGMpKF+G_4zA@mail.gmail.com>
	<21841.62252.436947.835265@stat.math.ethz.ch>
	<CAFDcVCQKCpJxkXq-gKcKg0UkAWS16mp8UJEQec83=42bQuFy2g@mail.gmail.com>
Message-ID: <CAFDcVCTs+EXzjfNJAygaq2-oe19-EvVjwZgLHfxLGDA9i_fstQ@mail.gmail.com>

As kindly pointed out to me (oh my decaying gray matter), is.object()
is better suited for this test;

$ svn diff src/library/base/R/diag.R
Index: src/library/base/R/diag.R
===================================================================
--- src/library/base/R/diag.R   (revision 68345)
+++ src/library/base/R/diag.R   (working copy)
@@ -23,9 +23,11 @@
             stop("'nrow' or 'ncol' cannot be specified when 'x' is a matrix")

         if((m <- min(dim(x))) == 0L) return(vector(typeof(x), 0L))
+        nms <- dimnames(x)
+        nrow <- dim(x)[1L]
         ## NB: need double index to avoid overflows.
-        y <- c(x)[1 + 0L:(m - 1L) * (dim(x)[1L] + 1)]
-        nms <- dimnames(x)
+        if (is.object(x)) x <- c(x)

/Henrik

On Tue, May 12, 2015 at 8:24 PM, Henrik Bengtsson
<henrik.bengtsson at ucsf.edu> wrote:
> Along Luke's lines, would(n't) it be enough to look for existence of
> attribute 'class' to decide whether to dispatch or not, i.e. if c() is
> needed or not?  Even without .subset(), there is a remarkable
> improvement.  I think it's worth condition the code on dispatch or
> not.  For example:
>
> [HB-X201]{hb}: svn diff diag.R
> Index: diag.R
> ===================================================================
> --- diag.R      (revision 68345)
> +++ diag.R      (working copy)
> @@ -23,9 +23,11 @@
>              stop("'nrow' or 'ncol' cannot be specified when 'x' is a matrix")
>
>          if((m <- min(dim(x))) == 0L) return(vector(typeof(x), 0L))
> +        nms <- dimnames(x)
> +        nrow <- dim(x)[1L]
>          ## NB: need double index to avoid overflows.
> -        y <- c(x)[1 + 0L:(m - 1L) * (dim(x)[1L] + 1)]
> -        nms <- dimnames(x)
> +        if (!is.null(attr(x, "class"))) x <- c(x)
> +        y <- x[1 + 0L:(m - 1L) * (nrow + 1)]
>          if (is.list(nms) && !any(sapply(nms, is.null)) &&
>              identical((nm <- nms[[1L]][seq_len(m)]), nms[[2L]][seq_len(m)]))
>              names(y) <- nm
>
> ?
>
> /Henrik
>
> On Tue, May 12, 2015 at 5:33 AM, Martin Maechler
> <maechler at lynne.stat.math.ethz.ch> wrote:
>>>>>>> Steve Bronder <sbronder at stevebronder.com>
>>>>>>>     on Thu, 7 May 2015 11:49:49 -0400 writes:
>>
>>     > Is it possible to replace c() with .subset()?
>>
>> It would be possible, but I think "entirely" wrong.
>>
>> .subset() is documented to be an internal function not to be
>> used "lightly" and more to the point it is documented to *NOT*
>> dispatch at all.
>>
>> If you read and understood what Peter and Luke wrote, you'd not
>> special case here:
>>
>> diag() should not work only for pure matrices, but for all
>> "matrix-like" objects for which ``the usual methods'' work, such
>> as
>>    as.vector(.), c(.)
>>
>> That's why there has been the c(.) in there.
>>
>> You can always make code faster if you write the code so it only
>> has to work in one special case .. and work there very fast.
>>
>>
>>     > Example below
>>     > ####
>>     > ####
>>
>>     > library(microbenchmark)
>>
>> ______________________________________________
>> R-devel at r-project.org mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel



[Rd] Why is the diag function so slow (for extraction)?
Henrik Bengtsson