Skip to content
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

[R] Use same RNG algorithm as in other interfaces #10029

Merged
merged 1 commit into from
Feb 4, 2024

Conversation

david-cortes
Copy link
Contributor

ref #9810

This PR removes the custom RNG option for the R package that uses R's own RNG in place of XGBoost's default RNG.

In order to still mimic the old behavior, it will draw a random seed through R's RNG system when the seed parameter is not manually specified, but otherwise will take the random seed (whether manually passed or automatically drawn) with the default random engine.

With this change, running the same call to xgb.train with the same data and parameters should produce the same results across interfaces.


By the way, I see that XGBoost is using a Mersenne-Twister generator. This family of RNG algorithms has been found to not hold all the statistical properties that one expects from an RNG algorithm, and many large projects (e.g. NumPy, DuckDB, Highway, among many others) have consequently moved towards newer RNGs like Xoshiro family and PCG family.

It might not matter much given the limited amount of random numbers that are drawn in XGBoost, but could be a potential improvement to look after.

@trivialfis
Copy link
Member

@hcho3 is the CI working correctly? I can't see the dashboard for some reason.

@hcho3
Copy link
Collaborator

hcho3 commented Feb 3, 2024

The Lambda function is broken for some reason

@hcho3
Copy link
Collaborator

hcho3 commented Feb 3, 2024

CI is now back to health. Fix: hcho3/xgboost-devops@9e789e5

@trivialfis
Copy link
Member

Excellent! Thank you for the fix!

@trivialfis trivialfis merged commit a730c7e into dmlc:master Feb 4, 2024
28 checks passed
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