[SPARK-8823] [MLlib] [PySpark] Optimizations for SparseVector dot products

[MechCoder]

Follow up for #5946

Currently we iterate over indices and values in SparseVector and can be vectorized.

[AmplabJenkins]

Merged build triggered.

[AmplabJenkins]

Merged build started.

[MechCoder]

The speed up is not that impressive, but I roughly get a 10x speedup averaged over 100 iterations

Dot Products
Two Sparse Vectors length 50000 n_values 5000
In master:0.031453819274902345
In this branch: 0.0016013431549072267

Length 50000 n_values:500
In master:0.00331263542175293
In this branch: 0.0006479525566101074

Length: 500000 n_values:50000
In master: 0.04630022764205933
In this branch: 0.014638817310333252

squared_distance
Length: 500000 n_values:50000
In this branch:0.0178
In master:0.158

Length: 50000 n_values:500
In master: 0.0017
In this branch:0.0007526993751525879

Length: 50000, n_values: 5000
In master: 0.0158
In this branch: 0.001717

[SparkQA]

Test build #36529 has started for PR 7222 at commit 4030782.

[SparkQA]

Test build #36529 has finished for PR 7222 at commit 4030782.

• This patch fails PySpark unit tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

Merged build finished. Test FAILed.

[AmplabJenkins]

Merged build triggered.

[AmplabJenkins]

Merged build started.

[SparkQA]

Test build #36530 has started for PR 7222 at commit 68fd92f.

[SparkQA]

Test build #36530 has finished for PR 7222 at commit 68fd92f.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

Merged build finished. Test PASSed.

[MechCoder]

I verify that the benchmarks remain the same even after the bug fix.

[mengxr]

Could we simply do the following?

return np.dot(self.values, self.values) + np.dot(other.values, other.values) - 2.0 * self.dot(other)

It might generate some numeric errors when both vectors are long and very close to each other. It should be sufficient for normal use cases.

[mengxr]

@MechCoder Thanks for the benchmark! The changes to dot looks good to me except an extra option we should try. I would recommend replacing the square_distance implementation by a simpler version. If that takes longer to discuss, we can split this PR into two.

[MechCoder]

@mengxr Thanks for the comments. I think we can merge this basic improvement for now and we can try out your idea and then reiterate on a new branch. wdyt?

[mengxr]

We should split this PR, keep only the changes to dot, and test the assume_unique flag. On a separate PR, let's test the performance about square_distance. The currently implementation of square_distance is not trivial to understand. We should iterate more before merge.

[MechCoder]

OK, done. I forgot that assume_unique=True means that each array should have unique elements and not that the two arrays should be different from another.

[AmplabJenkins]

Merged build triggered.

[AmplabJenkins]

Merged build started.

[SparkQA]

Test build #36689 has started for PR 7222 at commit dcb51d3.

[SparkQA]

Test build #36689 has finished for PR 7222 at commit dcb51d3.

• This patch passes all tests.
• This patch merges cleanly.
• This patch adds no public classes.

[AmplabJenkins]

Merged build finished. Test PASSed.

[mengxr]

LGTM. Merged into master. Please create a new JIRA for squared_distance. Thanks!