Skip to content

Remove conditional compilation blocks leftover from cuml-cpu#6572

Merged
rapids-bot[bot] merged 23 commits intorapidsai:branch-25.06from
jcrist:remove-gpubuild
Apr 24, 2025
Merged

Remove conditional compilation blocks leftover from cuml-cpu#6572
rapids-bot[bot] merged 23 commits intorapidsai:branch-25.06from
jcrist:remove-gpubuild

Conversation

@jcrist
Copy link
Copy Markdown
Member

@jcrist jcrist commented Apr 22, 2025

We're no longer releasing builds of cuml-cpu. This PR:

  • Removes the previous IF GPUBUILD blocks used to conditionally guard compilation for cuml-cpu builds. I didn't remove or simplify the usage of gpu_only_import/cpu_only_import, this PR is only tackling removal of the conditional compilation.
  • Removes setting GPUBUILD in the cmake file
  • Since the latter effectively breaks cuml-cpu builds, I went ahead and fully removed cuml-cpu from our cmake setup.

Fixes #6231. Part of #6523.

@jcrist jcrist requested review from a team as code owners April 22, 2025 21:09
@jcrist jcrist requested review from raydouglass, teju85 and vyasr April 22, 2025 21:09
@github-actions github-actions Bot added Cython / Python Cython or Python issue CMake labels Apr 22, 2025
@jcrist jcrist added the improvement Improvement / enhancement to an existing function label Apr 22, 2025
@jcrist jcrist added the non-breaking Non-breaking change label Apr 22, 2025
Copy link
Copy Markdown
Contributor

@gforsyth gforsyth left a comment

Choose a reason for hiding this comment

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

build.sh changes look good.

Copy link
Copy Markdown
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

Approving CMake/packaging changes.

The Python also looks fine to me (it's just removing a conditional and indentation).

@csadorf csadorf added the DO NOT MERGE Hold off on merging; see PR for details label Apr 22, 2025
@csadorf
Copy link
Copy Markdown
Contributor

csadorf commented Apr 22, 2025

Please hold off merge until I can complete my review.

Copy link
Copy Markdown
Contributor

@csadorf csadorf left a comment

Choose a reason for hiding this comment

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

Thanks a lot for tackling this!!

LGTM, just a few minor questions.

Comment thread python/cuml/CMakeLists.txt Outdated
Comment thread python/cuml/cuml/linear_model/ridge.pyx
Comment thread python/cuml/cuml/internals/logger.pxd
Comment thread python/cuml/cuml/internals/base.pyx
@csadorf csadorf removed the DO NOT MERGE Hold off on merging; see PR for details label Apr 22, 2025
@jcrist jcrist self-assigned this Apr 22, 2025
@jcrist
Copy link
Copy Markdown
Member Author

jcrist commented Apr 24, 2025

/merge

@rapids-bot rapids-bot Bot merged commit 99902c0 into rapidsai:branch-25.06 Apr 24, 2025
77 checks passed
@jcrist jcrist deleted the remove-gpubuild branch April 24, 2025 16:12
Ofek-Haim pushed a commit to Ofek-Haim/cuml that referenced this pull request May 13, 2025
…sai#6572)

We're no longer releasing builds of `cuml-cpu`. This PR:

- Removes the previous `IF GPUBUILD` blocks used to conditionally guard compilation for `cuml-cpu` builds. I didn't remove or simplify the usage of `gpu_only_import`/`cpu_only_import`, this PR is only tackling removal of the conditional compilation.
- Removes setting `GPUBUILD` in the cmake file
- Since the latter effectively breaks `cuml-cpu` builds, I went ahead and fully removed `cuml-cpu` from our cmake setup.

Fixes rapidsai#6231. Part of rapidsai#6523.

Authors:
  - Jim Crist-Harif (https://github.com/jcrist)

Approvers:
  - Gil Forsyth (https://github.com/gforsyth)
  - Bradley Dice (https://github.com/bdice)
  - Simon Adorf (https://github.com/csadorf)

URL: rapidsai#6572
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CMake Cython / Python Cython or Python issue improvement Improvement / enhancement to an existing function non-breaking Non-breaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cython deprecation warning: "The 'IF' statement is deprecated"

4 participants