Skip to content

Conversation

@eugen-natucci
Copy link

What changes were proposed in this pull request?

Use org.apache.spark.mllib.util.TestingUtils object across MLLIB component to compare floating point values in tests.

How was this patch tested?

build/mvn test - existing tests against updated code.

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks OK pending tests.

@SparkQA
Copy link

SparkQA commented Jul 18, 2019

Test build #4825 has finished for PR 25191 at commit d0c2ae9.

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

@eugen-natucci
Copy link
Author

addressed failed style checks reported by jenkins

@dongjoon-hyun
Copy link
Member

Retest this please.

@dongjoon-hyun dongjoon-hyun changed the title [MLLIB] Use TestingUtils to compare floating point values [SPARK-28440][MLLIB] Use TestingUtils to compare floating point values Jul 18, 2019
@SparkQA
Copy link

SparkQA commented Jul 18, 2019

Test build #107859 has finished for PR 25191 at commit a1cfc8c.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to fail in Jenkins environment. Did it pass in your local environment, @eugen-prokhorenko ?

sbt.ForkMain$ForkError: org.scalatest.exceptions.TestFailedException: Expected 0.6 and 1.0 to be within 1.0E-6 using absolute tolerance.
	at org.apache.spark.mllib.util.TestingUtils$DoubleWithAlmostEquals.$tilde$eq$eq(TestingUtils.scala:80)
	at org.apache.spark.mllib.fpm.AssociationRulesSuite.$anonfun$new$4(AssociationRulesSuite.scala:88)
	at org.apache.spark.mllib.fpm.AssociationRulesSuite.$anonfun$new$4$adapted(AssociationRulesSuite.scala:88)

Please make this PR pass the Jenkins.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know how I missed it. For usage in place of a predicate there is a less strict version of the operator (~= returns false without throwing the exception). The patch is underway.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the swift update!

@dongjoon-hyun
Copy link
Member

Retest this please.

@SparkQA
Copy link

SparkQA commented Jul 18, 2019

Test build #107860 has finished for PR 25191 at commit 49a7efb.

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

@dongjoon-hyun dongjoon-hyun changed the title [SPARK-28440][MLLIB] Use TestingUtils to compare floating point values [SPARK-28440][MLLIB][TEST] Use TestingUtils to compare floating point values Jul 19, 2019
Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM.
Thank you for your first contribution, @eugen-prokhorenko . Welcome to the Apache Spark community.

Merged to master.

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Jul 19, 2019

@eugen-prokhorenko . What is your Apache JIRA ID? I want to add you to the Apache Spark contributor group and assign SPARK-28440 to you.

@eugen-natucci
Copy link
Author

@dongjoon-hyun it's eugenzyx

@eugen-natucci
Copy link
Author

thank you

@dongjoon-hyun
Copy link
Member

dongjoon-hyun commented Jul 19, 2019

Thank YOU. It's assigned to you. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants