Why is the new PositionIndex horribly slow?

47

20

This issue has largely been mitigated in 10.0.1. New timings for the final test below are:

Needs["GeneralUtilities`"]
a = RandomInteger[9, 5*^5];
myPosIdx[a]      // AccurateTiming
cleanPosIdx[a]   // AccurateTiming   (* see self-answer below *)
PositionIndex[a] // AccurateTiming
0.0149384

0.0149554

0.0545865

Still several times slower here than the readily available alternatives but no longer devastating.


Disconcertingly I have discovered that the new (v10) PositionIndex is horribly slow.

Using Szabolcs's clever GatherBy inversion we can implement our own function for comparison:

myPosIdx[x_] :=
  <|Thread[x[[ #[[All, 1]] ]] -> #]|> & @ GatherBy[Range @ Length @ x, x[[#]] &]

Check that its output matches:

RandomChoice[{"a", "b", "c"}, 50];

myPosIdx[%] === PositionIndex[%]
True

Check performance in version 10.0.0 under Windows:

a = RandomInteger[99999, 5*^5];
myPosIdx[a]      // Timing // First
PositionIndex[a] // Timing // First
0.140401

0.920406

Not a good start for the System` function, is it? It gets worse:

a = RandomInteger[999, 5*^5];
myPosIdx[a]      // Timing // First
PositionIndex[a] // Timing // First
0.031200

2.230814

With fewer unique elements PositionIndex actually gets slower! Does the trend continue?

a = RandomInteger[99, 5*^5];
myPosIdx[a]      // Timing // First
PositionIndex[a] // Timing // First
0.015600

15.958902

Somewhere someone should be doing a face-palm right about now. Just how bad does it get?

a = RandomInteger[9, 5*^5];
myPosIdx[a]      // Timing // First
PositionIndex[a] // Timing // First
0.015600

157.295808

Ouch. This has to be a new record for poor computational complexity in a System function. :o

Mr.Wizard

Posted 2014-07-15T00:14:51.540

Reputation: 259 163

Any suggestions for making this a proper Question? By the way, if others confirm this behavior should this be tagged as a bug? I certainly seems like one, but heretofore I believe that tag has been used for things that produce actual errors. – Mr.Wizard – 2014-07-15T00:19:25.277

I think the formulation, "Is there a more efficient way to implement...," may have been used in the past. Then your current question could become one of the answers. Don't know about the "bug" tag, if PositionIndex computes what is claimed. – Michael E2 – 2014-07-15T01:06:36.487

Presumably whoever implemented would like to know so as to fix this slowdown – acl – 2014-07-15T01:08:22.647

10I like to see this in a Wolfram Blog post and I already hear the first paragraph: "In the new Wolfram Language we implemented this new kind of thing or as we called it complexity speed-up... the harder your problems get, the faster Mathematica will solve them!" – halirutan – 2014-07-15T01:50:16.573

3

I didn't dare to post my SunPosition is horribly slow rant here and used Wolfram Community. :)

– Sjoerd C. de Vries – 2014-07-15T05:51:00.677

1When you first posted this, I wrote a variant of your code using GroupBy, it is a bit slower than yours, but it is nice and succinct: posIdx[x_] := GroupBy[Range@Length@x, (x[[#]] &) -> Identity]. – rcollyer – 2014-07-15T12:56:25.227

@rcollyer Very nice! I haven't gotten around to reading about that function yet. Disappointing that it's a bit slower however. – Mr.Wizard – 2014-07-15T13:05:14.243

It's roughly on par with yours for the cases where PositionIndex is performing poorly, but loses out when there are few collisions. That said, the grouping syntax is awesome, and once I get used to it, it will replace my use of SelectEquivalents as it does the fast majority of what I want to do.

– rcollyer – 2014-07-15T13:09:20.803

@rcollyer Isn't GroupBy[Range @ Length @ x, x[[#]] &] sufficient? – Mr.Wizard – 2014-07-15T13:10:14.167

yep. I'm still getting used to the syntax myself. My day to day use usually doesn't involve grouping operations. – rcollyer – 2014-07-15T13:12:08.880

@rcollyer Do you care to post the GroupBy method as an answer? – Mr.Wizard – 2014-07-15T13:14:19.737

Sure. It will be a bit as I need to get something done. – rcollyer – 2014-07-15T13:15:31.727

3Note that PositionIndex does work correctly with held expressions, whereas this is a bit painful to implement using GatherBy. a = 1; PositionIndex[Unevaluated[{a, b, c, d}]] – Jacob Akkerboom – 2014-07-15T14:51:24.220

@Jacob Interesting observation. Nevertheless that can't account for the performance problems. – Mr.Wizard – 2014-07-15T14:57:36.853

@Jacob p.s. also, interesting implied challenge; I came up with f[x_] := AssociationThread @@ {Hold[Unevaluated[x]][[1, {1}, #[[All, 1]]]], #} &@ GatherBy[Range@Length@x, Hold[x][[{1}, #]] &] -- what did you write? – Mr.Wizard – 2014-07-15T15:04:15.497

@Mr.Wizard ah, that looks good. I'm currently looking at the general case, i.e. making a general "Position Function". I am also using AssociationThread. I thought I had it and it worked for my first example, but I will have to debug it. – Jacob Akkerboom – 2014-07-15T15:29:54.197

@Mr.Wizard I have updated my answer to the position function question. At first sight, it looks pretty ok – Jacob Akkerboom – 2014-07-15T16:09:38.043

"Why is the new PositionIndex so horribly slow?" – geordie – 2014-07-27T12:19:50.633

@geordie Since I haven't come up with a better idea I'll use that. :-) – Mr.Wizard – 2014-07-27T12:21:32.140

3The performance has improved considerably in v10.0.1.0, but your myPosIdx is still slightly faster. – RunnyKine – 2014-09-17T08:35:47.763

@RunnyKine Thanks for the report. How about cleanPosIdx posted below? – Mr.Wizard – 2014-09-17T09:36:28.567

With a lot of repetitions it is just as fast as myPosIdx, but with fewer repititions, it is slightly slower than both myPosIdx and PositionIndex. – RunnyKine – 2014-09-17T09:47:18.150

@RunnyKine Interesting. By the way how did you download 10.0.1? My User Portal is still showing "10.0.0.0" as the "Current Version." – Mr.Wizard – 2014-09-17T09:50:21.957

I see the same thing, so I just downloaded the trial version (which happens to be 10.0.1.0) on a separate machine for now. – RunnyKine – 2014-09-17T09:54:29.897

Results for 10.0.2 similar to those of 10.0.1. – Sjoerd C. de Vries – 2015-01-04T19:37:30.400

Results for 10.1 are similar to 10.0.1 also – dr.blochwave – 2015-05-12T07:18:56.737

Answers

42

First let me note that I didn't write PositionIndex, so I can't speak to its internals without doing a bit of digging (which at the moment I do not have time to do).

I agree performance could be improved in the case where there are many collisions. Let's quantify how bad the situation is, especially since complexity was mentioned!

We'll use the benchmarking tool in GeneralUtilities to plot time as a function of the size of the list:

Needs["GeneralUtilities`"]
myPosIdx[x_] := <|Thread[x[[#[[All, 1]]]] -> #]|> &@
   GatherBy[Range@Length@x, x[[#]] &];
BenchmarkPlot[{PositionIndex, myPosIdx}, RandomInteger[100, #] &, 16, "IncludeFits" -> True]

which gives:

PositionIndex benchmark

While PositionIndex wins for small lists (< 100 elements), it is substantially slower for large lists. It does still appear to be $O(n \log n)$, at least.

Let's choose a much larger random integer (1000000), so that we don't have any collisions:

enter image description here

Things are much better here. We can see that collisions are the main culprit.

Now lets see how the speed for a fixed-size list depends on the number of unique elements:

BenchmarkPlot[{PositionIndex, myPosIdx}, RandomInteger[#, 10^4] &, 
   2^{3, 4, 5, 6, 7, 8, 9, 10, 11, 12}]

enter image description here

Indeed, we can see that PositionIndex (roughly) gets faster as there are more and more unique elements, whereas myPosIdx gets slower. That makes sense, because PositionIndex is probably appending elements to each value in the association, and the fewer collisions the fewer (slow) appends will happen. Whereas myPosIdx is being bottlenecked by the cost of creating each equivalence class (which PositionIndex would no doubt be too, if it were faster). But this is all academic: PositionIndex should be strictly faster than myPosIdx, it is written in C.

We will fix this.

Taliesin Beynon

Posted 2014-07-15T00:14:51.540

Reputation: 10 244

I look forward to seeing this improved - as bad as it is with the user-side GatherBy implementation, it's even worse off against this

– ciao – 2014-07-15T04:31:38.180

2Would love to learn more about GeneralUtilities`. – Szabolcs – 2014-07-15T04:34:06.763

1@Szabolcs ?GeneralUtilities*` ... Many functions have a brief synopsis or code. – Michael E2 – 2014-07-15T05:22:33.460

Thanks again for taking the time to answer one of my Questions, even one as discourteous as this. Thank you also for the analysis. I was aware of cases where PositionIndex appears more favorable but as you said the bottom line is: "PositionIndex should be strictly faster than myPosIdx (since) it is written in C." Also your point about complexity is taken, but if I am not mistaken computational complexity is multidimensional: it is not merely about e.g. list length but also other factors (e.g. number of unique elements), therefore I stand by my statement that it has poor complexity. – Mr.Wizard – 2014-07-15T06:08:40.243

1Anyway, thanks again for your attention and affirmation that this will be corrected. – Mr.Wizard – 2014-07-15T06:09:20.107

@JacobAkkerboom added myself. Thanks! – Taliesin Beynon – 2014-07-16T22:27:22.463

Thanks for getting this fixed. :-) – Mr.Wizard – 2014-09-17T11:24:03.360

8

rcollyer pointed out in a comment that the the new GroupBy may be substituted for GatherBy in Szabolcs's original to produce the desired function:

cleanPosIdx[x_] := GroupBy[Range @ Length @ x, x[[#]] &]

I shall be using this code until PositionIndex receives an enhancement.

Mr.Wizard

Posted 2014-07-15T00:14:51.540

Reputation: 259 163

5

Here's an alternative using the "GroupByList" resource function:

pIndex[i_List] := ResourceFunction["GroupByList"][Range @ Length @ i, i]

Comparison:

a = RandomInteger[10, 10^6];

r1 = pIndex[a]; //RepeatedTiming
r2 = myPosIdx[a]; //RepeatedTiming
r3 = PositionIndex[a]; //RepeatedTiming

r1 === r2 === r3

{0.02, Null}

{0.043, Null}

{0.11, Null}

True

Carl Woll

Posted 2014-07-15T00:14:51.540

Reputation: 112 778

I don't have ResourceFunction in v10.1. Why not provide an implementation using the GatherByList code you already posted?

– Mr.Wizard – 2019-12-09T17:36:29.407