CIL icon indicating copy to clipboard operation
CIL copied to clipboard

pip build

Open casperdcl opened this issue 8 months ago • 13 comments

  • pip: use scikit-build-core to drive cmake
    • part of #932, #2097, #1875
    • cherry-picked some commits from #2105 /CC @purepani
  • update cilacc lib path
  • remove Wrappers/Python/CMakeLists.txt
  • [x] update conda recipe
  • [x] update docker build
  • [x] update CI workflow
  • [x] update docs
  • [ ] update CHANGELOG
  • [x] depends on #2148

testing

  1. cleanup any old installation (pip uninstall cil ; rm $CONDA_PREFIX/lib/*cilacc.*)
  2. follow the updated README

open follow-up issues

  • [x] #2154
  • [x] #2155
  • [ ] PyPI dist
  • [ ] drop data submodule
    • [ ] PyPI dist for https://github.com/TomographicImaging/CIL-Data

casperdcl avatar Apr 23 '25 12:04 casperdcl

Oh I just realized - are you not doing the build with scikit-build-core yet? The reason I changed the name for the folder was because I believe scikit-build-core needs the last component of the file name to be the same as the library(otherwise the build doesn't work).

Perhaps making that tiny change in a separate PR in anticipation is a good idea to minimize conflict issues.

Edit: OH I see what's happening; the python package is being built by cmake itself, whereas I just had the c code being built by cmake, which is needed since you're not building a c-extension but a normal shared library

purepani avatar Apr 23 '25 18:04 purepani

After attempting it myself, it might just be easier to do all 3 at once(at least for me). I don't personally have the cmake knowledge to do the steps separately, and using scikit-build-core makes it way simpler.

purepani avatar Apr 24 '25 04:04 purepani

Thanks @purepani.

simplify IPP library discovery in 1 PR

Good point; it's now in #2148

And yes, separate PRs make it easier for review/debugging/approvals.

casperdcl avatar Apr 24 '25 05:04 casperdcl

I'm seeing

 C:\Users\krisf\AppData\Local\Temp\pip-build-env-sfqm_26k\overlay\Lib\site-packages\setuptools\dist.py:761: SetuptoolsDeprecationWarning: License classifiers are deprecated.
  !!

          ********************************************************************************
          Please consider removing the following classifiers in favor of a SPDX license expression:

          License :: Public Domain

          See https://packaging.python.org/en/latest/guides/writing-pyproject-toml/#license for details.
          ********************************************************************************

KrisThielemans avatar May 05 '25 07:05 KrisThielemans

Trying testing this according to https://github.com/TomographicImaging/CIL/issues/2141#issuecomment-2850866293 on windows 11 via Powershell with miniconda 25.3.0 in a new environment with only python and pip fails:

  • I had to use python -m pip install -U pip etc as pip install -U pip failed and told me to do that.
  • After that, all tests failed as import numpy failed. conda install numpy didn't help. python -m pip install --upgrade numpy did.
  • After that, some tests work, but others fails, e.g. because it cannot import scipy.

Possible reasons

  • I seem to have too much stuff in my base environment. This has given me strange effects when creating a new environment.
  • some conda/pip conflict.

I guess I'll have to throw away my conda environment and reinstall miniconda. I have no time for this now though. sorry

full log.txt

KrisThielemans avatar May 05 '25 23:05 KrisThielemans

The numpy issue may be caused by the numpy version not being restricted to <2.0.0. I recall the library having issues with numpy 2. It looks like when you reinstalled, a pre-2.0.0 version of numpy got installed, which supports that theory.

For scipy, maybe it's the same issue? Because you reinstalled numpy, and an older version got installed, the newer version of scipy broke(since it depends on numpy). I expect pinning numpy to <2.0.0 will fix both of these.

purepani avatar May 05 '25 23:05 purepani

I tested on Windows

pip install .
Processing c:\users\ofn77899\dev\cil
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Preparing metadata (pyproject.toml) ... done
Requirement already satisfied: dxchange in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from cil==24.3.1.dev41+g7a90040f3.d20250507) (0.1.7)
Requirement already satisfied: h5py in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from cil==24.3.1.dev41+g7a90040f3.d20250507) (3.13.0)
Requirement already satisfied: numba in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from cil==24.3.1.dev41+g7a90040f3.d20250507) (0.61.2)
Requirement already satisfied: numpy>=1.23 in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from cil==24.3.1.dev41+g7a90040f3.d20250507) (1.26.4)
Requirement already satisfied: olefile>=0.46 in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from cil==24.3.1.dev41+g7a90040f3.d20250507) (0.47)
Requirement already satisfied: pillow in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from cil==24.3.1.dev41+g7a90040f3.d20250507) (11.1.0)
Requirement already satisfied: pywavelets in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from cil==24.3.1.dev41+g7a90040f3.d20250507) (1.8.0)
Requirement already satisfied: scipy>=1.4.0 in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from cil==24.3.1.dev41+g7a90040f3.d20250507) (1.15.2)
Requirement already satisfied: tqdm in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from cil==24.3.1.dev41+g7a90040f3.d20250507) (4.67.1)
Requirement already satisfied: zenodo_get>=1.6 in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from cil==24.3.1.dev41+g7a90040f3.d20250507) (1.6.1)
Requirement already satisfied: requests in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from zenodo_get>=1.6->cil==24.3.1.dev41+g7a90040f3.d20250507) (2.32.3)
Requirement already satisfied: wget in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from zenodo_get>=1.6->cil==24.3.1.dev41+g7a90040f3.d20250507) (3.2)
Requirement already satisfied: humanize in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from zenodo_get>=1.6->cil==24.3.1.dev41+g7a90040f3.d20250507) (4.12.3)
Requirement already satisfied: llvmlite<0.45,>=0.44.0dev0 in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from numba->cil==24.3.1.dev41+g7a90040f3.d20250507) (0.44.0)
Requirement already satisfied: charset_normalizer<4,>=2 in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from requests->zenodo_get>=1.6->cil==24.3.1.dev41+g7a90040f3.d20250507) (3.4.2)
Requirement already satisfied: idna<4,>=2.5 in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from requests->zenodo_get>=1.6->cil==24.3.1.dev41+g7a90040f3.d20250507) (3.10)
Requirement already satisfied: urllib3<3,>=1.21.1 in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from requests->zenodo_get>=1.6->cil==24.3.1.dev41+g7a90040f3.d20250507) (2.4.0)
Requirement already satisfied: certifi>=2017.4.17 in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from requests->zenodo_get>=1.6->cil==24.3.1.dev41+g7a90040f3.d20250507) (2025.4.26)
Requirement already satisfied: colorama in c:\apps\newminiconda3\envs\cil_pip2\lib\site-packages (from tqdm->cil==24.3.1.dev41+g7a90040f3.d20250507) (0.4.6)
Building wheels for collected packages: cil
  Building wheel for cil (pyproject.toml) ... done
  Created wheel for cil: filename=cil-24.3.1.dev41+g7a90040f3.d20250507-cp312-cp312-win_amd64.whl size=371608 sha256=83b03d8651a760ec8da2bea534918cdd33a58f73af99add3af8006a239d6e4c7
  Stored in directory: C:\Users\ofn77899\AppData\Local\Temp\pip-ephem-wheel-cache-z04xletd\wheels\1c\b7\3e\24b74ef91ba830467453e1bab9adb32a55fa06ce28e1c93bd6
Successfully built cil
Installing collected packages: cil
Successfully installed cil-24.3.1.dev41+g7a90040f3.d20250507

and get the following errors.

from cil.optimisation.operators import GradientOperator
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Apps\newminiconda3\envs\cil_pip2\Lib\site-packages\cil\optimisation\operators\__init__.py", line 24, in <module>
    from .GradientOperator import GradientOperator
  File "C:\Apps\newminiconda3\envs\cil_pip2\Lib\site-packages\cil\optimisation\operators\GradientOperator.py", line 283, in <module>
    cilacc = ctypes.cdll.LoadLibrary(dll)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Apps\newminiconda3\envs\cil_pip2\Lib\ctypes\__init__.py", line 460, in LoadLibrary
    return self._dlltype(name)
           ^^^^^^^^^^^^^^^^^^^
  File "C:\Apps\newminiconda3\envs\cil_pip2\Lib\ctypes\__init__.py", line 369, in __init__
    if '/' in name or '\\' in name:
       ^^^^^^^^^^^
TypeError: argument of type 'NoneType' is not iterable

Checking into the site-packages there is only cil-data, so I don't know exactly where the package has been installed to.

paskino avatar May 07 '25 15:05 paskino

Did you pull before pip installing @paskino? I just pushed a commit because cil.optimisation.operators wasn't importing from cil.framework for no good reason.

casperdcl avatar May 07 '25 16:05 casperdcl

In other news, I rebased windows CI #1918 over this PR and 'tis actually passing 🤯

casperdcl avatar May 07 '25 16:05 casperdcl

Since I did not have your branch on my local repo, I did:

cd CIL
git fetch -a
git checkout skbuild

My branch is at the following head:

commit 7a90040f3c1a9537eed4afb3513b492f2daa3324 (HEAD -> skbuild, origin/skbuild)
Author: Casper da Costa-Luis <[email protected]>
Date:   Mon May 5 07:24:19 2025 +0100

    cilacc: fix windows paths

With your last commit

commit faf981e9c0b1fc304c048b92fc0441c1fe6480c7 (HEAD -> skbuild, origin/skbuild)
Author: Casper da Costa-Luis <[email protected]>
Date:   Wed May 7 16:39:57 2025 +0100

    fix operators import

unittests seem to run well:

Ran 1006 tests in 207.244s

OK (skipped=40)

paskino avatar May 08 '25 06:05 paskino

Installing in to full test environment everything worked, with all test packages.

----------------------------------------------------------------------
TEST SYSTEM CONFIGURATION
CIL version:  24.3.1.dev42+gfaf981e9
{'has_astra': True,
 'has_ccpi_regularisation': True,
 'has_cvxpy': True,
 'has_ipp': True,
 'has_matplotlib': True,
 'has_numba': True,
 'has_nvidia': True,
 'has_tigre': True,
 'has_tomophantom': True,
 'has_zenodo_get': True}
----------------------------------------------------------------------

Does this need to be tested via conda-build too?

gfardell avatar May 08 '25 09:05 gfardell

Does this need to be tested via conda-build too?

Sure

casperdcl avatar May 08 '25 11:05 casperdcl

currently trying to test conda build but failing, potentially because my conda is old-ish?

paskino avatar May 08 '25 21:05 paskino