[R] Check invalid input for the cv function. #11264
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I didn't notice there was such a parameter in Python. Better to rename it straight away - there hasn't been any release with that function argument yet so no harm in modifying it. |
R-package/R/callbacks.R
Outdated
| @@ -647,6 +649,9 @@ xgb.cb.early.stop <- function( | |||
| if (inherits(model, "xgb.Booster") && !length(evals)) { | |||
| stop("For early stopping, 'evals' must have at least one element") | |||
| } | |||
| if (!inherits(model, "xgb.Booster") && keep_all_iter) { | |||
| stop("`keep_all_iter` must be set to FALSE for cv.") | |||
There was a problem hiding this comment.
Suggested change
| stop("`keep_all_iter` must be set to FALSE for cv.") | |
| stop("'keep_all_iter' must be set to FALSE when using early stopping in 'xgb.cv'.") |
R-package/R/xgb.cv.R
Outdated
| if (inherits(data, "xgb.DMatrix") && .Call(XGCheckNullPtr_R, data)) { | ||
| stop("'data' is an invalid 'xgb.DMatrix' object. Must be constructed again.") | ||
| } | ||
| if (inherits(data, "xgb.QuantileDMatrix")) { | ||
| stop("`xgb.QuantileDMatrix` is not yet supported for the cv function.") |
There was a problem hiding this comment.
Suggested change
| stop("`xgb.QuantileDMatrix` is not yet supported for the cv function.") | |
| stop("'xgb.QuantileDMatrix' is not supported as input to 'xgb.cv'.") |
|
Thank you for the quick review. I have renamed the parameter, and corrected the quotes. |
python-package/xgboost/callback.py
Outdated
| Whether training should return the best model or the last model. If set to | ||
| `True`, it will only keep the boosting rounds up to the detected best iteration, | ||
| discarding the ones that come after. This is only supported with tree | ||
| methods. Also, the `cv` function doesn't return a model, the parameter is not |
There was a problem hiding this comment.
This one could also mention the 'gblinear' part.
david-cortes
approved these changes
Feb 19, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Followup: #11255
Note: the
save_bestin Python is implemented in R with thekeep_all_iterparameter. Not sure if this is intended.