Avoid segment fault in ResetConfig for GBDT in prediction (fix #3317) #3373
Conversation
| @@ -718,7 +719,7 @@ void GBDT::ResetConfig(const Config* config) { | |||
| if (train_data_ != nullptr) { | |||
| ResetBaggingConfig(new_config.get(), false); | |||
| } | |||
| if (config_->forcedsplits_filename != new_config->forcedbins_filename) { | |||
| if (config_.get() != nullptr && config_->forcedsplits_filename != new_config->forcedbins_filename) { | |||
There was a problem hiding this comment.
@guolinke I think there might be a typo here, should it be new_config->forcedsplits_filename (instead of new_config->forcedbins_filename)?
There was a problem hiding this comment.
@btrotta you are right, it is typo.
There was a problem hiding this comment.
@btrotta can you help to create a PR?
There was a problem hiding this comment.
@shiyu1994 Apologies, I just now realized you are the author of this PR, not guolinke. I should have mentioned you in my original comment.
|
Hi @shiyu1994, I think it would have been better for LGBM_BoosterResetParameter() to just create the config if it doesn't exist. That way folks can set parameters once instead of for each predict (like we could before). Please reconsider this fix, thanks. |
|
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. |
This PR is to fix #3317. When a booster is created from model file, its config_ member will be undefined. However, the ResetConfig method of GBDT is not aware of this. As @glarb pointed out.