-
-
Notifications
You must be signed in to change notification settings - Fork 8.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[POC] Experimental support for l1 error. #7812
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks significantly better to me without trying to share the internal row representations inside the updaters.
src/common/partition_builder.h
Outdated
auto p_begin = row_set.Data()->data(); | ||
ParallelFor(row_set.Size(), ctx->Threads(), [&](size_t i) { | ||
auto const& node = row_set[i]; | ||
if (node.node_id < 0 || !tree[node.node_id].IsLeaf()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this actually occur?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On the CPU, all nodes are being kept track of including internal nodes. For node_id < 0, it should be an internal node and we can skip the IsLeaf
check, I will see if there's any edge case I missed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Turned the condition into a check.
src/tree/updater_gpu_hist.cu
Outdated
}); | ||
} else { | ||
// Avoid copying nodes by using evaluator to get leaf weight on-the-fly, this is | ||
// useful when tree leaf is not updated after tree construction. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think it's worth keeping this optimisation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't run the full benchmark yet, copying nodes is probably fine, but not sure about the memory allocation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
info_->num_col_, | ||
batch_param)); | ||
dh::safe_cuda(cudaSetDevice(ctx_->gpu_id)); | ||
info_->feature_types.SetDevice(ctx_->gpu_id); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lots of unrelated changes in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change for device_
-> ctx_->gpu_id
might not be strictly related, I changed it when I ran into a segfault during the test of gpu_id
being specified to 1. I can revert those if needed, but I think these changes are easy to skip.
This is an initial benchmark with gbm-bench. I haven't run the benchmark with the newly added l1 error, which can use some optimizations in quantile computation and I will run the benchmark after the related work is finished.
|
Hey @trivialfis, looks like great work! It seems to be implicit in the xgboost paper than the hessian is supposed to be nonnegative. |
At the moment XGBoost uses the sample weight as hessian (default to 1) for l1, and recomputes the leaf values based on input labels after growing the tree.
We haven't exposed the feature to custom objectives yet unless you are writing C++ extensions. |
Hi @trivialfis, This is a very interesting approach. How do you update the value after the tree is built ? The changelog mention line search, but I see no mention of that in the code. I guess that this is the method UpdateTreeLeaf of the objectives that does the job (in UpdateTreeLeafHost ?), but I don't understand what mathematical operation is done. Can you provide more details? Best regards, |
ok, I see. The line search is done thought the And as far as I can see, alpha is always set to 0.5. Is that right? |
For l1, the optimal is median, which is 0.5 quantile. |
ok, thanks for the answer. So no needs for line search in this case. |
Support adaptive tree, a feature supported by both sklearn and lightgbm. The tree leaf is recomputed based on residue of labels and predictions after construction.
For l1 error, the optimal value is the median (50 percentile).
This is marked as experimental support for the following reasons: