Skip to content

Conversation

@zhengruifeng
Copy link
Contributor

What changes were proposed in this pull request?

1, GeneralizedLinearRegressionSummary compute several statistics on single pass
2, LinearRegressionSummary use metrics.count

Why are the changes needed?

avoid extra passes on the dataset

Does this PR introduce any user-facing change?

No

How was this patch tested?

existing testsuites

@probot-autolabeler probot-autolabeler bot added the ML label Jul 3, 2020
@zhengruifeng
Copy link
Contributor Author

retest this please


private[regression] lazy val link: Link = familyLink.link


Copy link
Contributor

Choose a reason for hiding this comment

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

super nit: extra blank line?

lazy val numInstances: Long = predictions.count()
lazy val numInstances: Long = glrSummary.getLong(0)


Copy link
Contributor

Choose a reason for hiding this comment

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

remove this extra blank line too?

@huaxingao
Copy link
Contributor

The changes make sense to me.
cc @srowen

@SparkQA
Copy link

SparkQA commented Jul 5, 2020

Test build #124927 has finished for PR 28990 at commit 6b36835.

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

@dongjoon-hyun
Copy link
Member

Retest this please

@SparkQA
Copy link

SparkQA commented Jul 6, 2020

Test build #124983 has finished for PR 28990 at commit 6b36835.

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

@SparkQA
Copy link

SparkQA commented Jul 6, 2020

Test build #124997 has finished for PR 28990 at commit 2bdc956.

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

@huaxingao
Copy link
Contributor

I will merge this PR :)
I had problem with my jira account earlier and I think it's OK now. I will use this PR to test if everything works OK for me.
I will leave this for a day just in case other people have comments.

@huaxingao huaxingao closed this in 8d5c094 Jul 7, 2020
@huaxingao
Copy link
Contributor

Merged to master. Thanks everyone!

@zhengruifeng zhengruifeng deleted the glr_summary_opt branch July 8, 2020 01:22
@zhengruifeng
Copy link
Contributor Author

thanks all!

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants