[SPARK-6364] [MLlib] Implement equals and hashcode for Matrix

[MechCoder]

Implementing methods equals hashcode and equals for Matrix

[MechCoder]

cc @mengxr . Please review.

[MechCoder]

@mengxr

1. Is there any reason why forEachActive is private? It took me some time to realize that it existed (and I already wrote my implementation by then)
2. The hashcode in this PR is just a generalization of that in the Vectors. I'm unable to understand why it is implemented in this specific way. Would be great if you could give me some idea.

[SparkQA]

Test build #28781 has finished for PR 5081 at commit 4041b59.

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

[SparkQA]

Test build #28783 has finished for PR 5081 at commit b5033b5.

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

[MechCoder]

Also, I'm not sure what the failed tests are about.

[SparkQA]

Test build #28785 has finished for PR 5081 at commit bf0e0e7.

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

[srowen]

What's the use case for comparing matrices for equality? I think it's rarer than for vectors. Is it just for testing, mostly?

[MechCoder]

Yes, I required it in this PR #5048 (which is required in #4986) to test that the matrices after serializing and deserializing are the same. (Also given the fact that it was already implemented for DenseMatrices, I just refactored it and added support for SparseMatrices)

[mengxr]

The hashCode implementation was taken from Effective Java: http://stackoverflow.com/questions/10915309/need-explanation-for-hashcode-example-in-effective-java-textbook

foreachActive is a private API because we need to consider what operators to expose together.

@srowen It may help if we implement hashCode and content equals for both vectors and matrices. It is mostly used for testing.

[mengxr]

Hash numRows and numCols separately.

[mengxr]

@srowen There are two approaches:

1. implement hashCode/equals
2. use the default implementation and use toArray to compare matrices in unit tests

Which do you prefer?

[MechCoder]

I'm starting to think we should just test if .toArray is equal and this PR might be overkill if it is just for testing purposes.

[mengxr]

Okay, let's keep this PR open and could you update the MatrixUDT PR to use .toArray?

[MechCoder]

Done already 👍

[SparkQA]

Test build #31050 has started for PR 5081 at commit bf0e0e7.

• This patch does not merge cleanly.

[andrewor14]

@mengxr @MechCoder what is the status on this PR? It seems to fairly outdated at this point. Should we move forward or close it?

[MechCoder]

Please do not. We just had a discussion about this yesterday.

[SparkQA]

Test build #35250 has finished for PR 5081 at commit e6b4cc5.

• This patch fails Scala style tests.
• This patch merges cleanly.
• This patch adds no public classes.

[MechCoder]

@mengxr Please have a look at #6904 This will help in ruling out some cases with minimum code repetition.

[SparkQA]

Test build #35299 has finished for PR 5081 at commit 9d29561.

• This patch fails Scala style tests.
• This patch merges cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #35303 has finished for PR 5081 at commit 7f46d17.

• This patch fails Scala style tests.
• This patch merges cleanly.
• This patch adds no public classes.

[MechCoder]

There seems to be a unrelated whitespace error in L30 in /spark/examples/src/main/scala/org/apache/spark/examples/DFSReadWriteTest.scala

[JoshRosen]

Yep, we're fixing it now via a hotfix.

[andrewor14]

should be fixed now. retest this please

[SparkQA]

Test build #35305 has finished for PR 5081 at commit 7f46d17.

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