create buffer for gradients and hessians with goss and customized objective (fixes #3243)

[shiyu1994]

This is to fix Issue #3243.
The gradients_ and hessians_ of GBDT object is of size 0 if customized objective function is used.

LightGBM/src/boosting/gbdt.cpp

Lines 95 to 99 in 1cf13db

	if (objective_function_ != nullptr) {
	size_t total_size = static_cast<size_t>(num_data_) * num_tree_per_iteration_;
	gradients_.resize(total_size);
	hessians_.resize(total_size);
	}

However, they are used by the bagging function of GOSS, for example

LightGBM/src/boosting/goss.hpp

Lines 103 to 106 in 1cf13db

	for (int cur_tree_id = 0; cur_tree_id < num_tree_per_iteration_; ++cur_tree_id) {
	size_t idx = static_cast<size_t>(cur_tree_id) * num_data_ + cur_idx;
	grad += std::fabs(gradients_[idx] * hessians_[idx]);
	}

which causes the segment fault.

[guolinke]

Hi @shiyu1994 , can we only fix this in goss.hpp. It seems the normal GBDT is not affected.

[shiyu1994]

The code of normal GBDT is only modified to include two more parameters gradients and hessians for Bagging method and this won't affect its execution. These two parameters are only effective when GOSS is used. With GOSS we are supporting a bagging according to gradients and hessians. So I think including them in the interface of Bagging method should be meaningful, which indicates that our bagging process can use gradient information.

The original Bagging and BaggingHelper do not provide parameters for gradients and hessians. With customized objective, these two arrays will be passed through the arguments of TrainOneIter from the outside of GBDT object. In that case, I think expanding the parameters to include 'gradients' and 'hessians' should be necessary. Otherwise GOSS code will have no access to the arrays passed from outside the GBDT object to TrainOneIter (inherited by GOSS without modification) which further calls the Bagging method.

Is there a better way to do this or any other suggestion?

[guolinke]

I was thinking to override the TrainOneIter in GOSS.
in the GOSS:TrainOneIter, we can firstly copy the external grad/hess to grad_/hess_, then call GBDT::TrainOneIter(grad_, hess_).

[shiyu1994]

Ok. That seems more elegant. I'll modify this.

[StrikerRUS]

Compilation of current master fails for Windows with the following error:

d:\a\1\s\src\boosting\goss.hpp(62): error C3016: 'i': index variable in OpenMP 'for' statement must have signed integral type (compiling source file D:\a\1\s\src\boosting\boosting.cpp) [D:\a\1\s\build\lightgbm.vcxproj]

I believe it was introduced here.

BTW, it is quite strange that CI allowed to merge this PR.

[guolinke]

@StrikerRUS sorry, it is my bad!
I fix this directly in 9b26373

[StrikerRUS]

Ah, OK! Thanks a lot for the prompt fix!

[github-actions]

This pull request has been automatically locked since there has not been any recent activity since it was closed. To start a new related discussion, open a new issue at https://github.com/microsoft/LightGBM/issues including a reference to this.