[SPARK-39616][BUILD][ML] Upgrade Breeze to 2.0 #37002

zhengruifeng · 2022-06-27T09:29:05Z

What changes were proposed in this pull request?

1, Upgrade Breeze to 2.0
2, import a implict breeze.stats.distributions.Rand.FixedSeed.randBasis in ML for this upgrade: there are two options: FixedSeed and VariableSeed, I took a quick look and choose FixedSeed for reproductivity

Why are the changes needed?

since 1.3, breeze has replaced com.github.fommil.netlib with dev.ludovic.netlib

after upgrade to 2.0:

1, breeze should be faster because of this replacement;
2, completely resolve the license issue related to com.github.fommil.netlib:all

Does this PR introduce any user-facing change?

No

How was this patch tested?

existing UT

dongjoon-hyun

Thank you for working on this, @zhengruifeng .

srowen · 2022-06-27T21:03:48Z

Looks good if it works; I kinda thought it would break more

zhengruifeng · 2022-06-28T08:31:43Z

there are some failures in pyspark, I think I need some more time.

srowen · 2022-07-01T13:22:28Z

Looks OK pending tests

try 2.0

srowen

Is this ready to merge?

srowen · 2022-07-04T16:54:08Z

python/run-tests.py

@@ -96,7 +96,8 @@ def run_individual_python_test(target_dir, test_name, pyspark_python):
    os.mkdir(metastore_dir)

    # Also override the JVM's temp directory by setting driver and executor options.
-    java_options = "-Djava.io.tmpdir={0} -Dio.netty.tryReflectionSetAccessible=true".format(tmp_dir)
+    java_options = "-Djava.io.tmpdir={0}".format(tmp_dir)
+    java_options = java_options + " -Dio.netty.tryReflectionSetAccessible=true -Xss16M"


The -Xss16m seems OK, just wondering if we think that breeze 2.0 is going to cause stack overflows more readily without this setting? could be an issue

yes, without this setting, pyspark and sparkr always failed.

I think it should be a new restriction in breeze, found such a commit in breeze 2.0
scalanlp/breeze@07c2d06

but I guess configurations in practice should normally safisty this stack size requirement.
for example, the scala UT run with breeze 2.0 successfully.

also cc @HyukjinKwon @WeichenXu123 since this PR change the java option of tests in pySpark and sparkR.

Are we using smaller memory in PySpark and SparkR?

Scala UT (Maven and SBT) seems to use small stack size, 4m, at large memory like mx4g.

-Xmx4g -Xss4m -XX:MaxMetaspaceSize=2g

Although this is already passed, do you think we can use more consistent settings across Scala/Python/R?

Yeah, let's have the same settings.

let me have a try with -Xss4m

dongjoon-hyun

I left a comment. However, since I believe we can tune JVM memory and stack size options more later, I'm +1 for AS-IS code too, @zhengruifeng .

zhengruifeng · 2022-07-05T04:34:40Z

@dongjoon-hyun @HyukjinKwon -Xss4M also works, the javaoptions in PySpark and SparkR are different from the scala side, but here I only set stack size.

dongjoon-hyun · 2022-07-05T19:38:07Z

Thank you, @zhengruifeng , @srowen , @HyukjinKwon . Merged to master for Apache Spark 3.4.

HyukjinKwon · 2022-07-06T00:22:16Z

@zhengruifeng seems like there's one test flaky presumably after this (https://github.com/apache/spark/runs/7203043049?check_suite_focus=true):


File "/__w/spark/spark/python/pyspark/mllib/linalg/distributed.py", line 859, in __main__.IndexedRowMatrix.computeSVD
Failed example:
    svd_model.V # doctest: +ELLIPSIS
Expected:
    DenseMatrix(3, 2, [-0.4082, -0.8165, -0.4082, 0.8944, -0.4472, 0.0], 0)
Got:
    DenseMatrix(3, 2, [-0.4082, -0.8165, -0.4082, 0.8944, -0.4472, -0.0], 0)
**********************************************************************
File "/__w/spark/spark/python/pyspark/mllib/linalg/distributed.py", line 426, in __main__.RowMatrix.computeSVD
Failed example:
    svd_model.V # doctest: +ELLIPSIS
Expected:
    DenseMatrix(3, 2, [-0.4082, -0.8165, -0.4082, 0.8944, -0.4472, 0.0], 0)
Got:
    DenseMatrix(3, 2, [-0.4082, -0.8165, -0.4082, 0.8944, -0.4472, -0.0], 0)
**********************************************************************
   1 of   6 in __main__.IndexedRowMatrix.computeSVD
   1 of   6 in __main__.RowMatrix.computeSVD
***Test Failed*** 2 failures.
Had test failures in pyspark.mllib.linalg.distributed with python3.9; see logs.

Mind taking a look please?

dongjoon-hyun · 2022-07-06T00:39:37Z

Thank you for reporting that, @HyukjinKwon .

zhengruifeng · 2022-07-06T00:47:25Z

@HyukjinKwon @dongjoon-hyun let me take a look, thanks

### What changes were proposed in this pull request? Skip flaky doctests ### Why are the changes needed? ``` File "/__w/spark/spark/python/pyspark/mllib/linalg/distributed.py", line 859, in __main__.IndexedRowMatrix.computeSVD Failed example: svd_model.V # doctest: +ELLIPSIS Expected: DenseMatrix(3, 2, [-0.4082, -0.8165, -0.4082, 0.8944, -0.4472, 0.0], 0) Got: DenseMatrix(3, 2, [-0.4082, -0.8165, -0.4082, 0.8944, -0.4472, -0.0], 0) ********************************************************************** File "/__w/spark/spark/python/pyspark/mllib/linalg/distributed.py", line 426, in __main__.RowMatrix.computeSVD Failed example: svd_model.V # doctest: +ELLIPSIS Expected: DenseMatrix(3, 2, [-0.4082, -0.8165, -0.4082, 0.8944, -0.4472, 0.0], 0) Got: DenseMatrix(3, 2, [-0.4082, -0.8165, -0.4082, 0.8944, -0.4472, -0.0], 0) ********************************************************************** 1 of 6 in __main__.IndexedRowMatrix.computeSVD 1 of 6 in __main__.RowMatrix.computeSVD ***Test Failed*** 2 failures. Had test failures in pyspark.mllib.linalg.distributed with python3.9; see logs. ``` #37002 occasionally cause above tests output `-0.0` instead of `0.0`, I think they are both acceptable. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? updated doctests Closes #37097 from zhengruifeng/build_breeze_followup. Authored-by: Ruifeng Zheng <ruifengz@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>

zhengruifeng · 2022-08-30T04:09:58Z

shall we also delete the license file LICENSE-netlib?
@srowen @dongjoon-hyun @HyukjinKwon

srowen · 2022-08-30T04:13:32Z

I believe you could. Check the license of the ludovic package to make sure we haven't missed something there, and that the fommil package isn't in the build now

### What changes were proposed in this pull request? Delete the license of fommil-netlib ### Why are the changes needed? after upgrading breeze to 2.0 in apache#37002, fommil-netlib no longer exists in spark build ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? existing testsuites Closes apache#37717 from zhengruifeng/del_netlib_license. Authored-by: Ruifeng Zheng <ruifengz@apache.org> Signed-off-by: Hyukjin Kwon <gurwls223@apache.org>

…bra acceleration ### What changes were proposed in this pull request? The pr aims to update doc for mllib linear algebra acceleration after #37002 & #37700 ### Why are the changes needed? > A.Dependence on netlib-java has been completely eliminated after #37002 & #37700. > B.Some interfaces of dev.ludovic.netlib.blas have changed. ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? N/A Closes #37791 from panbingkun/update_doc_for_mllib. Authored-by: panbingkun <pbk1982@gmail.com> Signed-off-by: Sean Owen <srowen@gmail.com>

### What changes were proposed in this pull request? The pr aims to remove dependency `shapeless` on breeze. ### Why are the changes needed? After pr #37002, `shapeless` has been deleted in [spark-deps-hadoop-2-hive-2.3](https://github.com/apache/spark/pull/37002/files#diff-670b971a2758f55d602f0d1ef63f7af5f8d9ca095b5a55664bc3275e274ca395). ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? Pass GA. Closes #40378 from panbingkun/remove_dep_on_breeze. Authored-by: panbingkun <pbk1982@gmail.com> Signed-off-by: Sean Owen <srowen@gmail.com>

github-actions bot added BUILD ML MLLIB labels Jun 27, 2022

dongjoon-hyun reviewed Jun 27, 2022

View reviewed changes

zhengruifeng force-pushed the build_update_breeze branch from 413e901 to c8b3c23 Compare June 28, 2022 09:08

github-actions bot added CORE PYTHON labels Jun 28, 2022

zhengruifeng force-pushed the build_update_breeze branch 2 times, most recently from 9bfe2d9 to c408e3d Compare July 1, 2022 12:40

zhengruifeng force-pushed the build_update_breeze branch from c408e3d to 445924c Compare July 2, 2022 05:39

zhengruifeng added 6 commits July 4, 2022 20:05

init

e7b44e3

try 2.0

fix test

826e031

modify dep files

f0634ae

py_test_negate_zero

d2b3282

doctest: +ELLIPSIS

261328e

try to set xss

bc4d63f

zhengruifeng force-pushed the build_update_breeze branch from 445924c to bc4d63f Compare July 4, 2022 12:58

zhengruifeng added 2 commits July 4, 2022 22:06

update deps file

01dd910

add xss in run-tests.sh

b347b16

github-actions bot added the R label Jul 4, 2022

srowen reviewed Jul 4, 2022

View reviewed changes

dongjoon-hyun approved these changes Jul 5, 2022

View reviewed changes

HyukjinKwon approved these changes Jul 5, 2022

View reviewed changes

reduce xss to 4M

b32d592

dongjoon-hyun closed this in 40e00b8 Jul 5, 2022

zhengruifeng deleted the build_update_breeze branch July 6, 2022 00:10

zhengruifeng mentioned this pull request Jul 6, 2022

[SPARK-39616][BUILD][ML][FOLLOWUP] Fix flaky doctests #37097

Closed

zhengruifeng mentioned this pull request Aug 30, 2022

[SPARK-39616][FOLLOWUP] Delete the license of fommil-netlib #37717

Closed

panbingkun mentioned this pull request Sep 4, 2022

[SPARK-40251][MLLIB][DOCS][FOLLOWUP] Update doc for mllib linear algebra acceleration #37791

Closed

panbingkun mentioned this pull request Mar 11, 2023

[SPARK-42758][BUILD][MLLIB] Remove dependency on breeze #40378

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-39616][BUILD][ML] Upgrade Breeze to 2.0 #37002

[SPARK-39616][BUILD][ML] Upgrade Breeze to 2.0 #37002

zhengruifeng commented Jun 27, 2022 •

edited

dongjoon-hyun left a comment

srowen commented Jun 27, 2022

zhengruifeng commented Jun 28, 2022

srowen commented Jul 1, 2022

srowen left a comment

srowen Jul 4, 2022

zhengruifeng Jul 5, 2022

zhengruifeng Jul 5, 2022

zhengruifeng Jul 5, 2022

dongjoon-hyun Jul 5, 2022 •

edited

HyukjinKwon Jul 5, 2022

zhengruifeng Jul 5, 2022

dongjoon-hyun left a comment

zhengruifeng commented Jul 5, 2022 •

edited

dongjoon-hyun commented Jul 5, 2022

HyukjinKwon commented Jul 6, 2022 •

edited

dongjoon-hyun commented Jul 6, 2022

zhengruifeng commented Jul 6, 2022

zhengruifeng commented Aug 30, 2022

srowen commented Aug 30, 2022

[SPARK-39616][BUILD][ML] Upgrade Breeze to 2.0 #37002

[SPARK-39616][BUILD][ML] Upgrade Breeze to 2.0 #37002

Conversation

zhengruifeng commented Jun 27, 2022 • edited

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

dongjoon-hyun left a comment

Choose a reason for hiding this comment

srowen commented Jun 27, 2022

zhengruifeng commented Jun 28, 2022

srowen commented Jul 1, 2022

srowen left a comment

Choose a reason for hiding this comment

srowen Jul 4, 2022

Choose a reason for hiding this comment

zhengruifeng Jul 5, 2022

Choose a reason for hiding this comment

zhengruifeng Jul 5, 2022

Choose a reason for hiding this comment

zhengruifeng Jul 5, 2022

Choose a reason for hiding this comment

dongjoon-hyun Jul 5, 2022 • edited

Choose a reason for hiding this comment

HyukjinKwon Jul 5, 2022

Choose a reason for hiding this comment

zhengruifeng Jul 5, 2022

Choose a reason for hiding this comment

dongjoon-hyun left a comment

Choose a reason for hiding this comment

zhengruifeng commented Jul 5, 2022 • edited

dongjoon-hyun commented Jul 5, 2022

HyukjinKwon commented Jul 6, 2022 • edited

dongjoon-hyun commented Jul 6, 2022

zhengruifeng commented Jul 6, 2022

zhengruifeng commented Aug 30, 2022

srowen commented Aug 30, 2022

zhengruifeng commented Jun 27, 2022 •

edited

dongjoon-hyun Jul 5, 2022 •

edited

zhengruifeng commented Jul 5, 2022 •

edited

HyukjinKwon commented Jul 6, 2022 •

edited