You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/05/26 07:21:20 UTC
[GitHub] [spark] sarutak opened a new pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
sarutak opened a new pull request #32673:
URL: https://github.com/apache/spark/pull/32673
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
This PR fixes an test failure of `DifferentiableLossAggregatorSuite` with Java 11.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
I'm personally checking whether all the tests pass with Java 11 for the current master and I found DifferentiableLossAggregatorSuite fails.
https://github.com/sarutak/spark/runs/2661859541?check_suite_focus=true#step:9:13895
The reason seems that the implementation of Blas.daxpy is different between for Java 8 and Java 11. For Java 11, Math.fma is used.
https://github.com/luhenry/netlib/blob/v2.2.0/blas/src/main/java/dev/ludovic/netlib/blas/Java8BLAS.java#L92
https://github.com/luhenry/netlib/blob/0053ea30b11686336cbdb8c7fceb41d59d268fa2/blas/src/main/java/dev/ludovic/netlib/blas/Java11BLAS.java#L40
To remove the rounding error, this PR changes `TestAggregator.add` with fma.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
No.
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
I confirmed `DifferentiableLossAggregatorSuite` passes with both Java 8 and Java 11.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-848537981
**[Test build #138970 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138970/testReport)** for PR 32673 at commit [`89cd2b9`](https://github.com/apache/spark/commit/89cd2b9da60da77da218152c69e5c1f665dac217).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on a change in pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #32673:
URL: https://github.com/apache/spark/pull/32673#discussion_r640338758
##########
File path: mllib/src/test/scala/org/apache/spark/ml/optim/aggregator/DifferentiableLossAggregatorSuite.scala
##########
@@ -149,10 +152,43 @@ object DifferentiableLossAggregatorSuite {
override def add(instance: Instance): TestAggregator = {
val error = instance.label - BLAS.dot(coefficients, instance.features)
- weightSum += instance.weight
- lossSum += instance.weight * error * error / 2.0
- (0 until dim).foreach { j =>
- gradientSumArray(j) += instance.weight * error * instance.features(j)
+ val mathCls = Utils.classForName("java.lang.Math")
Review comment:
> I think we can just fix the test with tolerance at the assertion e.g.) assert(agg.weight ~== weightSum relTol 1e-5) like the above one
I considered using `~==` at first but can we accept the precision diff in this case?
The failed assertion uses `===` while some of other assertions use `~==` so I guessed `===` is intentionally used.
If we can use `~==` here, I'll fix with `~==`.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32673:
URL: https://github.com/apache/spark/pull/32673#discussion_r640401944
##########
File path: mllib/src/test/scala/org/apache/spark/ml/optim/aggregator/DifferentiableLossAggregatorSuite.scala
##########
@@ -149,10 +152,43 @@ object DifferentiableLossAggregatorSuite {
override def add(instance: Instance): TestAggregator = {
val error = instance.label - BLAS.dot(coefficients, instance.features)
- weightSum += instance.weight
- lossSum += instance.weight * error * error / 2.0
- (0 until dim).foreach { j =>
- gradientSumArray(j) += instance.weight * error * instance.features(j)
+ val mathCls = Utils.classForName("java.lang.Math")
Review comment:
I think its okay .. I did the similar things in few places before .. but should be good to have their input.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a change in pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
zhengruifeng commented on a change in pull request #32673:
URL: https://github.com/apache/spark/pull/32673#discussion_r641077227
##########
File path: mllib/src/test/scala/org/apache/spark/ml/optim/aggregator/DifferentiableLossAggregatorSuite.scala
##########
@@ -149,10 +152,43 @@ object DifferentiableLossAggregatorSuite {
override def add(instance: Instance): TestAggregator = {
val error = instance.label - BLAS.dot(coefficients, instance.features)
- weightSum += instance.weight
- lossSum += instance.weight * error * error / 2.0
- (0 until dim).foreach { j =>
- gradientSumArray(j) += instance.weight * error * instance.features(j)
+ val mathCls = Utils.classForName("java.lang.Math")
Review comment:
> An approximate equality test is appropriate here. I'm not sure why it varies in Java 11, but I could imagine tiny differences are possible. This is OK.
+1
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-849530307
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139017/
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-849529941
**[Test build #139017 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139017/testReport)** for PR 32673 at commit [`26d9cc8`](https://github.com/apache/spark/commit/26d9cc8109a5e8afc5aec42d5712e479a72e61f4).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-849537079
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43534/
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-849530307
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/139017/
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-848573508
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43490/
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-849558217
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43534/
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-848582028
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138970/
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-848581512
**[Test build #138970 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138970/testReport)** for PR 32673 at commit [`89cd2b9`](https://github.com/apache/spark/commit/89cd2b9da60da77da218152c69e5c1f665dac217).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-849558217
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43534/
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-849489013
**[Test build #139017 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139017/testReport)** for PR 32673 at commit [`26d9cc8`](https://github.com/apache/spark/commit/26d9cc8109a5e8afc5aec42d5712e479a72e61f4).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-849516748
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43534/
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-848618539
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43490/
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-848618539
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/43490/
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-848582028
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/138970/
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on a change in pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #32673:
URL: https://github.com/apache/spark/pull/32673#discussion_r640329780
##########
File path: mllib/src/test/scala/org/apache/spark/ml/optim/aggregator/DifferentiableLossAggregatorSuite.scala
##########
@@ -149,10 +152,43 @@ object DifferentiableLossAggregatorSuite {
override def add(instance: Instance): TestAggregator = {
val error = instance.label - BLAS.dot(coefficients, instance.features)
- weightSum += instance.weight
- lossSum += instance.weight * error * error / 2.0
- (0 until dim).foreach { j =>
- gradientSumArray(j) += instance.weight * error * instance.features(j)
+ val mathCls = Utils.classForName("java.lang.Math")
Review comment:
@sarutak, I think we can just fix the test with tolerance e.g.) `assert(agg.weight ~== weightSum relTol 1e-5)` like the above one or https://github.com/apache/spark/blob/master/mllib/src/test/scala/org/apache/spark/ml/tuning/RandomRangesSuite.scala#L37
Up to my knowledge, this kind of precision diff is a common problem in ML side. cc @srowen, @WeichenXu123 @zhengruifeng FYI
##########
File path: mllib/src/test/scala/org/apache/spark/ml/optim/aggregator/DifferentiableLossAggregatorSuite.scala
##########
@@ -149,10 +152,43 @@ object DifferentiableLossAggregatorSuite {
override def add(instance: Instance): TestAggregator = {
val error = instance.label - BLAS.dot(coefficients, instance.features)
- weightSum += instance.weight
- lossSum += instance.weight * error * error / 2.0
- (0 until dim).foreach { j =>
- gradientSumArray(j) += instance.weight * error * instance.features(j)
+ val mathCls = Utils.classForName("java.lang.Math")
Review comment:
@sarutak, I think we can just fix the test with tolerance at the assertion e.g.) `assert(agg.weight ~== weightSum relTol 1e-5)` like the above one or https://github.com/apache/spark/blob/master/mllib/src/test/scala/org/apache/spark/ml/tuning/RandomRangesSuite.scala#L37
Up to my knowledge, this kind of precision diff is a common problem in ML side. cc @srowen, @WeichenXu123 @zhengruifeng FYI
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon commented on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-850068886
Merged to master.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] HyukjinKwon closed pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
HyukjinKwon closed pull request #32673:
URL: https://github.com/apache/spark/pull/32673
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-848537981
**[Test build #138970 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/138970/testReport)** for PR 32673 at commit [`89cd2b9`](https://github.com/apache/spark/commit/89cd2b9da60da77da218152c69e5c1f665dac217).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] sarutak commented on a change in pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #32673:
URL: https://github.com/apache/spark/pull/32673#discussion_r640426186
##########
File path: mllib/src/test/scala/org/apache/spark/ml/optim/aggregator/DifferentiableLossAggregatorSuite.scala
##########
@@ -149,10 +152,43 @@ object DifferentiableLossAggregatorSuite {
override def add(instance: Instance): TestAggregator = {
val error = instance.label - BLAS.dot(coefficients, instance.features)
- weightSum += instance.weight
- lossSum += instance.weight * error * error / 2.0
- (0 until dim).foreach { j =>
- gradientSumArray(j) += instance.weight * error * instance.features(j)
+ val mathCls = Utils.classForName("java.lang.Math")
Review comment:
O.K. Changed to use `~==`.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] srowen commented on a change in pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #32673:
URL: https://github.com/apache/spark/pull/32673#discussion_r640624191
##########
File path: mllib/src/test/scala/org/apache/spark/ml/optim/aggregator/DifferentiableLossAggregatorSuite.scala
##########
@@ -149,10 +152,43 @@ object DifferentiableLossAggregatorSuite {
override def add(instance: Instance): TestAggregator = {
val error = instance.label - BLAS.dot(coefficients, instance.features)
- weightSum += instance.weight
- lossSum += instance.weight * error * error / 2.0
- (0 until dim).foreach { j =>
- gradientSumArray(j) += instance.weight * error * instance.features(j)
+ val mathCls = Utils.classForName("java.lang.Math")
Review comment:
An approximate equality test is appropriate here. I'm not sure why it varies in Java 11, but I could imagine tiny differences are possible. This is OK.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-848598821
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43490/
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #32673: [SPARK-35530][ML][TESTS] Fix rounding error in DifferentiableLossAggregatorSuite with Java 11
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #32673:
URL: https://github.com/apache/spark/pull/32673#issuecomment-849489013
**[Test build #139017 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/139017/testReport)** for PR 32673 at commit [`26d9cc8`](https://github.com/apache/spark/commit/26d9cc8109a5e8afc5aec42d5712e479a72e61f4).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org