-
Notifications
You must be signed in to change notification settings - Fork 215
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
feat: Add estimator type with reg and clf for NGBClassifier and NGBRegressor #325
base: master
Are you sure you want to change the base?
Conversation
hey @NeroHin, thanks for the suggestion. Does it make sense to use ngboost in an ensemble for classification or regression alongside eg xgboost? The two are fundamentally different in the case of regresion: ngboost is for probabilistic prediction- it outputs an entire predictive distribution. There's not much point in using ngboost for point prediction if that's all you need since it's only every about as good as xgboost but much slower (see paper for performance comparison). In the case of standard classification (without some parametric discrete distribution eg poisson) ngboost is basically the same as xgboost except, again, slower. So while there is nothing fundamentally "wrong" with this change, I feel like it is enabling a use-case that doesn't really make much sense or should not be encouraged. Does that make sense or am I missing something? |
@alejandroschuler Thanks for your reply. I've seen some new research to use ngboost has a better performance than xgboost (case1 case2). In my case, I have to use the ensemble method by But when I add the NGBoost classifier into |
I don't find those papers terribly convincing and in our own paper we found that ngboost is generally only as good as xgboost or a little worse. That makes sense: ngboost isn't trying to directly compete with xgboost. I still think it's probably a waste of time / energy to try to use ngboost in an ensemble if your goal is point prediction or classification, but I'm going to approve the PR here b/c it doesn't hurt to have the functionality and hopefully people will see this discussion and be dissuaded from using it :) |
Thanks for your comment ! |
Fixed #324
Problem: Currently, when using NGBClassifier or NGBRegressor with the sklearn ensemble voting classifier or regressor, a ValueError is returned with the message "The estimator NGBClassifier / NGBRegressor should be a classifier / regressor."
Solution: Added two attributes, self._estimator_type = "classifier" and self._estimator_type = "regressor", into the NGBClassifier and NGBRegressor classes respectively. These attributes indicate whether the model is a classifier or a regressor, allowing the models to be correctly identified by the sklearn ensemble voting classifier or regressor.
Impact: This change should resolve the ValueError issue when using NGBClassifier or NGBRegressor with the sklearn ensemble voting classifier or regressor, improving the usability of the models in an ensemble learning setting.