openmc icon indicating copy to clipboard operation
openmc copied to clipboard

wheel building with scikit build core

Open shimwell opened this issue 1 year ago • 23 comments

Description

Just making this PR to show a bit of progress with the production of wheels using scikit build core. Tagging @hassec as he made the OpenMCExtension part of this PR.

Running pip install . in the root folder of this repo and this will compile openmc and build a wheel and then install the python package and openmc executable from the wheel with default cmake args.

It is also possible to pass in cmake args via the command line in the same line as the pip install command.

you can also run pip install . --no-clean This will do the same but it won't delete the wheel afterwards. This allows you to inspect the wheel and manually install from the wheel. The path to the wheel is printed to the terminal and will be in your /tmp folder

I've added some if(SKBUILD) statements to the CMakeLists.txt to help insure the new commands only run if the process is being driven by Scikit build core and therefore this preserves the current behavior when someone wants to build the executable with cmake

I can follow up this PR to include ciwheelbuilder to make wheels for different OS versions

Fixes # (issue)

Checklist

  • [x] I have performed a self-review of my own code
  • [x] I have run clang-format (version 15) on any C++ source files (if applicable)
  • [x] I have followed the style guidelines for Python source files (if applicable)
  • [x] I have made corresponding changes to the documentation (if applicable)
  • [ ] I have added tests that prove my fix is effective or that my feature works (if applicable)

shimwell avatar Jul 19 '24 00:07 shimwell

for anyone interested in wheels that contain DAGMC, Double Down, Embree and Moab options in the OpenMC executable then

I have that working on this branch, clone or checkout that branch https://github.com/shimwell/openmc/tree/adding_dagmc_to_scikit_build_core then run the script bash pre_wheel_install_dagmc_embree_dd_moab.sh then run pip install . --no-clean and your wheel will be in a sub directory of the /tmp folder (full path is printed to terminal)

shimwell avatar Jul 19 '24 12:07 shimwell

just to note a new pip version is needed to pass in cmake arguments via the command line, if you get this error then upgrade pip (I'm using version 24.1.2)

pip install . --config-settings=cmake.args=-DCMAKE_BUILD_TYPE=RELEASE
>>> no such option: --config-settings

solve with

python -m pip install --upgrade pip

shimwell avatar Jul 23 '24 17:07 shimwell

nice the docs on readthedocs are now working, it needed a bit of tweaking as they now installing the package with this scikit build core method.

shimwell avatar Jul 26 '24 12:07 shimwell

Looks like the tests/unit_tests/test_endf.py:12: is failing with ValueError: could not convert string to float: '+ 2 . 3+ 1' so perhaps something has gone wrong with the cython

shimwell avatar Jul 27 '24 00:07 shimwell

Looks like the tests/unit_tests/test_endf.py:12: is failing with ValueError: could not convert string to float: '+ 2 . 3+ 1' so perhaps something has gone wrong with the cython

I think this PR will help with the current error as it makes use of the pip installable endf package for that functionality :smile:

shimwell avatar Jul 29 '24 17:07 shimwell

The endf tests are now passing but it looks like 3 regression tests are still failing in the CI

the difference in the numbers is very small, here is one example

 Result differences:
--- results_true.dat
+++ results_test.dat
@@ -1,2 +1,2 @@
 k-combined:
-1.716873E+00 5.266107E-02
+1.716873E+00 5.266094E-02

I'm not sure if it is best to change the regression test results to match or why this method of compiling produces small differences

shimwell avatar Aug 14 '24 15:08 shimwell

tests/regression_tests/deplete_with_transfer_rates/test.py which runs depletion simulations is failing due to small differences in the numbers of atoms

tests/regression_tests/source_parameterized_dlopen/test.py which makes use of cmake to compile sources is failing to run cmake command

CMake Error at CMakeLists.txt:5 (find_package):
  Could not find a package configuration file provided by "OpenMC" with any
  of the following names:

    OpenMCConfig.cmake
    openmc-config.cmake

jon-proximafusion avatar Aug 14 '24 20:08 jon-proximafusion

Looks like tests are failing with openmc: error while loading shared libraries: libmcpl.so: cannot open shared object file: No such file or directory

jon-proximafusion avatar Sep 25 '24 08:09 jon-proximafusion

locally I'm getting this error when trying to pip install .

        The link interface of target "xtensor" contains:
      
          OpenMP::OpenMP_CXX_xtensor
      
        but the target was not found.  Possible reasons include:

jon-proximafusion avatar Sep 25 '24 09:09 jon-proximafusion

CI is failing with this error

args = ['openmc']
output = 'openmc: error while loading shared libraries: libmcpl.so: cannot open shared object file: No such file or directory\n'
cwd = PosixPath('.')

perhaps we are missing a compile with mcpl flag somewhere

shimwell avatar Sep 26 '24 14:09 shimwell

It seems libmcpl.so was installed correctly and in the system lib dir, so there is no need to setup LD_LIBRARY_PATH for this. Need to test locally.

ahnaf-tahmid-chowdhury avatar Sep 26 '24 15:09 ahnaf-tahmid-chowdhury

Only one test is failing, stating that data/compton_profiles.h5 is not found. I tested this on my machine, and it was working. I'm not sure why sdist is not including it.

ahnaf-tahmid-chowdhury avatar Oct 21 '24 15:10 ahnaf-tahmid-chowdhury

I've solved the merge conflicts and kept this upto date with develop I've also reduced the diff in a few places so this PR is slightly more minimal

shimwell avatar Nov 08 '24 14:11 shimwell

CI is currently failing to find some h5 data files so it looks like the packaging is not copying across those files when being built

jon-proximafusion avatar Nov 27 '24 16:11 jon-proximafusion

I am not sure why the workflow is not including the .h5 files when creating the archive. Perhaps, should we consider using a MANIFEST file?

ahnaf-tahmid-chowdhury avatar Nov 27 '24 16:11 ahnaf-tahmid-chowdhury

Just in case anyone wants to try this pip install this command points to the latest commit on the branch

pip install git+https://github.com/shimwell/openmc/@6e8d1af4373b41eb0aab4adbc33e3da6c80c6a6e

updated to include the "do not ignore .h5" files comit

shimwell avatar Nov 27 '24 17:11 shimwell

Not sure how to deal with coverage/coveralls

ahnaf-tahmid-chowdhury avatar Nov 28 '24 09:11 ahnaf-tahmid-chowdhury

Congrats on getting h5 file to join the package and get the CI passing

Looks like most of the coverage drop is in the cpp files which aren't touched by the PR so that is interesting.

shimwell avatar Nov 29 '24 08:11 shimwell

The coverage command is in the CI here and it has flags for directories to include and ignore. https://github.com/shimwell/openmc/blob/b2abffb09915e9b7484e14050ecfad079061413f/.github/workflows/ci.yml#L160C1-L161C58 I'm wondering if we have an issue as the cpp and python are now both in the same directory.

shimwell avatar Dec 02 '24 16:12 shimwell

@ahnaf-tahmid-chowdhury congratulations for getting the coverage to pass as well. You are a super star.

I think this is ready for a review if any one has time

shimwell avatar Dec 03 '24 06:12 shimwell

Just solved a couple of conflicts to keep this branch up to date

jon-proximafusion avatar Jan 09 '25 16:01 jon-proximafusion

python package wheel can be built and tested on this PR using these commands from this branch with python 3.12, I can't get python 3.11 to work

# build manylinux image
docker build -t openmc -f tools/ci/manylinux.dockerfile .

docker build --no-cache --build-arg Python_ABI=cp312-cp312 -t openmc_wheel:python3.12 -f tools/ci/manylinux.dockerfile .

copy out the wheels Create a container from the built image Copy the wheel files from the container to the wheelhouse folder on host machine Remove the container Create a container from the built image Copy the wheel files from the container to the wheelhouse folder on host machine Remove the container

docker create --name openmc_wheel_container_3.12 openmc_wheel:python3.12
mkdir wheelhouse
docker cp openmc_wheel_container_3.12:/root/openmc/dist/. wheelhouse
docker rm openmc_wheel_container_3.12

docker image for testing resulting wheel called wheeltest.Dockerfile

FROM ubuntu:22.04
RUN apt upgrade -y
RUN apt update -y
RUN apt install libmpich-dev libhdf5-serial-dev libhdf5-mpich-dev libhdf5-hl-100 libhdf5-dev -y
RUN apt install -y python3.12 python3-pip -y
COPY wheelhouse/openmc-0.15.1.dev0-cp312-cp312-manylinux_2_28_x86_64.whl .
RUN python3.11 -m pip install openmc-0.15.1.dev0-cp312-cp312-manylinux_2_28_x86_64.whl
RUN python3.11 -c "import openmc.lib"

docker build -f wheeltest.Dockerfile --build-arg python_version=3.12 .

shimwell avatar Feb 05 '25 13:02 shimwell

RUN python3.11 -m pip install openmc-0.15.1.dev0-cp312-cp312-manylinux_2_28_x86_64.whl

How is Python 3.11 able to install a package built for Python 3.12!

ahnaf-tahmid-chowdhury avatar Feb 06 '25 16:02 ahnaf-tahmid-chowdhury