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

CMake and CPU instructions docs #11121

Closed
david-cortes opened this issue Dec 19, 2024 · 3 comments · Fixed by #11124
Closed

CMake and CPU instructions docs #11121

david-cortes opened this issue Dec 19, 2024 · 3 comments · Fixed by #11124

Comments

@david-cortes
Copy link
Contributor

The instructions for building the R package from source here:
https://xgboost.readthedocs.io/en/latest/build.html#building-r-package-from-source
.. mention:

But if you want to use CMake build for better performance (which has the logic for detecting available CPU instructions)

In the CMake file, I only see a deprecated "USE_AVX" option, which doesn't modify the resulting build:

message(SEND_ERROR "The option `USE_AVX` is deprecated as experimental AVX features have been removed from XGBoost.")

What is this CPU intruction detection feature? As far as I am aware, XGBoost doesn't have code paths for different instruction levels, or does it? Are these docs outdated?

@trivialfis
Copy link
Member

@david-cortes
Copy link
Contributor Author

Thanks. Couple questions:

  • Wouldn't a check of the form "test that it compiles" without "test that it runs" potentially cause it to generate artifacts that are not usable for some platforms?
  • For the R package in specific, why is this check done only in the cmake build and not in the regular CRAN build? Did it trigger errors at CRAN?

@trivialfis
Copy link
Member

  • That can happen, for instance, apple silicon x86->arm translation couldn't handle this. But other than that, we are using sse2, which should be quite safe.
  • Because no one is motivated enough to work with autotools yet ... I'm sure R has good reasons to continue its use, but so far I haven't met anyone who is keen on working with it. I myself really want to replace it when possible.

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 a pull request may close this issue.

2 participants