-
Notifications
You must be signed in to change notification settings - Fork 69
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
Unit tests fail on OS X 10.9 (Mavericks) with CBC 2.8.8 #11
Comments
If it helps, using the pre-installed Python or MacPorts installed Python the results are that the CyLP build fails
|
Thanks for posting this issue. It's an inconvenience but the problem with isspace can be easily (but may be not so gracefully) solved:
$ find %s -name "*.cpp" -print | xargs sed -i "" 's/(isspace/(std::isspace/g' which is the same command used by the custom install. About the seg fault, your workaround is really nice! Thanks for sharing it. It gives me an idea to ask the maintainer of coin-or (brew) formulas to see if it's possible to use the necessary flags by default. In that case, to install CyLP on a mac you'll only need to "brew install cbc". You may have actually solved another thing that was bugging me. I recently released binaries of CyLP on pypi for major platforms, with CBC statically linked. This way no separate installation of CBC is required. On ubuntu and windows there is no problem. But on mac (especially 10.9) I was clueless. I'll try your suggestion and if it works with static libraries it'll make life a lot easier for Maverick users. Thanks again! |
Ooops! I just got confused there. Correction: Setup.py will run the custom install only if CYLP_USE_CYTHON is NOT set. I'm sorry for that. |
FYI, I was able to get CyLP to build and compile with the stock Python which comes with Mavericks as well as the one installed by MacPorts without having to add any special flags when configuring and compiling Cbc. All that was required was to run the following line in the CyLP source directory, slightly different that what is in the setup.py file since there are now isspace functions which do not have a leading (, make sure to only run this once or else the std:: prefix starts to recurse.
All the unit tests pass, everything is built with clang/clang++ and linked against libc++. I believe this doesn't work for the Anaconda Python since the interpreter and various libraries in that distribution are build with gcc/g++ and linked against libstdc++. This issues with Anaconda can also be solved by building Cbc and CyLP in using the |
Thanks! Good to know. I was just playing with a similar environment (Mavericks, Python that comes with it, no extra arguments for CBC) and CyLP seems to build fine and all the tests pass. Also the custom install ran ok and did the replacements. I wonder why this wasn't the case for you (that you had to run the patch from the command line). Btw, linking CyLP against static CBC libraries still fails with a "segmentation fault 11". I understand that you provide a CyLP binary for anaconda: does it require CBC libraries at run-time? |
The advantage of the setup patch is that it will not recurse upon multiple executions. |
When I tried to build CyLP using the custom install in setup.py the isspace functions which were not preceded by a ( were not patched and caused a compiler error (for example line 33840 of CyClpSimplex.cpp). Maybe these lines look different when generated with different versions of Cython? In Anaconda I build CBC as shared libraries. When building CyLP it is necessary to change the path to the dynamic shared libraries in the .so files to point to correct CBC libraries. The conda build command takes care of this behind the scenes using the install_name_tool utility that comes with OS X. You might be able to do the same in your case. |
install_name_tool can set relative paths to dynamic libraries. For the pip release you could put the CBC shared libraries in the cylp directory and adjust the cylp .so file to point there. |
"When I tried to build CyLP using the custom install in setup.py the isspace functions which were not preceded by a ( were not patched and caused a compiler error (for example line 33840 of CyClpSimplex.cpp). Maybe these lines look different when generated with different versions of Cython?" Possibly. In the context, "isspace" is only preceded either by "(" or " ". I would guess sometimes the " " isn't a space and possibly a "tab"(?!). If only sed supported negative lookbehind... "In Anaconda I build CBC as shared libraries. When building CyLP it is necessary to change the path to the dynamic shared libraries in the .so files to point to correct CBC libraries. The conda build command takes care of this behind the scenes using the install_name_tool utility that comes with OS X. You might be able to do the same in your case." I see. Anaconda seems like a convenient environment to work in. " This is exactly what I was looking for!! Thanks for the tip. I'll give it a try. Many thanks! |
Actually I was missing the second find/sed line in the custom install part of setup.py which is why I was running into the isspace issue. I really should have just let setup.py take care of this. With the latest updates I can build CyLP against CBC 2.8.8 without issue on Mac OS X 10.9 for use with the MacPort's Python and Anaconda Python. I'm building the extension in-place using Feel free to close out this issue, thanks for the help and the great module! |
"Actually I was missing the second find/sed line in the custom install part of setup.py which is why I was running into the isspace issue. I really should have just let setup.py take care of this." Oh I see. Btw I thought the patch could become problematic later on, so I changed it. It does the same thing but a tad cleaner. "With the latest updates I can build CyLP against CBC 2.8.8 without issue on Mac OS X 10.9 for use with the MacPort's Python and Anaconda Python." That's great! Thanks to your suggestion about install_name_tool, I added a bit of code that fixes the library paths in the libraries. I uploaded fresh binaries to pypi for 10.6, 10.7, 10.9 (CyLP 0.7.1) so to install CyLP you could just say "easy_install cylp". No installation of Cbc is required. "I'm building the extension in-place using I see. I don't know how that's possible but there has to be a way. I'll investigate. Thanks again for your feedback and suggestions! |
On a machine running Mac OS X 10.9 (Mavericks), using Python 2.7.6 provided by Anaconda 1.8.0.
CBC version 2.8.8 was built using the defaults:
CyLP builds fine but the unit tests fail as follows:
Trying to use the CyClpSimplex object in Python code from the above build leads to either a malloc error as in above, or a segmentation fault.
Doing some debugging it looks like the CBC libraries are linked to libc++ (The clang C++ standard library that is the default in 10.9), where as the CyLP shared libraries are linked against libstdc++ (The GCC library). Unfortunately these two libraries are not ABI compatible which is what I think is leading to the memory errors.
I tried to force CyLP to link against the libc++ libraries but ran into issue
This seems to be the problem that PR #8 tries to address but I never was able to get the customInstall class to run the fixes.
The work-around I have been using for the time is add the necessary flags when building CBC so that the libraries link against libstdc++.
When built with the above, CyLP passes all tests, but oddly the CBC libraries still are linked to libc++ when checked with otool.
I did not run into this issue on OS X 10.8, but no longer have access to a machine running that version to test on.
The text was updated successfully, but these errors were encountered: