[SPARK-31309][ML] Migrate the ChiSquareTest from MLlib to ML

[zhengruifeng]

What changes were proposed in this pull request?

1, Move the impl of ChiSq from .mllib to the .ml side;
2, in .mllib.ChiSqTest, call the impl in .ml.ChiSquareTest

Why are the changes needed?

We should migrate the algs from MLlib to ML

Does this PR introduce any user-facing change?

No

How was this patch tested?

existing testsuites

[SparkQA]

Test build #120626 has finished for PR 28078 at commit 9e708d6.

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

[SparkQA]

Test build #120627 has finished for PR 28078 at commit ee2248f.

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

[dongjoon-hyun]

I have a question, @zhengruifeng . According to the new rubric, mllib will survive like HiveContext forever?

We should migrate the algs from MLlib to ML

[srowen]

I think .mllib will be around forever, yes. It's not that crazy; it still provides the RDD API for many of the same operations, and they share mostly one code base under the hood. And RDDs are not going away.

[dongjoon-hyun]

Thanks, @srowen .

[zhengruifeng]

@srowen @dongjoon-hyun OK, we will keep the .mllib, so I will close this PR. Thanks for pointing it out.

[srowen]

I don't think that means we can't refactor and improve the relationship between .ml and .mllib; I'm just saying I don't think we will deprecate or remove the latter. If this is just moving code, OK, maybe not worth it. If we can reduce duplication, that's OK.

[zhengruifeng]

OK, I did not get the point.

I think it worthwhile to move some impls to the .ml side, since in .ml we can use both Dataset and RDD in implementation. Also beacuse that we will still need to maintain those impls, moving them to .ml may help keeping consistence

[SparkQA]

Test build #120666 has finished for PR 28078 at commit 35c6db2.

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

[srowen]

It's not a bad change and I don't mind some cleanup personally; some are just formatting changes.
The general pattern is to let .ml reuse code in .mllib where they need to share, rather than both point to each other. I'm sure there are cases of both unfortunately (? I'd have to check). I might not move the code just to move it though unless it buys some simplification.

But yet the logic here doesn't depend on whether .mllib will be removed. That isn't an end goal as far as I can tell.

[zhengruifeng]

@srowen This PR is mainly moving the code without bringing more simplification. But it maybe the first step to futher improvements, a possible point may be returning results in rows instead a single row (in this PR the methods chiSquaredDenseFeatures and chiSquaredSparseFeatures are added to directly return rdd of test results, I thought migration is enough to open a PR, so I did not point it out).

For other impls, if I want to make an improvements based on DF or DS, should I just use DF or DS in the .mllib side (since .mllib side are almost impled on RDD) or move it to .ml at first?

I'm sure there are cases of both unfortunately (? I'd have to check).

Yes, at least GMM was implemented in both side.

[srowen]

Given the other discussion, I'm not sure we want to change the requests of the chi-squared test method. That doesn't mean there aren't other reasons to do this.

Yes the general idea is to call .mllib from .ml and not the other way around. If that's becoming hard or impossible, that could justify moving the logic.

If there isn't a strong reason I suppose I just wouldn't do it. I don't feel strongly.

[zhengruifeng]

OK, I will close this PR.