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

fails to install on macos when USE_OMP=1 #51

Closed
Chrismarsh opened this issue Jul 30, 2020 · 24 comments
Closed

fails to install on macos when USE_OMP=1 #51

Chrismarsh opened this issue Jul 30, 2020 · 24 comments
Assignees

Comments

@Chrismarsh
Copy link

Chrismarsh commented Jul 30, 2020

On macos pip install pykdtree fails due to how omp is added to the command line

clang -Wno-unused-result -Wsign-compare -Wunreachable-code -DNDEBUG -g -fwrapv -O3 -Wall -I/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include -I/Library/Developer/CommandLineTools/SDKs/MacOSX.sdk/usr/include -I/Users/chris/.pyenv/versions/3.8.2/envs/analysis/include -I/Users/chris/.pyenv/versions/3.8.2/include/python3.8 -I/Users/chris/.pyenv/versions/3.8.2/envs/analysis/lib/python3.8/site-packages/numpy/core/include -c pykdtree/kdtree.c -o build/temp.macosx-10.15-x86_64-3.8/pykdtree/kdtree.o -std=c99 -O3 -fopenmp
    clang: error: unsupported option '-fopenmp'
    error: command 'clang' failed with exit status 1

MacOS requires the following omp invocation as detailed here
https://iscinumpy.gitlab.io/post/omp-on-high-sierra/

Alternatively, setting USE_OMP=0 by default on macos may be a better user experience.

edit: I should note that I see the setup.py code that checks for this. I am using a default install without conda. However, this seems to not be triggered correctly. Just checked the following:

>>> import sys
>>> 'darwin' not in sys.platform
False
>>> sys.platform
'darwin'
@mraspaud
Copy link
Collaborator

Hi,

The full check that you link to is use_omp = 'darwin' not in sys.platform or is_conda_interpreter()
So it would imply that setup thinks you are running conda.
What does sys.version return on your environment?

@Chrismarsh
Copy link
Author

It returns:

>>> sys.version
'3.8.2 (default, Jul  8 2020, 16:21:29) \n[Clang 11.0.0 (clang-1100.0.33.16)]'

@mraspaud
Copy link
Collaborator

mraspaud commented Aug 6, 2020

@Chrismarsh ok. Can you try testing the master branch on this repo, to see if this works better?

@Chrismarsh
Copy link
Author

@mraspaud worked for me in a fresh venv

@mraspaud
Copy link
Collaborator

mraspaud commented Aug 6, 2020

Ok then we need to make a new release

@mraspaud mraspaud self-assigned this Aug 6, 2020
@cavemanloverboy
Copy link

I am running into this on apple clang 14

@djhoese
Copy link
Collaborator

djhoese commented Mar 6, 2023

@cavemanloverboy Can you please provide more information? What version of Python? What version of pykdtree is being installed? What is the exact error including the traceback (if any) that shows up?

@djhoese
Copy link
Collaborator

djhoese commented Mar 6, 2023

Or...are you forcing USE_OMP (by specifying it as an environment variable) and then getting an error?

@cavemanloverboy
Copy link

I forced it and fixed it with the same flag -Xpreprocessor mentioned above.

@djhoese
Copy link
Collaborator

djhoese commented Mar 6, 2023

What does your final install command look like (for future reference)?

@cavemanloverboy
Copy link

What does your final install command look like (for future reference)?

don't have it on hand but I just added -Xpreprocessor right before -fopenmp

@Chrismarsh
Copy link
Author

@cavemanloverboy on macos 13.1 (arm64) with homebrew, I had to specify the libomp paths and fix the libomp library name for this to work.

The total link needs to look like this:
clang -Xpreprocessor -fopenmp -lomp -I"$(brew --prefix libomp)/include" -L"$(brew --prefix libomp)/lib" myfile

  • -fopenmp becomes -Xpreprocessor -fopenmp
  • the -I and -L search paths are built, from brew --prefix if homebrew is used. I'm uncertain what macports does or if you want to support
  • -lgomp needs to become -lomp

I can write a PR targeting homebrew env if you want.

@djhoese
Copy link
Collaborator

djhoese commented Mar 8, 2023

@Chrismarsh If we can avoid it, I'd really like to not have something specific to an environment. The only reason conda was special-cased in the setup.py was because it added functionality and was only used to control if openmp was used or not.

I'm surprised homebrew doesn't provide something that automatically includes the homebrew include/lib directories...but I guess that would require us/it to use its own provided compilers. I'll ask around at my work to see what other Mac developers use.

Bottom line I'm ok with whatever we have to do, but it'd be nice if we can get it as generic as possible. I'd still like to default to not attempting an OpenMP if we aren't absolutely sure that it will work. Put another way, I'd rather get "pykdtree isn't as fast as you say" than "pykdtree can't be installed on OSX" issues.

@Chrismarsh
Copy link
Author

@djhoese

I'm surprised homebrew doesn't provide something that automatically includes the homebrew include/lib directories

It looks like they changed how libomp is being installed as per Homebrew/homebrew-core#112107 (comment):

Users that now want to link to libomp will need to set additional flags:
==> Caveats
libomp is keg-only, which means it was not symlinked into /opt/homebrew,
because it can override GCC headers and result in broken builds.

For compilers to find libomp you may need to set:
export LDFLAGS="-L/opt/homebrew/opt/libomp/lib"
export CPPFLAGS="-I/opt/homebrew/opt/libomp/include"

The following works if

  • -Xpreprocessor is added to extra_compile_args = ['-std=c99', '-O3', '-Xpreprocessor', '-fopenmp']
  • -lgomp is changed to -lomp

USE_OMP=1 LDFLAGS="-L$(brew --prefix libomp)/lib" CPPFLAGS="-I$(brew --prefix libomp)/include" pip install .

Would you be open to adding a USE_OMP + darwin + not conda codepath to use
clang -Xpreprocessor -fopenmp -lomp

which could then be hinted via LDFLAGS which you could note in the readme? Then the default is "it just works" with " it works faster with more user intervention" as a user-opt in that is not tied to a specific environment? It seems that macos clang correctly hints -lomp when -fopenmp is used as I can remove extra_link_args=['-lomp']. However assuming that on macos omp is lomp seems pretty safe as both homebrew and macports use that.

@djhoese
Copy link
Collaborator

djhoese commented Mar 9, 2023

So it seems like everyone is having a different experience between the people in this thread and some coworkers who are trying some stuff out (@rayg-ssec). On @rayg-ssec's system he was able to specify USE_OMP=1 python3.11 -m pip wheel . which we think built pykdtree with OpenMP support on OSX. This was using macports Python 3.11.

setuptools/distutils Compiler

First, let's go over some things I've learned (yes, I know distutils is being deprecated, but it was the easiest source to find):

  1. We already have access to a Compiler object in the setup.py so we should hopefully have access to any information that we need as far as what compiler there is. We also have sysconfig from the standard library which can tell us how the current Python was compiled/installed.
  2. The Compiler object is in this stdlib module: https://github.com/python/cpython/blob/3.11/Lib/distutils/ccompiler.py. For OSX the compiler defaults to the same as the unix compiler class but with some flag updates for OSX. You can see this class in this module where you can see this dictionary of the binary compiler that will be called for various building steps: https://github.com/python/cpython/blob/ffb41eaaf4b04f7f52ee095253fcc1c5ba62ca28/Lib/distutils/unixccompiler.py#L55-L63
  3. The cc mentioned in the above stdlib seems to be a hard or soft link on OSX machines to clang. So this complicates things in our setup.py since we can't necessarily say "oh we're using cc so that means we're using clang".

Other examples

Next, I realized there is no way pykdtree is the only python package attempting to link to OpenMP so I did a github search and found some interesting strategies. Also, I think I've changed my mind on not doing anythign homebrew specific. If homebrew isn't automatically embedding the include and lib directories then that's not great, but if they exist then sure lets add them. Here are some of the examples I found:

  1. https://github.com/alejandrobll/py-sphviewer/blob/8fb92bd4f74ae571c7313d2b40c7b1d492c01a57/setup.py

This one is interesting because they actually make a small/little C program and try to build it with certain flags to see what is available. This might be a hacky but good option to say "we're 95% sure these flags should work on this system, but if compilation fails for this simple program turn off OpenMP and warn the user".

I saw at least one other package that attmepted this strategy of compiling a mini-program to determine supported flags.

  1. https://github.com/invesalius/inv3_plugins/blob/4f72564692652e7a9626c4796c36ea3bf6b336b4/porous_creation/setup.py

This one just has hardcoded flags for each platform.

Other concerns

I think no matter what is done the flags for OSX should not assume clang. Someone could use gcc on mac. If we want to assume cc means clang on OSX, fine, but then we should also check if some property of the CCompiler class (.executables?) includes mentions of gcc.

I think if we can code something up with the basic flags for each platform and maybe have a check for "are we using homebrew python and need addition includes/libs?" that would be a fine solution.

I guess if it is overall an improvement then 👍. Let me know what the rest of you think? @mraspaud @storpipfugl and the rest of you in this issue.

@Chrismarsh
Copy link
Author

I noticed that if I had a stale build/ folder, pip would report it was compiling and install but not actually do a new compiled. You can inject total garbage into setup.py, and run it without USE_OMP=1 to produce a "working" build/ folder, and then until you remove build/ it will blindly work. For example

No OMP base install

pip install .
Processing /Users/cmarsh/Desktop/pykdtree
  Preparing metadata (setup.py) ... done
Requirement already satisfied: numpy in /Users/cmarsh/.pyenv/versions/3.10.4/envs/snowcast-dev/lib/python3.10/site-packages (from pykdtree==1.3.6) (1.23.5)
Building wheels for collected packages: pykdtree
  Building wheel for pykdtree (setup.py) ... done
  Created wheel for pykdtree: filename=pykdtree-1.3.6-cp310-cp310-macosx_12_0_arm64.whl size=53821 sha256=a512c061f90fdc419fb1ab008b07d735bb77002de9ac7d875f535a98d61e925c
  Stored in directory: /private/var/folders/tr/v_kqwm7x25x3y68l_p5qh1gw0000gn/T/pip-ephem-wheel-cache-legenj4a/wheels/b6/ae/51/ad03df621aabfddff6624ea12de81ab05e54b49da13042f0ec
Successfully built pykdtree
Installing collected packages: pykdtree
Successfully installed pykdtree-1.3.6

[notice] A new release of pip is available: 23.0 -> 23.0.1
[notice] To update, run: pip install --upgrade pip

Uninstall and reinstall with omp "seems to work"

USE_OMP=1 pip install .
Processing /Users/cmarsh/Desktop/pykdtree
  Preparing metadata (setup.py) ... done
Requirement already satisfied: numpy in /Users/cmarsh/.pyenv/versions/3.10.4/envs/snowcast-dev/lib/python3.10/site-packages (from pykdtree==1.3.6) (1.23.5)
Building wheels for collected packages: pykdtree
  Building wheel for pykdtree (setup.py) ... done
  Created wheel for pykdtree: filename=pykdtree-1.3.6-cp310-cp310-macosx_12_0_arm64.whl size=53821 sha256=f214bbe19cdb87bb86fb6074339e381845b62b0e180e264f7adb93e4cf211408
  Stored in directory: /private/var/folders/tr/v_kqwm7x25x3y68l_p5qh1gw0000gn/T/pip-ephem-wheel-cache-syq6adx5/wheels/b6/ae/51/ad03df621aabfddff6624ea12de81ab05e54b49da13042f0ec
Successfully built pykdtree
Installing collected packages: pykdtree
Successfully installed pykdtree-1.3.6

[notice] A new release of pip is available: 23.0 -> 23.0.1
[notice] To update, run: pip install --upgrade pip

except that setup.py was modified to have garbage in the link line which means it cannot possibly have worked.

+                extra_link_args=['-lasdfomp']

I git reset the setup.py changes, and rm -rf build, causing the clean omp build to fail as one would expected

USE_OMP=1 pip install .
Processing /Users/cmarsh/Desktop/pykdtree
  Preparing metadata (setup.py) ... done
Requirement already satisfied: numpy in /Users/cmarsh/.pyenv/versions/3.10.4/envs/snowcast-dev/lib/python3.10/site-packages (from pykdtree==1.3.6) (1.23.5)
Building wheels for collected packages: pykdtree
  Building wheel for pykdtree (setup.py) ... error
  error: subprocess-exited-with-error

  × python setup.py bdist_wheel did not run successfully.
  │ exit code: 1
  ╰─> [16 lines of output]
      running bdist_wheel
      running build
      running build_py
      creating build
      creating build/lib.macosx-12.2-arm64-cpython-310
      creating build/lib.macosx-12.2-arm64-cpython-310/pykdtree
      copying pykdtree/test_tree.py -> build/lib.macosx-12.2-arm64-cpython-310/pykdtree
      copying pykdtree/__init__.py -> build/lib.macosx-12.2-arm64-cpython-310/pykdtree
      copying pykdtree/render_template.py -> build/lib.macosx-12.2-arm64-cpython-310/pykdtree
      running build_ext
      building 'pykdtree.kdtree' extension
      creating build/temp.macosx-12.2-arm64-cpython-310
      creating build/temp.macosx-12.2-arm64-cpython-310/pykdtree
      clang -Wno-unused-result -Wsign-compare -Wunreachable-code -DNDEBUG -g -fwrapv -O3 -Wall -I/Library/Developer/CommandLineTools/SDKs/MacOSX12.3.sdk/usr/include -I/Library/Developer/CommandLineTools/SDKs/MacOSX12.3.sdk/usr/include -I/Users/cmarsh/.pyenv/versions/snowcast-dev/include -I/Users/cmarsh/.pyenv/versions/3.10.4/include/python3.10 -I/Users/cmarsh/.pyenv/versions/snowcast-dev/lib/python3.10/site-packages/numpy/core/include -c pykdtree/_kdtree_core.c -o build/temp.macosx-12.2-arm64-cpython-310/pykdtree/_kdtree_core.o -std=c99 -O3 -fopenmp
      clang: error: unsupported option '-fopenmp'
      error: command '/usr/bin/clang' failed with exit code 1
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
  ERROR: Failed building wheel for pykdtree
  Running setup.py clean for pykdtree
Failed to build pykdtree
Installing collected packages: pykdtree
  Attempting uninstall: pykdtree
    Found existing installation: pykdtree 1.3.6
    Uninstalling pykdtree-1.3.6:
      Successfully uninstalled pykdtree-1.3.6
  Running setup.py install for pykdtree ... error
  error: subprocess-exited-with-error

  × Running setup.py install for pykdtree did not run successfully.
  │ exit code: 1
  ╰─> [18 lines of output]
      running install
      /Users/cmarsh/.pyenv/versions/snowcast-dev/lib/python3.10/site-packages/setuptools/command/install.py:34: SetuptoolsDeprecationWarning: setup.py install is deprecated. Use build and pip and other standards-based tools.
        warnings.warn(
      running build
      running build_py
      creating build
      creating build/lib.macosx-12.2-arm64-cpython-310
      creating build/lib.macosx-12.2-arm64-cpython-310/pykdtree
      copying pykdtree/test_tree.py -> build/lib.macosx-12.2-arm64-cpython-310/pykdtree
      copying pykdtree/__init__.py -> build/lib.macosx-12.2-arm64-cpython-310/pykdtree
      copying pykdtree/render_template.py -> build/lib.macosx-12.2-arm64-cpython-310/pykdtree
      running build_ext
      building 'pykdtree.kdtree' extension
      creating build/temp.macosx-12.2-arm64-cpython-310
      creating build/temp.macosx-12.2-arm64-cpython-310/pykdtree
      clang -Wno-unused-result -Wsign-compare -Wunreachable-code -DNDEBUG -g -fwrapv -O3 -Wall -I/Library/Developer/CommandLineTools/SDKs/MacOSX12.3.sdk/usr/include -I/Library/Developer/CommandLineTools/SDKs/MacOSX12.3.sdk/usr/include -I/Users/cmarsh/.pyenv/versions/snowcast-dev/include -I/Users/cmarsh/.pyenv/versions/3.10.4/include/python3.10 -I/Users/cmarsh/.pyenv/versions/snowcast-dev/lib/python3.10/site-packages/numpy/core/include -c pykdtree/_kdtree_core.c -o build/temp.macosx-12.2-arm64-cpython-310/pykdtree/_kdtree_core.o -std=c99 -O3 -fopenmp
      clang: error: unsupported option '-fopenmp'
      error: command '/usr/bin/clang' failed with exit code 1
      [end of output]

  note: This error originates from a subprocess, and is likely not a problem with pip.
  Rolling back uninstall of pykdtree
  Moving to /Users/cmarsh/.pyenv/versions/3.10.4/envs/snowcast-dev/lib/python3.10/site-packages/pykdtree-1.3.6.dist-info/
   from /Users/cmarsh/.pyenv/versions/3.10.4/envs/snowcast-dev/lib/python3.10/site-packages/~ykdtree-1.3.6.dist-info
  Moving to /Users/cmarsh/.pyenv/versions/3.10.4/envs/snowcast-dev/lib/python3.10/site-packages/pykdtree/
   from /Users/cmarsh/.pyenv/versions/3.10.4/envs/snowcast-dev/lib/python3.10/site-packages/~ykdtree
error: legacy-install-failure

× Encountered error while trying to install package.
╰─> pykdtree

note: This is an issue with the package mentioned above, not pip.
hint: See above for output from the failure.

[notice] A new release of pip is available: 23.0 -> 23.0.1
[notice] To update, run: pip install --upgrade pip

I think support homebrew and macports would be very welcome to macos users. Unhelpfully, to get around this in my python code I use CMake to directly determine compiler and library locations.

@djhoese
Copy link
Collaborator

djhoese commented Mar 9, 2023

Ok so @rayg-ssec sent me a diff that adds much better handling of clang versus gcc by changing the usage of USE_OMP to allow for "guessing" of what platform and compiler is being used. In its current state it still requires you the user to provide include/lib directories, but Ray and I talked and we think it would be pretty easy to have it inspect the current python executablve being used and determine some possible defaults for these extra includes (with some intelligent "exists" checks). I think if we assume standard directories for homebrew and macports we can get 99% of OSX users to compile source with OpenMP support. I think we (the maintainers) would still need to distribute wheels without OpenMP, but if someone says they want to install from source then it should be "automatic".

I'm extremely behind on most of my real work projects, but as long as things go well tomorrow I should be able to make a PR for these changes tomorrow or this weekend. Then everyone here could give it a test hopefully.

@rayg-ssec
Copy link
Contributor

I wound up taking a swing at fully automating a default detection of brew/port openmp while maintaining backward compatibility and manual configurability between omp and gomp, but it could use some testers. In theory this is most easily checked with pip install -v .

#83

https://github.com/rayg-ssec/pykdtree/tree/feat-macos-libomp-automatic

@djhoese
Copy link
Collaborator

djhoese commented Mar 10, 2023

If people want to try this without cloning Ray's git manually, you should be able to do:

pip install -v git+https://github.com/rayg-ssec/pykdtree.git@feat-macos-libomp-automatic

Edit: Fixed the username in the above URL

@rayg-ssec
Copy link
Contributor

The key line in the debug output notes the compiler category (unix), the omp/gomp selection, compiler flags, linker flags for OpenMP.

  >>>> unix omp ['-Xpreprocessor', '-fopenmp', '-I/opt/local/include/libomp'] ['-lomp', '-L/opt/local/lib/libomp']

@rayg-ssec
Copy link
Contributor

rayg-ssec commented Mar 13, 2023

A quick confirmation check recipe...

python3.11 -m venv --system-site-packages venv-pykdtree
source venv-pykdtree/bin/activate
pip install -v git+https://github.com/rayg-ssec/pykdtree.git@feat-macos-libomp-automatic
SOFILE="$(python -c 'from pykdtree import kdtree; print(kdtree.__file__)')"
echo $SOFILE 
otool -L "$SOFILE"  # will show libomp for macports case, but brew has static libomp
nm  "$SOFILE" |grep omp  # should show _.omp_ symbols for libomp, _omp_ for libgomp

To try it with gcc (e.g. gcc-mp-12 from macports), substitute
USE_OMP=gcc CC=gcc-mp-12 pip install -v git+https://github.com/rayg-ssec/pykdtree.git@feat-macos-libomp-automatic

And to turn off openmp
USE_OMP=0 pip install -v git+https://github.com/rayg-ssec/pykdtree.git@feat-macos-libomp-automatic

@Chrismarsh
Copy link
Author

Seems to work for me (macos + brew)

pip install -v git+https://github.com/rayg-ssec/pykdtree.git@feat-macos-libomp-automatic

I get

nm "$SOFILE" | grep omp
00000000000037a8 t _.omp_outlined.
0000000000004e50 t _.omp_outlined..3
                 U _PyObject_RichCompare
                 U _PyObject_RichCompareBool
                 U _PyUnicode_Compare

@djhoese
Copy link
Collaborator

djhoese commented Mar 20, 2023

#83 has now been merged. I'm going to release a patch release in a little bit. Closing this as fixed. If you run into issues with these changes please file a new github issue with details on the errors you are receiving and your system/environment.

@djhoese djhoese closed this as completed Mar 20, 2023
@djhoese
Copy link
Collaborator

djhoese commented Mar 20, 2023

This is now released as version 1.3.7 on PyPI: https://pypi.org/project/pykdtree/. This really only affects people installing with the --no-binary flag with pip. Otherwise, linux is still the only platform where wheels are built with OpenMP.

A conda-forge update will come later, but there won't be any changes to how that binary package uses OpenMP.

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

5 participants