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

Issue building or-tools with Python support on OpenBSD #4249

Closed
ron-at-swgy opened this issue May 30, 2024 · 18 comments · Fixed by #4266
Closed

Issue building or-tools with Python support on OpenBSD #4249

ron-at-swgy opened this issue May 30, 2024 · 18 comments · Fixed by #4266

Comments

@ron-at-swgy
Copy link
Contributor

ron-at-swgy commented May 30, 2024

What version of OR-Tools and what language are you using?
Version: Latest (git clone of stable branch)
Language: Attempting to build Python bindings

Which solver are you using (e.g. CP-SAT, Routing Solver, GLOP, BOP, Gurobi)
N/A

What operating system (Linux, Windows, ...) and version?
OpenBSD-current

What did you do?
Steps to reproduce the behavior:

  1. $ git clone --depth=1 https://github.com/google/or-tools.git
  2. # pkg_add lzlib abseil-cpp protobuf eigen3 re2
  3. Build COIN-OR dependencies:
$ for dep in CoinUtils Osi Clp Cgl Cbc
do git clone --depth=1 https://github.com/coin-or/$dep $dep
done

$ for dep in CoinUtils Osi Clp Cgi
do cd $dep ; ./configure -C --prefix=/home/myuser/coin --exec-prefix=/home/myuser/coin ;
make && make install
clean
cd ..
done

The Cbc repository includes a compilation error on BSD systems that must be corrected. Replace the reinterpret_cast at src/CbcModel.cpp line ~6255 with static_cast. This is due to the way NULL is defined on BSD systems. See this commit for more information.

Then:

$ ./configure -C --prefix=/home/myuser/coin --exec-prefix=/home/myuser/coin ;
make && make install
clean
  1. In your installation virtual environment, pip install pybind11.
  2. Determine the path to pybind11's cmake files (this can be done easily by starting python with the verbose flag and importing pybind11).
  3. Add the following line to or-tools/CMakeLists.txt to point to pybind11's CMake files:
set(pybind11_DIR /home/myuser/venv/lib/python3.11/site-packages/pybind11/share/cmake/pybind11)
find_package(pybind11 REQUIRED)
  1. Locate pybind11_protobuf project here.
  2. Search and search and search for installation instructions or a guide on pointing the or-tools CMake configuration to a cloned copy of pybind11_protobuf
  3. Configure LD_LIBRARY_PATH to include the installed COIN-OR libraries as well as any BLAS on your system (I use libopenblas):
        export LD_LIBRARY_PATH=/home/myuser/coin/lib:/home/myuser/libopenblas/lib
  1. Repeatedly try turning flags on and off while building or-tools:
        cmake -S. -Bbuild \
            -DBUILD_PYTHON=ON \
            -DUSE_SCIP=OFF \
            -DBUILD_DEPS=OFF

What did you expect to see

A successful build producing a python package suitable for use on platforms for which no "wheel" is prebuilt on pypi.org.

What did you see instead?

CMake Error at cmake/deps.cmake:169 (find_package):
  By not providing "Findpybind11_protobuf.cmake" in CMAKE_MODULE_PATH this
  project has asked CMake to find a package configuration file provided by
  "pybind11_protobuf", but CMake did not find one.

  Could not find a package configuration file provided by "pybind11_protobuf"
  with any of the following names:

    pybind11_protobufConfig.cmake
    pybind11_protobuf-config.cmake

  Add the installation prefix of "pybind11_protobuf" to CMAKE_PREFIX_PATH or
  set "pybind11_protobuf_DIR" to a directory containing one of the above
  files.  If "pybind11_protobuf" provides a separate development package or
  SDK, be sure it has been installed.
Call Stack (most recent call first):
  CMakeLists.txt:435 (include)

Anything else we should know about your project / environment

For operating systems other than macOS, GNU/Linux, and some Windows flavors, there are no prebuilt wheels for the python or-tools library. This means pip install or-tools fails to find a candidate:

$ pip install or-tools
ERROR: Could not find a version that satisfies the requirement or-tools (from versions: none)
ERROR: No matching distribution found for or-tools

As such, one must resort to building the library from source. Luckily several dependencies are prebuilt for my platform (see the pkg_add step above) and the COIN-OR projects are easier to build than their documentation portrays. My current roadblock is this pybind11_protobuf piece. It is admittedly outside my area of expertise, so without documentation detailing installation or integration procedures, I am left with a guess-test-and-revise strategy.

@lperron
Copy link
Collaborator

lperron commented May 31, 2024

small comment, you can disable CBC with -DUSE_COINOR=OFF

@lperron lperron changed the title Issue building or-tools with Python support Issue building or-tools with Python support on Open-BSD May 31, 2024
@Mizux
Copy link
Collaborator

Mizux commented Jun 3, 2024

my 2 cents,

  1. to build a local ortools python package you need to use our cmake based build (no python wheel generation support when using bazel based build)

  2. OR-Tools rely on pybind11 and pybind11_protobuf to generate its python wrapper (i.e. we "just" wrap the libortools C++ using pybind11 than reimplementing/porting ortools to python)

  3. The pybind11_protobuf cmake support is borken and do not provide any config.cmake or pkg file yet so the idiomatic find_package(pybind11_protobuf) can't currently work with pybind11-protobuf src (even worse pybind11_protobuf do not provide any release yet)
    ref: https://github.com/pybind/pybind11_protobuf

  4. pybind11_protobuf rely on python/google/protobuf/proto_api.h which is an header file not available in the Protobuf cmake based build (not available if you FetchContent protobuf nor if you build it, install it, then consume it) -> you need to patch Protobuf src to make this header available (ed in bazel there is the @com_google_protobuf//:proto_api target...)

  5. You could let or-tools build a protobuf and pybind11_protobuf for you using -DBUILD_Protobuf=ON and -DBUILD_pybind11_protobuf=ON (and -DBUILD_pybind11)

  6. You can see the patch we apply in https://github.com/google/or-tools/tree/main/patches
    and the CMake orchestration is here: https://github.com/google/or-tools/blob/main/cmake/dependencies/CMakeLists.txt#L155-L208

see: https://github.com/google/or-tools/blob/main/cmake/README.md#cmake-options

note: I have this pet project to test pybind11_protobouf integration (Bazel and CMake support)
https://github.com/Mizux/bazel-pybind11-protobuf very usefull to fast iterate, find bug have a minimal reproducing integration example etc......

@ron-at-swgy ron-at-swgy changed the title Issue building or-tools with Python support on Open-BSD Issue building or-tools with Python support on OpenBSD Jun 4, 2024
@ron-at-swgy
Copy link
Contributor Author

Thank you @Mizux for the recommendation. I was able to get to CMake reporting 32% using the following steps:

        # pkg_add swig
        $ export LD_LIBRARY_PATH=/home/ro071072/coin/lib:/home/ro071072/libopenblas/lib:/usr/lib:/usr/local/lib
        (venv) $ pip install wheel mypy protobuf mypy_protobuf virtualenv
        $ cmake -S. -Bbuild -DBUILD_PYTHON=ON -DBUILD_DEPS=ON
        $ cmake --build build

The build emits a few warnings along the way but eventually fails with the following:

/home/user/or-tools/ortools/util/fp_utils.h:95:11: error: no member named '__control_word' in 'fenv_t'
    fenv_.__control_word &= ~excepts;
    ~~~~~ ^

For more context, here is the last 265 lines of build output -> https://gist.github.com/ron-at-swgy/1905e788030f0c53be1a16d588678d3f

@ron-at-swgy
Copy link
Contributor Author

@lperron I was able to get the minor fix needed for COIN-OR's Cbc component (see coin-or/Cbc#653)

@Mizux
Copy link
Collaborator

Mizux commented Jun 4, 2024

void EnableExceptions(int excepts) {
#if defined(_MSC_VER)
// _controlfp(static_cast<unsigned int>(excepts), _MCW_EM);
#elif (defined(__GNUC__) || defined(__llvm__)) && defined(__x86_64__) && \
!defined(__ANDROID__)
CHECK_EQ(0, fegetenv(&fenv_));
excepts &= FE_ALL_EXCEPT;
#if defined(__APPLE__)
fenv_.__control &= ~excepts;
#elif defined(__FreeBSD__)
fenv_.__x87.__control &= ~excepts;
#else // Linux
fenv_.__control_word &= ~excepts;
#endif
fenv_.__mxcsr &= ~(excepts << 7);
CHECK_EQ(0, fesetenv(&fenv_));
#endif
}

We have a hack for FreeBSD (line 93) maybe we need to add an OpenBSD #define... (PR welcome ;) )

ref: https://sourceforge.net/p/predef/wiki/OperatingSystems/

@ron-at-swgy
Copy link
Contributor Author

I am running a build locally with the addition of __OpenBSD__ to the FreeBSD check. I verified the fenv structure is the same on OpenBSD (at least for amd64).

You can see the github mirror here or the official CVSWeb source here about 2/3rds of the way down.

It seems a safe change - I'll prepare a pull request.

@ron-at-swgy
Copy link
Contributor Author

I've added OpenBSD as a companion to the FreeBSD conditionals found throughout the codebase. I also added it as a classifier for the python package that gets generated. I was able to progress much further in the build before failing with swig errors. I am using swig 4.2.1.

I have included the end of the build log in a gist here -> https://gist.github.com/ron-at-swgy/a1862262f163c6d472c4156965baeac7

@Mizux
Copy link
Collaborator

Mizux commented Jun 4, 2024

Random guess, could be this lines:

if(UNIX AND NOT APPLE)
if (CMAKE_SIZEOF_VOID_P EQUAL 8)
list(APPEND CMAKE_SWIG_FLAGS "-DSWIGWORDSIZE64")
else()
list(APPEND CMAKE_SWIG_FLAGS "-DSWIGWORDSIZE32")
endif()
endif()

out of curiosity did you see the -DSWIGWORDSIZE64 on the command line for the constraint solver swig target ?

try something like this (not tested yet) to rerurn the swig generation target + build:

touch ortools/constraint_solver/python/routing.i
cmake --build build --target pywrapcp -v

@ron-at-swgy
Copy link
Contributor Author

ron-at-swgy commented Jun 4, 2024

I searched through the logs and did not find a reference to SWIG anywhere. I also tried changing the conditional you linked to instead always use -DSWIGWORDSIZE64. The resulting build had the same error as above - it is trying to use a long when an int64_t is expected. If you provide me with a hint about what to configure or change, I will be happy to do so - and provide a PR!

I have installed SWIG using my system's package manager - would that have any impact? I have no experience with SWIG and cannot help.

Thank you

@ron-at-swgy
Copy link
Contributor Author

On my system, all of these types are 64 bits long:

#include <unistd.h>
#include <stdio.h>

int
main(int argc, char **argv)
{
        printf("sizeof(long) = %d\n", sizeof(long));
        printf("sizeof(int64_t = %d\n", sizeof(int64_t));
        printf("sizeof(unsigned long long) = %d\n", sizeof(unsigned long long));
        printf("sizeof(uint64_t) = %d\n", sizeof(uint64_t));
        return 0;
}

and the output:

sizeof(long) = 8
sizeof(int64_t = 8
sizeof(unsigned long long) = 8
sizeof(uint64_t) = 8

I'm following @Mizux from 2020 across stack overflow and the SWIG project 😆

I'm going to try doing something with PRIMITIVE_TYPEMAP as is done for Java in ortools/base/base.i

@Mizux
Copy link
Collaborator

Mizux commented Jun 5, 2024

Few tests/question needed:
0. did you still use the stable branch ? (note: need to check if we add some swig change on main contrary to stable)

  1. Which compiler did you use ?

  2. Could you provide the information, to know if int64_t is bind to long or long long by your compiler ?
    please take a look a this link below should be something like this <compiler> -dM -E -x c /dev/null | grep INT64
    ref: https://github.com/google/or-tools/blob/main/cmake/docs/swig.md#int64_t-management

AFAIK, on your system int64_t seems to be bind to long long thus the vector<long> to vector<int64_t> compile issue.

note: SWIG 4+ should be OK so it is not an issue with SWIG but more with how we call the swig compiler i.e. with or without the -DSWIGWORDSIZE64 (in your case should be without so int64_t is bind to long long like on macos)

Will try to gen locally the python binding of routing.i on stable to do some tests and compare with you...

@ron-at-swgy
Copy link
Contributor Author

ron-at-swgy commented Jun 5, 2024

  1. I am building with origin/stable
  2. Clang:
host$ cc --version
OpenBSD clang version 16.0.6
Target: amd64-unknown-openbsd7.5
Thread model: posix
InstalledDir: /usr/bin
  1. Output of the requested command:
host$ cc -dM -E -x c /dev/null | grep INT64
#define __INT64_C_SUFFIX__ LL
#define __INT64_FMTd__ "lld"
#define __INT64_FMTi__ "lli"
#define __INT64_MAX__ 9223372036854775807LL
#define __INT64_TYPE__ long long int
#define __UINT64_C_SUFFIX__ ULL
#define __UINT64_FMTX__ "llX"
#define __UINT64_FMTo__ "llo"
#define __UINT64_FMTu__ "llu"
#define __UINT64_FMTx__ "llx"
#define __UINT64_MAX__ 18446744073709551615ULL
#define __UINT64_TYPE__ long long unsigned int

Regarding the vector<long> and vector<int64_t> issue - is this something that could be handled conditionally in the SWIG *.i files?

@Mizux
Copy link
Collaborator

Mizux commented Jun 5, 2024

  1. Thx, so INT64_TYPE is bind to long long int like on osx so -DSWIGWORDSIZE64 must be not present on your command line (so swig keep its conservative x86 style to bind int64_t to long long)

AFAIK it can only controlled by the swig cli argument -DSWIGWORDSIZE64 or its absence... (we can control through CMAKE in our python.cmake source file)

in your trace we can see

/home/user/or-tools/build/python/ortools/constraint_solver/routingPYTHON_wrap.cxx:4976:45: note: candidate function not viable: no known conversion from 'const vector<long>' to 'const vector<int64_t>' for 2nd argument
SWIGINTERN operations_research::Constraint *operations_research_IntExpr_Member(operations_research::IntExpr *self,std::vector< int64_t > const &values){

Would it be possible to share the code of this generated source file build/python/ortools/constraint_solver/routingPYTHON_wrap.cxx around this lines ? (so I can compare with mine locally)

@ron-at-swgy
Copy link
Contributor Author

Yes - I have just started a build with the latest stable. I will post it to a gist and share here.

@ron-at-swgy
Copy link
Contributor Author

The generated routingPYTHON_wrap.cxx file can be found here - it's rather large so I don't think a gist is appropriate.

@ron-at-swgy
Copy link
Contributor Author

Exciting news - I was able to get a successful build!

In cmake/python.cmake, I removed the block that conditionally sets -DSWIGWORDSIZENN, then set -DSMALL_LONG.

This is not going to work for the project, so we will need to conditionally set it based on conditions Cmake can check. How can I check for OpenBSD with cmake?

@ron-at-swgy
Copy link
Contributor Author

Installing collected packages: immutabledict, absl-py, ortools
Successfully installed absl-py-2.1.0 immutabledict-4.2.0 ortools-9.10.9999

@Mizux
Copy link
Collaborator

Mizux commented Jun 7, 2024

You can try to use CMAKE_SYSTEM_NAME
ref: https://cmake.org/cmake/help/latest/variable/CMAKE_SYSTEM_NAME.html#variable:CMAKE_SYSTEM_NAME

to test in CMake:

message(STATUS "system name: ${CMAKE_SYSTEM_NAME}")

if(CMAKE_SYSTEM_NAME STREQUAL "OpenBSD")
  message(STATUS "On OpenBSD")
  list(APPEND CMAKE_SWIG_FLAGS "-DSMALL_LONG")
else()
  message(STATUS "NOT on OpenBSD")
endif()

message(FATAL_ERROR "stop cmake configure to fast iterate ! (to remove after dev/debug)")

note: In CMake 3.25 we could use BSD directly but we currently try to target older CMake (3.18 or 3.19 IIRC)
ref: https://cmake.org/cmake/help/latest/variable/BSD.html

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

Successfully merging a pull request may close this issue.

3 participants