2
\$\begingroup\$

This script fetches a list of "end-positions" from a MySQL database. The positions are array positions that are the same in both MySQL and R.

Next, the script takes the previous 34 elements for each metric and calculates their correlation with a query series.

The top X highest correlated (in total for each metric) and bottom X lowest correlated rows are saved.

Finally the initial MySQL list is trimmed to contain only those highly correlated points.

I am very new to R, and I am pretty sure that I did a very inefficient job of implementing this. I am looking for response times under 1 second in length.

#Executed during initialization: 
library(RODBC)
library(stats)

channel<- odbcConnect("data")
c<-mat.or.vec(3000,5) #will hold correlations
n1<-seq(-33,0)

z <- sqlQuery(channel,"SELECT RPos,M1,M2,M3,M4 FROM `data`.`z` ") #Get whole series
al_string <- "select RPos,OpenTime FROM z JOIN actionlist on(OpenTime = pTime)"
trim_string<- "DELETE FROM ActionList WHERE OpenTime NOT IN (SELECT OpenTime FROM ReducedList)"


#This segment is called repeatedly. Each time actionlist contains different positions.
actionlist<- sqlQuery(channel,al_string)

#SIMULATION: (x will be filled by something else in reality)
x <- sqlQuery(channel,"SELECT z.pTime AS pTime,M1,M2,M3,M4 FROM z JOIN (SELECT pTime FROM z ORDER BY rand() limit 1) AS X ON (z.pTime<=X.pTime AND z.pTime> x.pTime-(300*34))")
i<-1

This is the part I am most interested in speeding up:

GetTopN<-function(n)
{
  for(i in 1:nrow(actionlist))
  {
              c[i,1]<-actionlist$OpenTime[i];
              for(j in 2:ncol(z)) c[i,j]<-cor(z[actionlist$RPos[i]+n1,j],x[,j]);        
  }             
  avc     <- (cbind(c[,1],rowSums(c[,2:5])));
  topx    <- c[order(avc[,2], decreasing=T),1][1:n];
  bottomx <- c[order(avc[,2], decreasing=F),1][1:n];
  DF<-as.data.frame(c(topx,bottomx),row.names=NULL);
  colnames(DF)[1]<-"OpenTime";

     sqlSave(channel,dat=DF,tablename="ReducedList",append=FALSE,rownames=FALSE,safer=FALSE);
  sqlQuery(channel,trim_string);
}
\$\endgroup\$
  • 5
    \$\begingroup\$ Just off the top of my head: 1) Don't name things c. It's a very important base R function. 2) Now that you've renamed it, don't sort it twice. Sort it once, and then use head and tail. \$\endgroup\$ – joran Feb 7 '12 at 4:02
  • 3
    \$\begingroup\$ Also, change c[i,1]<-actionlist$OpenTime[i] to c[,1]<-actionlist$OpenTime and move it out of the loop. \$\endgroup\$ – Richie Cotton Feb 7 '12 at 11:33
  • 1
    \$\begingroup\$ Also, look at ?Rprof to know how you can check which part of your function is slowing everything down. \$\endgroup\$ – Joris Meys Feb 7 '12 at 15:31
2
\$\begingroup\$

The comments are good; I copy them here so this question has a "proper" answer, and add some my own:

Just off the top of my head: 1) Don't name things c. It's a very important base R function. 2) Now that you've renamed it, don't sort it twice. Sort it once, and then use head and tail. – joran

and

Also, change c[i,1]<-actionlist$OpenTime[i] to c[,1]<-actionlist$OpenTime and move it out of the loop. – Richie Cotton

and

Also, look at ?Rprof to know how you can check which part of your function is slowing everything down. – Joris Meys

This last is important; if you don't know where the code is slow, you don't know where to apply effort to speed it up.

My comments are that, if you have a function that is doing some work, all the information that it needs should generally be passed in as arguments. It took me a bit to figure out what c and z (after I overcame the cognitive dissonance of thinking c[i,1] was itself an error).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.