Skip to content

Fix predict() ignoring base_margin in xgb.DMatrix#11880

Closed
Sanidhyavijay24 wants to merge 23 commits intodmlc:masterfrom
Sanidhyavijay24:fix-r-predict-margin
Closed

Fix predict() ignoring base_margin in xgb.DMatrix#11880
Sanidhyavijay24 wants to merge 23 commits intodmlc:masterfrom
Sanidhyavijay24:fix-r-predict-margin

Conversation

@Sanidhyavijay24
Copy link
Copy Markdown
Contributor

Fixes #11872

The Issue ->
Previously, predict.xgb.Booster prevented passing base_margin when the input was xgb.DMatrix. Additionally, if base_marginwas NULL (default), the function failed to check internalDMatrix` attributes, causing predictions to ignore the margin entirely.

The Fix ->

  • Removed the stop() block restriction.
  • Added logic to apply base_margin via setinfo if passed explicitly.
  • Added auto-detection to retrieve getinfo(newdata, "base_margin") if the argument is NULL.

Test ->
Added a regression test verifying that changing the margin inside a DMatrix alters the prediction, and that explicit arguments work.

Added a check for nrounds=0 to prevent an invalid sequence in the iteration loop.
Fix handling of nrounds=0 to ensure 'iteration' is defined. But left a trailing whitespace by mistake , now fixed it .
whitespace/style error in previous commit
Added test for xgb.train with nrounds set to 0 to ensure it results in 0 iterations.
Updated test for xgb.train with nrounds = 0 to handle potential NULL return for empty models.
Removed unnecessary blank line in test case.
Enhanced test for xgb.train with nrounds = 0, including checks for serialization, continuation, and callbacks.
Updated test cases to use global 'train' variable instead of 'agaricus.train'. Adjusted parameters to include 'nthread' for consistency in xgb.train calls.
Added support for base_margin when using xgb.DMatrix.
@Sanidhyavijay24
Copy link
Copy Markdown
Contributor Author

Note on Commit History ->
This branch includes commits from my previous PR because I branched off my local master. However, the Files Changed tab is clean and correctly shows only the relevant changes for this fix (xgb.Booster.R and test_basic.R).

I can rebase/clean up the history if preferred, but "Squash and Merge" should handle it automatically.
Sorry for the mess!.

Removed unnecessary checks for internal margin when base_margin is NULL.
Updated test case to use a fresh DMatrix for prediction override testing.
@trivialfis
Copy link
Copy Markdown
Member

Doing some tests here: #11885 Not sure how practical it is to reject all changes after DMatrix construction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Behaviour of base_margin during prediction

3 participants