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

Evaluate using scikit-build-core #645

Open
jakirkham opened this issue Nov 29, 2023 · 13 comments
Open

Evaluate using scikit-build-core #645

jakirkham opened this issue Nov 29, 2023 · 13 comments

Comments

@jakirkham
Copy link
Member

jakirkham commented Nov 29, 2023

Currently cuCIM contains

  • C++ library
  • Python/C++ bindings using pybind11
  • Pure Python code

When building wheels, the first two are built together as part of a CMake build. The result of which is bundled into a Python wheel with the last component, which is then shipped

The result is wheels are treated as pure, which caused some issues ( #641 ). Though those can be worked around

That said, it may be worth using scikit-build-core to streamline all of this and create a single wheel

One consideration is some users just use the Pure Python portion. Maybe we can add a flag to enable/disable the C++ build

Edit: To update to scikit-build-core as that is preferred

@jakirkham
Copy link
Member Author

More history on build changes made in issue: #305

@thewtex
Copy link

thewtex commented Feb 6, 2024

👍 !

A few notes: scikit-build's core has been refactored / rebuilt into scikit-build-core, which is a PEP517 build-backend that can be added to pyproject.toml.

There is outstanding pybind11 support.

Support for optional built components with pure python fallback is planned.

CC @henryiii @jcfr

@jakirkham jakirkham changed the title Evaluate using scikit-build Evaluate using scikit-build-core Feb 20, 2024
@thewtex
Copy link

thewtex commented Feb 25, 2024

Side note: the current pip install command is failing for me:

❯ pip install cucim-cu12
Collecting cucim-cu12
  Using cached cucim-cu12-24.2.0.tar.gz (6.9 kB)
  Preparing metadata (setup.py) ... error
  error: subprocess-exited-with-error
  
  × python setup.py egg_info did not run successfully.
  │ exit code: 1
  ╰─> [6 lines of output]
      Traceback (most recent call last):
        File "<string>", line 2, in <module>
        File "<pip-setuptools-caller>", line 34, in <module>
        File "/tmp/pip-install-65f7ajw3/cucim-cu12_b8221968d94c4da495894b499ea01754/setup.py", line 90, in <module>
          raise RuntimeError("Bad params")
      RuntimeError: Bad params
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: metadata-generation-failed

× Encountered error while generating package metadata.
╰─> See above for output.

note: This is an issue with the package mentioned above, not pip.
❯ pip --version
pip 24.0 from /home/matt/bin/micromamba/envs/itk-wasm/lib/python3.11/site-packages/pip (python 3.11)

@jakirkham
Copy link
Member Author

What Python version are you using?

@jcfr
Copy link

jcfr commented Feb 25, 2024

Based on the output of pip --version, that would Python 3.11

@jakirkham
Copy link
Member Author

Right only Python 3.9 & 3.10 are supported atm

Python 3.11 is in the works: rapidsai/build-planning#3

@thewtex
Copy link

thewtex commented Feb 26, 2024

Indeed 3.11. Great to hear that it is on the roadmap for support.

Another side note, dependency specification / resolution issue noting here, although I do not know if we can resolve it with current Python packaging standards and pip's resolver: Adding cucim as dependency brought in CUDA 11 packages. This conflicted with existing CUDA 12 packages. Perhaps, at a minimum the cucim package, if it brings in CUDA 11 has a dependency on cuda-version~=11, etc.

@jakirkham
Copy link
Member Author

This is for the conda packages or the wheels?

Conda packages use cuda-version to select the CUDA version

Wheels have a named suffix. So cucim-cu11 for CUDA 11 and cucim-cu12 for CUDA 12

@jakirkham
Copy link
Member Author

Also here is cuCIM's Python 3.11 PR: #704

@vyasr
Copy link
Contributor

vyasr commented Feb 27, 2024

Note that since this issue was first created the rest of RAPIDS that was using scikit-build has been migrated to scikit-build-core.

@thewtex
Copy link

thewtex commented Feb 27, 2024

This is for the conda packages or the wheels?

This is with the workflow:

  • Install cucim, cuda-version, kvikio via micromamba, conda-forge and rapidsai channels. I had to use python 3.10 and cuda-version 12 for a working kvikio.
  • pip install -e . for a package that depends on cucim.

That package had the cucim package in its pyproject.toml dependencies.

@jakirkham
Copy link
Member Author

Yeah the move to the suffixed naming convention is relatively new. Would imagine there are still a few projects that will need updates. Would recommend filing an issue on the project repo where the old dependency name is used and link them back to current recommendation in the README

@thewtex
Copy link

thewtex commented Mar 5, 2024

This is with the workflow:

Additional discussion and ideas on this here: #710

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

No branches or pull requests

4 participants