rustychris / stomel

Spatial tools for ocean modeling in python
GNU General Public License v2.0
10 stars 6 forks source link

OSX support #1

Open pwolfram opened 8 years ago

pwolfram commented 8 years ago

What are the requirements to provide OSX support (listed in README)? Great to see you put this on github Rusty!

pwolfram commented 8 years ago

I should note there is no rush on this and I'm happy to help as possible.

rustychris commented 8 years ago

Hi Phil - Good to hear from you.

I've been trying to get this to work in anaconda, but the main sticking point is compiling the python-cgal-bindings. The compilation process in CGAL/SWIG is a little too clever for anaconda, and some of the paths embedded in the libraries don't get updated, or at least that's my best interpretation.

Here is the error I get:

python ~/anaconda/envs/tom_test/lib/python2.7/site-packages/stomel/tom.py -b spinup-shoreline.shp -s density-spinup.shp 
/Users/rusty/anaconda/envs/tom_test/lib/python2.7/site-packages/matplotlib/cbook.py:133: MatplotlibDeprecationWarning: The matplotlib.delaunay module was deprecated in version 1.4. Use matplotlib.tri.Triangulation instead.
  warnings.warn(message, mplDeprecation, stacklevel=1)
CGAL unavailable.
dlopen(/Users/rusty/anaconda/envs/tom_test/lib/python2.7/site-packages/CGAL/_CGAL_Triangulation_2.so, 2): Library not loaded: /Users/rusty/anaconda/conda-bld/work/lib/libCGAL_Triangulation_2_cpp.dylib
  Referenced from: /Users/rusty/anaconda/envs/tom_test/lib/python2.7/site-packages/CGAL/_CGAL_Triangulation_2.so
  Reason: image not found

This is from a cleanly installed anaconda environment: .condarc

channels:
 - ioos
 - rustychris
 - defaults 

Followed by conda create -n tom_test python=2.7, source activate tom_test, and conda install stomel.

If you have any thoughts, the recipe for python-cgal-bindings is here

pwolfram commented 8 years ago

@rustychris, I'm assuming that this works for you provided you do the local compile, right? I tried hacking this by updating all the library paths by hand from your binaries but it seems like something is off with the cmake script for CGAL so that it is platform dependent (due to boost?). Ultimately I ended up with a different error due to threading, like this:

In [2]: import CGAL._CGAL_Triangulation_2
Fatal Python error: PyThreadState_Get: no current thread
/Users/pwolfram/miniconda2/envs/tom_test/bin/python.app: line 3: 23297 Abort trap: 6           /Users/pwolfram/miniconda2/envs/tom_test/python.app/Contents/MacOS/python "$@"

I'll try building from source on my platform next which should hopefully solve the above problem (although obviously not in a general way).

pwolfram commented 8 years ago

It appears that https://code.google.com/r/rustychris-apollonius/ is down... (see https://github.com/rustychris/conda-recipes/pull/3)

pwolfram commented 8 years ago

Part of the issue is likely due to using $ORIGIN on linux / @loader_path on OSX (http://conda.pydata.org/docs/building/shared-libraries.html)

pwolfram commented 8 years ago

Another potential problem is that it may be linking to the OSX version of python and not anaconda's: e.g., /System/Library/Frameworks/Python.framework/Versions/2.7/Python when the bindings are built from source. Essentially, it is linking to the clang python framework associated with OS X.

pwolfram commented 8 years ago

Here are some minor changes I needed to do to get this to compile: https://github.com/pwolfram/conda-recipes/tree/generalize, but I still have that Fatal Python error: PyThreadState_Get: no current thread error and don't know to resolve it. Google searches imply that it is a mismatch between the library and calling python versions but it is unclear to me what should be linked instead of what we have presently (if this is the problem):

└─▪ otool -L /Users/pwolfram/miniconda2/envs/tom_test/lib/python2.7/site-packages/CGAL/_CGAL_Triangulation_2.so
/Users/pwolfram/miniconda2/envs/tom_test/lib/python2.7/site-packages/CGAL/_CGAL_Triangulation_2.so:
    /usr/local/lib/libmpfr.4.dylib (compatibility version 6.0.0, current version 6.2.0)
    @rpath/./libgmp.10.dylib (compatibility version 12.0.0, current version 12.3.0)
    @rpath/libCGAL_ImageIO.10.dylib (compatibility version 10.0.0, current version 10.0.2)
    @rpath/libCGAL.10.dylib (compatibility version 10.0.0, current version 10.0.2)
    /usr/local/lib/libboost_thread-mt.dylib (compatibility version 0.0.0, current version 0.0.0)
    /usr/local/lib/libboost_system-mt.dylib (compatibility version 0.0.0, current version 0.0.0)
    /System/Library/Frameworks/AGL.framework/Versions/A/AGL (compatibility version 1.0.0, current version 1.0.0)
    /System/Library/Frameworks/OpenGL.framework/Versions/A/OpenGL (compatibility version 1.0.0, current version 1.0.0)
    /usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.5)
    /Users/pwolfram/miniconda2/conda-bld/work/lib/libCGAL_Triangulation_2_cpp.dylib (compatibility version 0.0.0, current version 0.0.0)
    /System/Library/Frameworks/Python.framework/Versions/2.7/Python (compatibility version 2.7.0, current version 2.7.5)
    /Users/pwolfram/miniconda2/conda-bld/work/lib/libCGAL_Kernel_cpp.dylib (compatibility version 0.0.0, current version 0.0.0)
    /usr/lib/libstdc++.6.dylib (compatibility version 7.0.0, current version 60.0.0)

(e.g., perhaps something similar to Scenario 2 from https://github.com/Homebrew/homebrew-science/issues/3401)

It is probably due to not linking to the brew python framework, if you are using that for your version of boost (see also https://github.com/boostorg/build/pull/78).

pwolfram commented 8 years ago

Summary

  1. Paths are not installed correctly in OS X. Potentially solution is modifying the build.sh to use @loader_path or @rpath instead of $ORIGIN (http://conda.pydata.org/docs/building/shared-libraries.html, https://cmake.org/Wiki/CMake_RPATH_handling)
  2. Shared object files, e.g., /Users/rusty/anaconda/envs/tom_test/lib/python2.7/site-packages/CGAL/_CGAL_Triangulation_2.so appear to link to system python framework /System/Library/Frameworks/Python.framework/Versions/2.7/Python that may be different than boost installation, e.g., /usr/local/lib/libboost_thread-mt.dylib, and result in Fatal Python error: PyThreadState_Get: no current thread errors. This appears to be due to linking to the system python framework, even though boost may have been built with a brew version of python (https://github.com/boostorg/build/pull/78). Essentially, the \*.so library cannot be linked to a python framework that is different than the other frameworks used to build the libraries linked to for CGAL and the python bindings. "The best solution is to provide -bundle -undefined dynamic_lookup, which has the same behavior as the Linux linker." (http://blog.tim-smith.us/2015/09/python-extension-modules-os-x/).
pwolfram commented 8 years ago

@rustychris, were you able to get a working copy of stomel on OSX, even with some hacking by hand? It seems like it should be possible.

pwolfram commented 8 years ago

Also, for future reference to demonstrate that this works well on linux, in general, with only minor modifications: see my fork at https://github.com/pwolfram/conda-recipes/tree/icc_mods which was used to build on institutional computing. Current anaconda channel packages for pwolfram are for this build (cc @vanroekel).

I tested it to make sure that tom_test.ipynb worked (no errors and good output), same was true for the new refine_existing_grid.ipynb.

rustychris commented 8 years ago

@pwolfram, I was able to eventually get a good compile on OSX, though have not yet been able to get the rpath stuff worked out. A few notes along the way:

Thanks for the patience - let's keep the conversation going and get this working!

xantares commented 8 years ago

@rustychris cgal is available from https://anaconda.org/conda-forge/cgal

pwolfram commented 8 years ago

@rustychris, thanks for sharing your notes! I suspect we may have all the information needed now and all that is left is to give this a couple more tries.

Regarding @xantares's comment, is the challenge of using CGAL 4.8.1 building the bindings using SWIG? I'm wondering if a bigger challenge here that would benefit everyone, including the larger computational geometry community, is production of generalized python bindings that are "officially supported" by the CGAL community. I recognize the challenge here but think it may be worth considering.

If I recall correctly, when we build the bindings used in stomel not all all of CGAL is available, just some key parts needed by core stomel algorithms, right? What I'm trying to get at is would it be possible to switch to something like https://github.com/CGAL/cgal-swig-bindings in stomel? The currently supported packages appear to be https://github.com/CGAL/cgal-swig-bindings/wiki/Package_wrappers_available but I don't see Appolonius, for instance.

If we were easily able to switch to the general bindings we may be able to effectively "work out" this issue with the installation because the respective communities are providing conda-compatible installations. This seems like the best long-term solution but there may be technical challenges I am not fully appreciating. For instance, is it possible @xantares to install the cgal-swig-bindings via conda, e.g., is https://github.com/CGAL/cgal-swig-bindings/issues/70 still unresolved or is there an unsupported work-around?

xantares commented 8 years ago

@pwolfram, yes, https://anaconda.org/conda-forge/cgal conda package does provide cgal and cgal-swig-bindings.

rustychris commented 8 years ago

Quick follow up - re 4.8.1: I think some changes are required on the swig bindings side in order to support 4.8.1, but apparently this has been worked out by others.

My apollonius fork is based off a dated version of CGAL/cgal-swig-bindings (from before the google code => github transfer). At the time, there was also a memory management bug in the swig bindings to the triangulation classes which I kludged a fix for.

Probably the thing to do is (i) make sure this memory bug is no longer an issue in the trunk cgal-swig-bindings, (ii) fork the current bindings and add in apollonius (and try to get a PR back to them), and (iii) replicate whatever black magic was required to get the conda build going, so we can test the results in stomel.

pwolfram commented 8 years ago

@rustychris and @xantares, this is a good plan. I just confirmed that the https://anaconda.org/conda-forge/cgal package has bindings out of the box, nothing fancy needed-- sweet!

https://anaconda.org/conda-forge/cgal bindings: screenshot 2016-08-09 10 50 52

previous bindings: screenshot 2016-08-09 10 48 25

Thus, as far as I can tell the missing entries are

Addition of these should give us the compatibility we need for OS X in general.

pwolfram commented 8 years ago

@xantares, once we have this PR in do we have to do anything else, e.g., does conda-forge auto-generate them?

pwolfram commented 8 years ago

Note, Surface_mesher depends on CGAL_ImageIO which is currently not compiled on conda-forge, but the capability appears to be there if needed, e.g., change the conda-feedstock recipe to not turn it off -DWITH_CGAL_ImageIO=ON at https://github.com/conda-forge/cgal-feedstock/blob/master/recipe/build.sh#L9. Hence, we only need