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

Numerous build issues with newer CMake, Python, virtual environments #2376

Open
nhubbard opened this issue Oct 26, 2024 · 5 comments
Open

Numerous build issues with newer CMake, Python, virtual environments #2376

nhubbard opened this issue Oct 26, 2024 · 5 comments
Labels
bug Unexpected behaviour that should be corrected (type)

Comments

@nhubbard
Copy link

nhubbard commented Oct 26, 2024

🐞Describing the bug

I'm on macOS 15.1 RC 2, Python 3.12.7 provided by pyenv, and CMake 3.30.5 provided by Homebrew.

I'm trying to use Python 3.12 with the latest 8.3 release of Ultralytics and coremltools to export my YOLO11 models. Due to the distributed coremltools not being compatible with 3.12, and my desire to build both NumPy and Torch from source to get maximum performance, I elected to build coremltools from source.

When building coremltools, I ran into several issues with CMake failing to detect the Python and NumPy include libraries in my virtual environment, along with some other issues that may need to be addressed.

I found the following problems, in no particular order:

  1. The build.sh script mis-documents the --num-procs option as --num_procs.
  2. The FindPythonInterp and FindPythonLibs CMake modules have been deprecated since CMake 3.12, and are considered as no-ops since CMake 3.27.
  3. The custom detection logic used to find NumPy include directories is very buggy; I found out that the old Python detection logic was trying to use my system Python, and once I fixed that, it ran into "permission denied" (???) errors running the inline Python script to get the NumPy include directory.
  4. I had to comment out all of the Anaconda setup because I didn't have Anaconda installed. I personally don't want to use Anaconda for my environment; I don't consider this a problem for you to address, but I do think it's a mistake for the build scripts to barrel forward without checking whether conda is available. Commenting out the uses of conda may also have caused some of the issues in 3.
  5. I'm confused as to why Numpy is pinned to 2.0.0, when it works just fine with my from-source copy of NumPy 2.1.2. The build script tried to overwrite my from-source copy of Numpy with 2.0.0.
  6. Building and running coremltools on Python 3.12.7 caused the distributed wheel file to be called coremltools-8.0-cp-none-macosx_11_0_arm64.whl, which isn't compliant with the PyPI file name convention. This caused Pip to refuse installation of the wheel file. I had to rename the wheel to coremltools-8.0-py3-none-macosx_11_0_arm64.whl to allow installation.

How to fix the issues

I would open a PR with my fixes, but some of them are more specific to my needs, so they would need to be cherry picked based on the needs of the general project.

Instead, I've chosen to list all of the fixes here, and provide a diff showing the changes I made to fix some of the issues, so the maintainers can discuss the changes first.

I also feel that if I were to preemptively split the changes up, it would cause discussion about the changes, some of which are inherently linked to one another, to be split in multiple places.

CMake

  1. Raise the minimum CMake version to 3.14 to get access to both the FindPython3 module and the NumPy component. CMake 3.10.2 is woefully out of date. Homebrew, Anaconda, and PyPI provide far more up to date versions of CMake in a readily accessible manner.
  2. Use the components option for the FindPython3 module to get the interpreter, development, and NumPy variables automatically.
  3. Fix the Python3 and NumPy include directory logic by using the provided Python3_EXECUTABLE, Python3_VERSION_STRING, Python3_INCLUDE_DIRS, and Python3_NumPy_INCLUDE_DIRS result variables.
  4. Stop using the custom inline Python script to find the include directories, as it's prone to unusual issues on newer versions of CMake.

Dependencies

  1. Consider dropping support for Python 3.7 and 3.8. They are both unsupported upstream, and newer versions of macOS come with 3.9 out of the box.
  2. Release a wheel with Python 3.12 support; as far as I can tell, the only issue with 3.12 (Replace deprecated imp module with importlib #1846) has been resolved.
  3. Test coremltools with PyTorch 2.5.0. As far as I can tell, there's nothing different, but I don't use all of the possible options, so this may be more difficult than I expect.
  4. Allow newer versions of Numpy, beyond the pinned 2.0.0, to be used.

Build script

  1. Fix the help in the build.sh script so that --num-procs is documented correctly.
  2. Add a check for the conda executable, and potentially an environment variable to skip using conda to create a new environment for users that don't want to use it (with the note of no support if it causes issues).
  3. Determine why the output wheel is named incorrectly.

Patches

This diff should fix issues 1-4 for CMake, issue 4 for the dependencies, and issue 1 for the build script.

diff --git a/CMakeLists.txt b/CMakeLists.txt
index d4625252..65ce6f71 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -1,4 +1,4 @@
-cmake_minimum_required(VERSION 3.10.2)
+cmake_minimum_required(VERSION 3.14.0)

 set(CMAKE_DISABLE_IN_SOURCE_BUILD ON)

@@ -34,12 +34,12 @@ endif()
 add_subdirectory(deps)
 add_subdirectory(mlmodel)

-find_package(PythonInterp)
-find_package(PythonLibs)
+set(Python3_FIND_VIRTUALENV FIRST)
+find_package(Python3 REQUIRED COMPONENTS Interpreter Development NumPy)

-message("Found python at ${PYTHON_EXECUTABLE}")
-message("Found python version ${PYTHON_VERSION_STRING}")
-message("Found python includes ${PYTHON_INCLUDE_DIRS}")
+message("Found python at ${Python3_EXECUTABLE}")
+message("Found python version ${Python3_VERSION_STRING}")
+message("Found python includes ${Python3_INCLUDE_DIRS}")

 include_directories(
   .
@@ -47,7 +47,8 @@ include_directories(
   deps/pybind11/include
   deps/nlohmann
   mlmodel/src
-  ${PYTHON_INCLUDE_DIRS}
+  ${Python3_INCLUDE_DIRS}
+  ${Python3_NumPy_INCLUDE_DIRS}
   )

 if(APPLE)
@@ -133,19 +134,8 @@ find_library(CORE_ML CoreML)
 find_library(FOUNDATION Foundation)

 if (APPLE AND CORE_VIDEO AND CORE_ML AND FOUNDATION)
-  execute_process(
-      COMMAND ${PYTHON_EXECUTABLE} -c "import numpy; print(numpy.get_include())"
-      RESULT_VARIABLE NUMPY_INCLUDE_STATUS
-      OUTPUT_VARIABLE NUMPY_INCLUDE
-  )
-
-  if("${NUMPY_INCLUDE}" STREQUAL "" OR NOT NUMPY_INCLUDE_STATUS EQUAL 0)
-      message(FATAL_ERROR "Could not find numpy include path. Exit code: ${NUMPY_INCLUDE_STATUS}")
-  endif()
-  message("Found numpy include path at ${NUMPY_INCLUDE}")
-
   include_directories(
-    ${NUMPY_INCLUDE}
+	  ${Python3_NumPy_INCLUDE_DIR}
   )

   add_library(coremlpython
diff --git a/reqs/build.pip b/reqs/build.pip
index 2c4d1ced..828ac6a5 100644
--- a/reqs/build.pip
+++ b/reqs/build.pip
@@ -1,7 +1,4 @@
numpy==1.21.0; platform_machine == "arm64" and python_version < "3.9"
numpy<1.20; platform_machine != "arm64" and python_version < "3.9"
-numpy==2.0.0; python_version >= "3.9"
-
+numpy>=2.0.0; python_version >= "3.9"
 six
 sympy
 tqdm
diff --git a/scripts/build.sh b/scripts/build.sh
index 56998f8b..85a7cdc5 100755
--- a/scripts/build.sh
+++ b/scripts/build.sh
@@ -33,7 +33,7 @@ print_help() {
   echo
   echo "Usage: build.sh"
   echo
-  echo "  --num_procs=n (default 1)       Specify the number of processes to run in parallel."
+  echo "  --num-procs=n (default 1)       Specify the number of processes to run in parallel."
   echo "  --python=*                      Python to use for configuration."
   echo "  --protobuf                      Rebuild & overwrite the protocol buffers in MLModel."
   echo "  --debug                         Build without optimizations and stripping symbols."

System environment (please complete the following information):

  • coremltools version: main branch
  • OS (e.g. MacOS version or Linux type): macOS 15.1 RC 2
  • Any other relevant version information (e.g. PyTorch or TensorFlow version): PyTorch 2.5.0
@nhubbard nhubbard added the bug Unexpected behaviour that should be corrected (type) label Oct 26, 2024
@YifanShenSZ
Copy link
Collaborator

Hi @nhubbard thanks for raising this issue. (@TobyRoseman correct me if I'm wrong) we are also experiencing build failures when trying to support python 3.12, once we resolve that we should have more clarity

@YifanShenSZ YifanShenSZ added triaged Reviewed and examined, release as been assigned if applicable (status) and removed triaged Reviewed and examined, release as been assigned if applicable (status) labels Oct 28, 2024
@TobyRoseman
Copy link
Collaborator

No build failure with Python 3.12. We do have plenty of test failures, mostly related to the dependency versions we're using.

@YifanShenSZ
Copy link
Collaborator

Our 8.1 release has added python 3.12 support 🎊 Please try it out

@nhubbard
Copy link
Author

nhubbard commented Nov 22, 2024

Can confirm, the 8.1 release is wonderful!

There's still a few more issues, like the build script options being midocumented, the fragile Python finding process in the CMakeLists, and the pinned Numpy version. Should we keep this issue open for those or open PRs to fix them? I'm more concerned about opening multiple PRs to fix the individual issues and wanted to collect feedback before doing that.

@YifanShenSZ
Copy link
Collaborator

Sure 🙏 Thanks for pointing out those issues

@YifanShenSZ YifanShenSZ reopened this Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Unexpected behaviour that should be corrected (type)
Projects
None yet
Development

No branches or pull requests

3 participants