[ML] Correct logistic loss function #1032
Conversation
| @@ -1220,7 +1220,7 @@ BOOST_AUTO_TEST_CASE(testLogisticRegression) { | |||
|
|
|||
| LOG_DEBUG(<< "mean log relative error = " | |||
| << maths::CBasicStatistics::mean(meanLogRelativeError)); | |||
| BOOST_TEST_REQUIRE(maths::CBasicStatistics::mean(meanLogRelativeError) < 0.5); | |||
| BOOST_TEST_REQUIRE(maths::CBasicStatistics::mean(meanLogRelativeError) < 0.52); | |||
There was a problem hiding this comment.
Do you have an intuition why the error goes up here?
There was a problem hiding this comment.
No! Interestingly though (looking at the debug) the hyperparameter choices become more stable with this change.
My best guess is there are multiple ways the code can "learn" to avoid the bug. For example, it'll potentially choose hyperparameter values which result in deeper trees so that it'll take the non-buggy path more often. Later trees added to the forest correct for the bug, since they're more likely to have different values for at least one of the rows so take the non-buggy path.
I've tested on some additional data sets and I haven't seen evidence of any accuracy degradation, but it will be interesting to review the results on our overnights.
There was a problem hiding this comment.
So of course there was a subtle undefined behaviour bug with capture by reference. In fact, the result is still slightly worse for one data set in this test, but slightly better for a different one, with the average error being back to almost exactly what it was before.
I do still think the comments above somewhat explain why the previous behaviour wasn't causing a more obvious degradation in accuracy.
…n computing node values for classification
|
retest |
1 similar comment
|
retest |
This was a bug in the computation of the best leaf weight when predictions for all its training rows are identical. In this case, we should find the weight to add to the current prediction which minimises the cross-entropy.