Skip to content

Conversation

@huaxingao
Copy link
Contributor

What changes were proposed in this pull request?

Add summary to RandomForestClassificationModel...

Why are the changes needed?

so user can get a summary of this classification model, and retrieve common metrics such as accuracy, weightedTruePositiveRate, roc (for binary), pr curves (for binary), etc.

Does this PR introduce any user-facing change?

Yes

RandomForestClassificationModel.summary
RandomForestClassificationModel.evaluate

How was this patch tested?

Add new tests

predictionColName,
$(labelCol),
weightColName,
Array(0.0))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

for non iterative algorithm, set objectiveHistory to Array(0.0).

@SparkQA
Copy link

SparkQA commented Jun 24, 2020

Test build #124434 has finished for PR 28913 at commit 1e3e3d5.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
  • sealed trait RandomForestClassificationSummary extends ClassificationSummary
  • class RandomForestClassificationSummary(_ClassificationSummary):
  • class RandomForestClassificationTrainingSummary(RandomForestClassificationSummary,
  • class BinaryRandomForestClassificationSummary(_BinaryClassificationSummary):
  • class BinaryRandomForestClassificationTrainingSummary(BinaryRandomForestClassificationSummary,

@SparkQA
Copy link

SparkQA commented Jun 28, 2020

Test build #124578 has finished for PR 28913 at commit e989a3e.

  • This patch fails Scala style tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 28, 2020

Test build #124580 has finished for PR 28913 at commit 55b52bd.

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

@zhengruifeng
Copy link
Contributor

It seems that you need to add summary in ml one by one.
Is it possible to add it as a generic function in Classifier/Regressor or even in Predictor?

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 good to me.

@huaxingao
Copy link
Contributor Author

@zhengruifeng
Thanks for your comments.
Seems I have to add summary to each of the Classifiers one by one. Using this PR as an example, I think I need to have the following in RandomForestclassifier.scala, to be consistent with the implementation of LogisticRegressionTrainingSummary.

sealed trait RandomForestClassificationSummary extends ClassificationSummary 

sealed trait RandomForestClassificationTrainingSummary extends RandomForestClassificationSummary
  with TrainingSummary

sealed trait BinaryRandomForestClassificationSummary extends BinaryClassificationSummary

sealed trait BinaryRandomForestClassificationTrainingSummary extends
  BinaryRandomForestClassificationSummary with RandomForestClassificationTrainingSummary

private class RandomForestClassificationTrainingSummaryImpl

private class RandomForestClassificationSummaryImpl

private class BinaryRandomForestClassificationTrainingSummaryImpl

private class BinaryRandomForestClassificationSummaryImpl

@srowen srowen closed this in f7d9e3d Jul 1, 2020
@srowen
Copy link
Member

srowen commented Jul 1, 2020

Merged to master

@huaxingao
Copy link
Contributor Author

Thanks! @srowen @zhengruifeng

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