Skip to content

Conversation

jensgoossens-tomtom
Copy link
Contributor

Small PR to remove the configurable TODO in XGBoostEstimator.
Instead of using the hardcoded value, simply access getInferBatchSize - which has the same default as the current hardcoded value, but is now configurable.

@trivialfis
Copy link
Member

cc @wbo4958

Copy link
Contributor

@wbo4958 wbo4958 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thx

@wbo4958
Copy link
Contributor

wbo4958 commented Oct 14, 2025

Wait, Could you be able to add a unit test for this?

@jensgoossens-tomtom
Copy link
Contributor Author

Wait, Could you be able to add a unit test for this?
Would you like me to test the default values and setting the inferBatchSize via params works?
Testing that it actually uses different batch size internally is a bit is hard to test in a unit test based on how the code is currently structured

Copy link
Member

@trivialfis trivialfis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the fix, will merge once the CI is green.

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.

3 participants