google / or-tools

Google's Operations Research tools:
https://developers.google.com/optimization/
Apache License 2.0
11.24k stars 2.13k forks source link

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

Closed ron-at-swgy closed 4 months ago

ron-at-swgy commented 5 months ago

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](https://github.com/coin-or/Cbc/commit/b127c88b9f53e36197d1542f2e65fec3ebb2cb04) for more information. 

Then:

```sh
$ ./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)
  4. Locate pybind11_protobuf project here.

  5. 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

  6. 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
11. Repeatedly try turning flags on and off while building or-tools:
```sh
        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 commented 5 months ago

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

Mizux commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

@lperron I was able to get the minor fix needed for COIN-OR's Cbc component (see https://github.com/coin-or/Cbc/pull/653)

Mizux commented 5 months ago

https://github.com/google/or-tools/blob/5b87d86172a8089db9b84db464d70069b6f3f999/ortools/util/fp_utils.h#L84-L101

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 commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

Random guess, could be this lines: https://github.com/google/or-tools/blob/5b87d86172a8089db9b84db464d70069b6f3f999/cmake/python.cmake#L34-L40

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 commented 5 months ago

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 commented 5 months ago

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 :laughing:

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

Mizux commented 5 months ago

Few tests/question needed:

  1. did you still use the stable branch ? (note: need to check if we add some swig change on main contrary to stable)

  2. Which compiler did you use ?

  3. 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 commented 5 months ago
  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
  3. 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 commented 5 months ago
  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 commented 5 months ago

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

ron-at-swgy commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago
Installing collected packages: immutabledict, absl-py, ortools
Successfully installed absl-py-2.1.0 immutabledict-4.2.0 ortools-9.10.9999
Mizux commented 5 months ago

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