Skip to content

Conversation

@jkbradley
Copy link
Member

Currently, the LogLoss used by GradientBoostedTrees has 2 issues:

  • the gradient (and therefore loss) does not match that used by Friedman (1999)
  • the error computation uses 0/1 accuracy, not log loss

This PR updates LogLoss.
It also adds some doc for boosting and forests.

I tested it on sample data and made sure the log loss is monotonically decreasing with each boosting iteration.

CC: @mengxr @manishamde @codedeft

@SparkQA
Copy link

SparkQA commented Nov 25, 2014

Test build #23811 has started for PR 3439 at commit 7c38962.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 25, 2014

Test build #23811 has finished for PR 3439 at commit 7c38962.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23811/
Test PASSed.

Copy link
Contributor

Choose a reason for hiding this comment

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

MSE is not usually defined with multiplier 1/2. Shall we use a different name here, or example, mean squared loss or average loss?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll remove the 1/2. It's probably better to have an odd loss (which only experts need to know about) than to have an odd name (which everyone needs to recognize).

…test suite since it effectively doubles the gradient and loss.

* Added doc for developers within RandomForest.
* Small cleanup in test suite (generating data only once)
@jkbradley
Copy link
Member Author

I just pushed an update which includes:

  • removing the 1/2 from SquaredError. This also required updating the test suite since it effectively doubles the gradient and loss.
  • Added doc for developers within RandomForest.
  • small cleanup in test suite (generating data only once)

@SparkQA
Copy link

SparkQA commented Nov 25, 2014

Test build #23849 has started for PR 3439 at commit 5e52bff.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 25, 2014

Test build #23849 has finished for PR 3439 at commit 5e52bff.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23849/
Test PASSed.

@manishamde
Copy link
Contributor

@jkbradley I am trying to find my reference for the LogLoss calculations.

Copy link
Contributor

Choose a reason for hiding this comment

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

There is an issue with numerical stability. Maybe we can fix it in this PR. The problem appears when w = -2.0 * point.label * prediction is large. math.exp(w) would overflow while math.log(1 + math.exp(w)) should be close to w. When w < 0, we can use

math.log1p(math.exp(w)).

Otherwise, we should use

w + math.log1p(math.exp(-w))

Copy link
Member Author

Choose a reason for hiding this comment

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

Will do!

@manishamde
Copy link
Contributor

@jkbradley LGTM. Thanks for the documentation too -- it is really helpful.

@SparkQA
Copy link

SparkQA commented Nov 26, 2014

Test build #23854 has started for PR 3439 at commit ed5da2c.

  • This patch merges cleanly.

@jkbradley
Copy link
Member Author

Updated LogLoss.
@mengxr @manishamde Thanks for looking at this!

@SparkQA
Copy link

SparkQA commented Nov 26, 2014

Test build #23856 has started for PR 3439 at commit a27eb6d.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 26, 2014

Test build #23856 has finished for PR 3439 at commit a27eb6d.

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

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23856/
Test FAILed.

@SparkQA
Copy link

SparkQA commented Nov 26, 2014

Test build #23862 has started for PR 3439 at commit cfec17e.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Nov 26, 2014

Test build #23854 has finished for PR 3439 at commit ed5da2c.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23854/
Test PASSed.

@SparkQA
Copy link

SparkQA commented Nov 26, 2014

Test build #23862 has finished for PR 3439 at commit cfec17e.

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

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/23862/
Test PASSed.

@mengxr
Copy link
Contributor

mengxr commented Nov 26, 2014

LGTM. Merged into master and branch-1.2. Thanks!

mengxr pushed a commit to mengxr/spark that referenced this pull request Nov 26, 2014
Currently, the LogLoss used by GradientBoostedTrees has 2 issues:
* the gradient (and therefore loss) does not match that used by Friedman (1999)
* the error computation uses 0/1 accuracy, not log loss

This PR updates LogLoss.
It also adds some doc for boosting and forests.

I tested it on sample data and made sure the log loss is monotonically decreasing with each boosting iteration.

CC: mengxr manishamde codedeft

Author: Joseph K. Bradley <[email protected]>

Closes apache#3439 from jkbradley/gbt-loss-fix and squashes the following commits:

cfec17e [Joseph K. Bradley] removed forgotten temp comments
a27eb6d [Joseph K. Bradley] corrections to last log loss commit
ed5da2c [Joseph K. Bradley] updated LogLoss (boosting) for numerical stability
5e52bff [Joseph K. Bradley] * Removed the 1/2 from SquaredError.  This also required updating the test suite since it effectively doubles the gradient and loss. * Added doc for developers within RandomForest. * Small cleanup in test suite (generating data only once)
e57897a [Joseph K. Bradley] Fixed LogLoss for GradientBoostedTrees, and updated doc for losses, forests, and boosting

(cherry picked from commit c251fd7)
Signed-off-by: Xiangrui Meng <[email protected]>
mengxr pushed a commit to mengxr/spark that referenced this pull request Nov 26, 2014
Currently, the LogLoss used by GradientBoostedTrees has 2 issues:
* the gradient (and therefore loss) does not match that used by Friedman (1999)
* the error computation uses 0/1 accuracy, not log loss

This PR updates LogLoss.
It also adds some doc for boosting and forests.

I tested it on sample data and made sure the log loss is monotonically decreasing with each boosting iteration.

CC: mengxr manishamde codedeft

Author: Joseph K. Bradley <[email protected]>

Closes apache#3439 from jkbradley/gbt-loss-fix and squashes the following commits:

cfec17e [Joseph K. Bradley] removed forgotten temp comments
a27eb6d [Joseph K. Bradley] corrections to last log loss commit
ed5da2c [Joseph K. Bradley] updated LogLoss (boosting) for numerical stability
5e52bff [Joseph K. Bradley] * Removed the 1/2 from SquaredError.  This also required updating the test suite since it effectively doubles the gradient and loss. * Added doc for developers within RandomForest. * Small cleanup in test suite (generating data only once)
e57897a [Joseph K. Bradley] Fixed LogLoss for GradientBoostedTrees, and updated doc for losses, forests, and boosting

(cherry picked from commit c251fd7)
Signed-off-by: Xiangrui Meng <[email protected]>
@asfgit asfgit closed this in c251fd7 Nov 26, 2014
asfgit pushed a commit that referenced this pull request Nov 26, 2014
…+ doc updates

We reverted #3439 in branch-1.2 due to missing `import o.a.s.SparkContext._`, which is no longer needed in master (#3262). This PR adds #3439 back to branch-1.2 with correct imports.

Github is out-of-sync now. The real changes are the last two commits.

Author: Joseph K. Bradley <[email protected]>
Author: Xiangrui Meng <[email protected]>

Closes #3474 from mengxr/SPARK-4583-1.2 and squashes the following commits:

aca2abb [Xiangrui Meng] add import o.a.s.SparkContext._ for v1.2
6b5564a [Joseph K. Bradley] [SPARK-4583] [mllib] LogLoss for GradientBoostedTrees fix + doc updates
@jkbradley jkbradley deleted the gbt-loss-fix branch December 4, 2014 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants