pan3793 commented on PR #49986: URL: https://github.com/apache/spark/pull/49986#issuecomment-2666214616
@srowen I addressed your previous comments(replied inline in each thread) and found some other issues during work on this PR: 1) `spark.executor.extraJavaOptions` forbids shipping value contains `-Dspark`, this also affects SPARK-47383 (https://github.com/apache/spark/pull/45504#issuecomment-2665957194). To fix that, we can either give an exception to `spark.ml.allowNativeBlas` when checking `spark.executor.extraJavaOptions` or choose a different prefix, e.g. `netlib.allowNativeBlas` 2) In addition to `mllib-local/src/main/scala/org/apache/spark/ml/linalg/BLAS.scala`, I found other three places that need to modify - `mllib/src/main/scala/org/apache/spark/mllib/linalg/BLAS.scala` - `mllib/src/main/scala/org/apache/spark/mllib/linalg/ARPACK.scala` - `mllib/src/main/scala/org/apache/spark/mllib/linalg/LAPACK.scala` I'm not sure if we want to have 3 configurations(e.g. `spark.ml.allowNativeBlas`, `spark.ml.allowNativeArpack`, `spark.ml.allowNativeLapack`) or just a single one `spark.ml.allowNativeBlas`(or a better name?) 3) the official Spark docker image does not install the BLAS native libraries, should we install them? would be great if you could have another look, thank you in advance. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
