Upgrade to XGBoost 2.1.0 #167

hcho3 · 2024-06-29T00:41:22Z

Checklist

Used a personal fork of the feedstock to propose changes
Reset the build number to 0 (if the version changed)
Re-rendered with the latest conda-smithy (Use the phrase @conda-forge-admin, please rerender in a comment in this PR for automated rerendering)

Fixes #172

hcho3 · 2024-06-29T00:41:28Z

conda-forge-webservices · 2024-06-29T00:41:30Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipe:

It looks like the 'libxgboost' output doesn't have any tests.
It looks like the '_py-xgboost-mutex' output doesn't have any tests.
It looks like the 'py-xgboost-cpu' output doesn't have any tests.
It looks like the 'py-xgboost-gpu' output doesn't have any tests.
It looks like the 'xgboost' output doesn't have any tests.
It looks like the '_r-xgboost-mutex' output doesn't have any tests.
It looks like the 'r-xgboost-cpu' output doesn't have any tests.
It looks like the 'r-xgboost-gpu' output doesn't have any tests.

…nda-forge-pinning 2024.06.28.19.57.36

jakirkham · 2024-06-29T03:15:16Z

@conda-forge-admin , please restart CI

jakirkham · 2024-06-29T03:36:23Z

Did the source layout change somehow? CI is noting that it had trouble finding the xgboost directory

Not seeing this issue when building for the previous release

trivialfis · 2024-06-29T04:02:48Z

The rabit directory is removed.

This directory doesn't exist in XGBoost 2.1.0.

recipe/install-libxgboost.sh

jakirkham · 2024-06-29T06:54:37Z

Still seeing the same error on CI:

cp: cannot stat ‘/home/conda/feedstock_root/build_artifacts/xgboost-split_1719641630231/work/xgboost’: No such file or directory

Are there other layout changes that occurred?

recipe/meta.yaml

recipe/install-libxgboost.sh

Co-authored-by: jakirkham <[email protected]>

…nda-forge-pinning 2024.06.29.20.13.13

recipe/conda_build_config.yaml

trivialfis · 2024-06-30T07:12:18Z

Is It possible that we skip R build on this release? The rewrite is still on going.

jakirkham · 2024-07-25T02:03:07Z

recipe/bld.bat

+:: CCCL has different logic depending on the `__cplusplus` macro.
+:: However Visual Studio leaves `__cplusplus` as `199711L` by default.
+:: As a result code paths needing new C++ standard won't be run.
+:: This happens in spite of the fact that VS 2019 & 2022 default to C++14.
+:: To make sure `__cplusplus` matches, we pass an additional flag to VC.
+:: Ideally other tooling would do this for us, for now we do it ourselves.
+::
+:: https://learn.microsoft.com/en-us/cpp/build/reference/zc-cplusplus?view=msvc-160
+:: https://learn.microsoft.com/en-us/cpp/build/reference/zc-cplusplus?view=msvc-170
+:: https://github.com/NVIDIA/cccl/blob/82a3ed0282893d6316abde04394f59a9a37a3747/libcudacxx/include/nv/target#L27-L30
+:: https://github.com/NVIDIA/cutlass/issues/1403#issuecomment-2130383392
+:: https://gitlab.kitware.com/cmake/cmake/-/issues/18837
+set "CXXFLAGS=%CXXFLAGS% /Zc:__cplusplus"


Ok did a bit of a deep dive and have a theory of what is going on. Tried to explain this in the comment above

Basically some C++ detection logic in libraries (like CCCL) doesn't work correct with Visual Studio due to its default behavior. Have added a flag to Visual Studio to make it conform more to typical C++ compiler behavior (setting __cplusplus correctly)

Based on similar experiences from Visual Studio users with CUTLASS ( NVIDIA/cutlass#1403 (comment) ), who saw a similar error to us and fixed it with this flag, am hopeful this will help in our case too

Ideally other tooling would handle this for us, but it appears CMake hasn't done this yet and Visual Studio continues to require users to add this flag if they want a correct __cplusplus value. So for now adding to this build script. If this works, we may want to add this to XGBoost as well

Have raised an upstream CCCL issue: NVIDIA/cccl#2072

Though after looking more closely at the code there, they are probably ok with their current approach. This would be a potential simplification for them

jakirkham · 2024-07-25T02:04:17Z

Let's consider dropping 11.8 for Windows then?

If the latest change doesn't work, then we can turn this into an issue and skip it

recipe/meta.yaml

Co-authored-by: jakirkham <[email protected]>

jakirkham · 2024-07-25T05:43:52Z

@conda-forge-admin , please re-render

github-actions · 2024-07-25T05:45:44Z

Hi! This is the friendly automated conda-forge-webservice.

I tried to rerender for you, but it looks like there was nothing to do.

This message was generated by GitHub actions workflow run https://github.com/conda-forge/xgboost-feedstock/actions/runs/10088820119.

jakirkham · 2024-07-25T06:04:21Z

.ci_support/win_64_cuda_compilernvcccuda_compiler_version11.8python3.8.____cpython.yaml

-r_base:
- '4.1'
+python:
+- 3.8.* *_cpython
 target_platform:
 - win-64


This is the Windows CUDA 11.8 file. It should be removed by the skip, but it is not for some reason. Will need to take a closer look

Note part of the issue here is even if the build doesn't create any packages simply downloading the CTK can take over 20mins. So will want to make sure this job is properly removed so it isn't wasting CI cycles

Edit: Looks like the build is still running, which is also an issue. So the skip needs a fix

@jakirkham I added back noarch: python and re-rendered the recipe. It looks like our replacement for noarch: python somehow broke the selector logic. Once I added back noarch: python to meta.yaml, the build for Windows CUDA 11.8 is finally removed.

recipe/meta.yaml

conda-forge-webservices · 2024-07-25T23:37:24Z

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipe/meta.yaml) and found some lint.

Here's what I've got...

For recipe/meta.yaml:

noarch packages can't have skips with selectors. If the selectors are necessary, please remove noarch: python.

jakirkham · 2024-07-26T00:06:17Z

recipe/meta.yaml

+  noarch: python
  # Windows CUDA 11.8 fails due to a compilation error
  # xref: https://github.com/conda-forge/xgboost-feedstock/issues/173
  skip: True  # [win and cuda_compiler_version == "11.8"]
-  {% if python is defined %}
-  skip: {{ python.split(".")[:2] != min_python.split(".")[:2] }}
-  {% endif %}


Good intuition Hyunsu! 🙏

Think we can fix this up to handle both cases then and fix the linter

Though ok doing this as a follow up

jakirkham · 2024-07-26T00:08:49Z

Thanks Hyunsu

Looks good. Please feel free to add the automerge label when you are ready

The bot will then merge when CI completes. If that doesn't happen for some reason, it will leave a comment letting us know why

github-actions · 2024-07-26T01:34:34Z

Hi! This is the friendly conda-forge automerge bot!

I considered the following status checks when analyzing this PR:

linter: failed
azure: passed

Thus the PR was not passing and not merged.

jakirkham · 2024-07-26T03:48:29Z

Thanks Hyunsu! 🙏

trivialfis · 2024-07-27T19:10:10Z

Hi @jakirkham @hcho3 is it normal to have this under a CUDA 12.5 environment:

  + libxgboost                                   2.1.0  cuda120_h9dfd3e9_1                 conda-forge/linux-64       93MB
  + xgboost                                      2.1.0  cuda118_pyh98e67c5_1               conda-forge/noarch         17kB

I'm updating an environment that has XGBoost as a dependency. The above two lines are part of the mamba output, and it seems a bit weird to me to have mismatched CUDA versions.

Second question, since all C++ includes are distributed, maybe it makes sense to distribute the CMake config files as well?

jakirkham · 2024-07-31T17:36:11Z

@trivialfis this is a good question

AIUI in terms of CUDA usage, only libxgboost has CUDA bits. So xgboost doesn't have CUDA bits (as it is pure Python)

That said, it certainly is confusing to have xgboost mislabeled in this way. Also users that constrain xgboost based on CUDA version may also expect this to carry through to libxgboost

So agree we should fix this. Though think the risk is relatively low. Started doing this in PR: #188

jakirkham · 2024-08-01T02:32:42Z

Second question, since all C++ includes are distributed, maybe it makes sense to distribute the CMake config files as well?

Also sorry for not answering the CMake question earlier

Thank you for opening a new issue to discuss: #189

Hopefully the answers there are helpful

Upgrade to XGBoost 2.1.0

41956de

hcho3 requested review from aldanor, fhoehle, jakirkham, ksangeek, mfansler and xhochy as code owners June 29, 2024 00:41

MNT: Re-rendered with conda-build 24.5.1, conda-smithy 3.36.2, and co…

f01972f

…nda-forge-pinning 2024.06.28.19.57.36

conda-forge-webservices bot closed this Jun 29, 2024

conda-forge-webservices bot reopened this Jun 29, 2024

Drop rabit install

8cd97b9

This directory doesn't exist in XGBoost 2.1.0.

jakirkham reviewed Jun 29, 2024

View reviewed changes

recipe/install-libxgboost.sh Show resolved Hide resolved

List source directory contents before copying

eee9c0e

jakirkham reviewed Jun 29, 2024

View reviewed changes

recipe/meta.yaml Outdated Show resolved Hide resolved

jakirkham reviewed Jun 29, 2024

View reviewed changes

recipe/install-libxgboost.sh Outdated Show resolved Hide resolved

hcho3 and others added 7 commits June 29, 2024 00:52

Remove CLI

7a47d9b

Fix buildno

aa2382d

Co-authored-by: jakirkham <[email protected]>

Require R 4.3+

0cb0928

Require cross-r-base 4.3

ffa745d

Consistently require R 4.3

c875ff1

MNT: Re-rendered with conda-build 24.3.0, conda-smithy 3.36.2, and co…

800bdbd

…nda-forge-pinning 2024.06.29.20.13.13

Use evals instead of watchlist

f4e8edd

jakirkham reviewed Jun 30, 2024

View reviewed changes

recipe/conda_build_config.yaml Outdated Show resolved Hide resolved

Add Microsoft links to /Zc:__cplusplus comment

2ad8f99

jakirkham reviewed Jul 25, 2024

View reviewed changes

jakirkham added 2 commits July 24, 2024 19:08

Link Microsoft blog on /Zc:__cplusplus

2bc4e87

Pass /Zc:__cplusplus from nvcc to cl

1f90186

jakirkham mentioned this pull request Jul 25, 2024

Windows CUDA 11.8 failing #173

Open

jakirkham reviewed Jul 25, 2024

View reviewed changes

recipe/meta.yaml Show resolved Hide resolved

hcho3 and others added 2 commits July 24, 2024 22:42

Update recipe/meta.yaml

f7636eb

Co-authored-by: jakirkham <[email protected]>

Drop Windows __cplusplus workaround

72e01f3

jakirkham reviewed Jul 25, 2024

View reviewed changes

recipe/meta.yaml Outdated Show resolved Hide resolved

Try bringing back noarch: python

f0e8fc3

Tidy up patches

0349fc6

jakirkham reviewed Jul 26, 2024

View reviewed changes

jakirkham approved these changes Jul 26, 2024

View reviewed changes

hcho3 added the automerge Merge the PR when CI passes label Jul 26, 2024

jakirkham merged commit be9dd6a into conda-forge:main Jul 26, 2024
14 of 15 checks passed

This was referenced Jul 26, 2024

Fix linter error from 2.1.0 (due to noarch: python) #174

Merged

Merge conda-forge/main into rapidsai/main (pt. 13) rapidsai/xgboost-feedstock#65

Merged

jakirkham mentioned this pull request Jul 31, 2024

Align string_prefix across dependencies #188

Merged

5 tasks

hcho3 deleted the xgb_210 branch July 31, 2024 17:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade to XGBoost 2.1.0 #167

Upgrade to XGBoost 2.1.0 #167

hcho3 commented Jun 29, 2024 •

edited by jakirkham

Loading

hcho3 commented Jun 29, 2024

conda-forge-webservices bot commented Jun 29, 2024

jakirkham commented Jun 29, 2024

jakirkham commented Jun 29, 2024

trivialfis commented Jun 29, 2024

jakirkham commented Jun 29, 2024

trivialfis commented Jun 30, 2024

jakirkham Jul 25, 2024

jakirkham Jul 25, 2024

jakirkham commented Jul 25, 2024 •

edited

Loading

jakirkham commented Jul 25, 2024

github-actions bot commented Jul 25, 2024

jakirkham Jul 25, 2024 •

edited

Loading

hcho3 Jul 25, 2024

conda-forge-webservices bot commented Jul 25, 2024

jakirkham Jul 26, 2024

jakirkham commented Jul 26, 2024

github-actions bot commented Jul 26, 2024

jakirkham commented Jul 26, 2024

trivialfis commented Jul 27, 2024 •

edited

Loading

jakirkham commented Jul 31, 2024 •

edited

Loading

jakirkham commented Aug 1, 2024

Upgrade to XGBoost 2.1.0 #167

Upgrade to XGBoost 2.1.0 #167

Conversation

hcho3 commented Jun 29, 2024 • edited by jakirkham Loading

hcho3 commented Jun 29, 2024

conda-forge-webservices bot commented Jun 29, 2024

jakirkham commented Jun 29, 2024

jakirkham commented Jun 29, 2024

trivialfis commented Jun 29, 2024

jakirkham commented Jun 29, 2024

trivialfis commented Jun 30, 2024

jakirkham Jul 25, 2024

Choose a reason for hiding this comment

jakirkham Jul 25, 2024

Choose a reason for hiding this comment

jakirkham commented Jul 25, 2024 • edited Loading

jakirkham commented Jul 25, 2024

github-actions bot commented Jul 25, 2024

jakirkham Jul 25, 2024 • edited Loading

Choose a reason for hiding this comment

hcho3 Jul 25, 2024

Choose a reason for hiding this comment

conda-forge-webservices bot commented Jul 25, 2024

jakirkham Jul 26, 2024

Choose a reason for hiding this comment

jakirkham commented Jul 26, 2024

github-actions bot commented Jul 26, 2024

jakirkham commented Jul 26, 2024

trivialfis commented Jul 27, 2024 • edited Loading

jakirkham commented Jul 31, 2024 • edited Loading

jakirkham commented Aug 1, 2024

hcho3 commented Jun 29, 2024 •

edited by jakirkham

Loading

jakirkham commented Jul 25, 2024 •

edited

Loading

jakirkham Jul 25, 2024 •

edited

Loading

trivialfis commented Jul 27, 2024 •

edited

Loading

jakirkham commented Jul 31, 2024 •

edited

Loading