Take the 2-minute tour ×
Mathematica Stack Exchange is a question and answer site for users of Mathematica. It's 100% free, no registration required.

This question already has an answer here:

See the example below. Is there any way to make this execute much faster and still get the same output ? I have read Conditionals slower than operators? but it didn't really help me as I can't seem to apply these methods to my problem. I have to execute about 100 of these operations in succession and each one can't take about a second to execute as running the algorithm would take way too long for the end user.

CurrentEquipID = 40

40

AbsoluteTiming[
 Table[If[HTimeModelSelection[CurrentEquipID][[i]] == 
    0, (1 - HOperatingEfficiency[CurrentEquipID][[i]])*
    HUtilizedTime[CurrentEquipID][[i]], 
   Flatten[ConstantArray[
      Select[EquipParams, #[[colEquipID]] == CurrentEquipID &][[;; , 
        colOperatingDelayTime]], 20]][[i]]], {i, 1, 20}]]

{1.851185, {744.6512332, 744.6512332, 746.713979, 744.6512332, 
  744.6512332, 744.6512332, 746.713979, 744.6512332, 744.6512332, 
  744.6512332, 746.713979, 744.6512332, 744.6512332, 744.6512332, 
  746.713979, 744.6512332, 744.6512332, 744.6512332, 746.713979, 
  744.6512332}}
share|improve this question
2  
I'm not going to try to work out exactly what your incomplete code is doing but Select inside a loop may be slow. Consider creating a hash table for each value (or position) rather than finding it with Select. –  Mr.Wizard Jun 11 '13 at 13:05
    
@Mr.Wizard its worse than that, that entire branch is redundant, and should be excised. See my answer. –  rcollyer Jun 11 '13 at 13:40
1  
Considering how common an issue this is, I'm voting to close in favor of a comprehensive question that you should read. –  rcollyer Jun 11 '13 at 13:48
add comment

marked as duplicate by rcollyer, Sjoerd C. de Vries, Yves Klett, Oleksandr R., m_goldberg Jun 11 '13 at 21:29

This question has been asked before and already has an answer. If those answers do not fully address your question, please ask a new question.

2 Answers

I was going to leave a comment, but without seeing the full code, I think it can be improved drastically.

At fault is that you are performing the same search every time through the loop. (If has the attribute HoldRest, so the branches are not executed unless they are used which means the search is re-executed every through.) So, at a minimum move the Select statement outside of the loop.

 currentEP = Select[EquipParams, #[[colEquipID]] == CurrentEquipID &][[;; , 
    colOperatingDelayTime]];

Additionally, your use of ConstantArray[..., 20][[i]] is redundant, and should be eliminated in its entirety. Replace it with currentEP. Lastly, using Table to index a List is inelegant; there are better ways. Consider this use of MapThread:

MapThread[
 If[ #1 == 0, (1 - #2) #3, Evaluate@Flatten[currentEP]]&,
 {
  HTimeModelSelection[CurrentEquipID],
  HOperatingEfficiency[CurrentEquipID],
  HUtilizedTime[CurrentEquipID]
 }
]

The nice thing about this construct is you do not need to know what iteration your on, simplifying your code. Also, if the lists change in size, you do not need to change the code. A caveat is that they must all be the same size, or MapThread will complain, loudly. But, there are other methods ...

share|improve this answer
    
I think that Evaluate@Flatten@currentEP should be a real number -- the OP extracts part i from the flattened list. Probably Select returns just one element, but who knows. –  Michael E2 Jun 11 '13 at 16:31
    
@MichaelE2 you are likely right, but I was not assuming anything, particularly with Flatten used after the OP extracts the info from the list. But, Evaluate could be removed, as I believe Flatten is very fast, and the list is very short. –  rcollyer Jun 11 '13 at 16:38
    
Anyway, I posted alternatives, but I think your approach is easier to understand. Unitize, UnitStep, etc. and vectorized arithmetic seems a bit harder to me. –  Michael E2 Jun 11 '13 at 19:25
add comment

Without knowing much about the data, it seems likely that it consists of numbers, and the the times and efficiencies are positive real numbers. Further I have to guess that

Select[EquipParams, #[[colEquipID]] == CurrentEquipID &][[;; , colOperatingDelayTime]]

returns a list consisting of a single number; otherwise, I cannot see how one would get predictable results picking the i-th element in the flattened array.

Here is some made-up data, on which your function works (i.e. runs without error and returns a list of real numbers):

nEquip = 10000;
numEquipmentStats = 100;
  colEquipID = 1; (* index *)
  colOperatingDelayTime = 3;  (* random index *)
CurrentEquipID = 40;
HTimeModelSelection[CurrentEquipID] = RandomInteger[{0, 2}, nEquip];
HOperatingEfficiency[CurrentEquipID] = RandomReal[1, nEquip];
HUtilizedTime[CurrentEquipID] = RandomReal[1, nEquip];
EquipParams = Transpose @ Join[{Range[nEquip]}, RandomReal[1, {numEquipmentStats, nEquip}]];

Your function takes 0.136795 sec. (Perhaps the slowness your function has to do with the functions HTimeModelSelection, HOperatingEfficiency, or HUtilizedTime -- your code calls them repeatedly on the same input -- something to avoid if your functions take an appreciable amount of time to evaluate.)

If the data in this calculation, except HTimeModelSelection, are positive numbers, then the following will be fast.

AbsoluteTiming[
 Unitize[HTimeModelSelection[CurrentEquipID]] (1. - 
       HOperatingEfficiency[CurrentEquipID]) HUtilizedTime[
      CurrentEquipID] /. 
    0 -> Select[EquipParams,
          #[[colEquipID]] == CurrentEquipID &, 1][[1, colOperatingDelayTime]];]
{0.002531, Null}

If the data is not all positive numbers, then here is a variation that works:

Transpose[{
     N@Unitize[HTimeModelSelection[CurrentEquipID]],
     (1. - HOperatingEfficiency[CurrentEquipID]) HUtilizedTime[CurrentEquipID]
    }] /. 
   {{0., _} -> Select[EquipParams,
                      #[[colEquipID]] == CurrentEquipID &, 1][[1, colOperatingDelayTime]],
    {1., x_} :> x};   // AbsoluteTiming
{0.006147, Null}

If that's not fast enough, then perhaps compiling will help:

cf = Compile[{{model, _Real, 1}, {eff, _Real, 1}, {time, _Real, 1}, {delay, _Real}},
   If[#[[1]] == 0., #[[2]], delay] & /@ Transpose[{model, (1. - eff) time}]];

cf[Unitize[HTimeModelSelection[CurrentEquipID]], 
   HOperatingEfficiency[CurrentEquipID],
   HUtilizedTime[CurrentEquipID],
   Select[EquipParams,
     #[[colEquipID]] == CurrentEquipID &, 1][[1, colOperatingDelayTime]]]; // AbsoluteTiming
{0.001194, Null}
share|improve this answer
    
You've got the order wrong, the ith element of ConstantArray[...] is picked then it is flattened. So, consistent results can be acheived. Oh, and +1 for pointing out that the OPs functions may be at fault, also. –  rcollyer Jun 11 '13 at 19:46
    
@rcollyer Aren't the [[;;, colOperatingDelayTime]] elements picked, ConstantArrayed, and flattened; and then the i-th element is picked? Seems like we're getting who-knows-which delay time, unless there's just one or they're all the same. –  Michael E2 Jun 11 '13 at 20:49
    
You were right, I was reading the closing ] of If as the closing ] of Flatten. But, a lot of duplication ... –  rcollyer Jun 11 '13 at 20:53
add comment

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