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

Add google/or-tools #2717

Closed
pmlandwehr opened this issue Apr 1, 2017 · 22 comments · Fixed by #16147
Closed

Add google/or-tools #2717

pmlandwehr opened this issue Apr 1, 2017 · 22 comments · Fixed by #16147

Comments

@pmlandwehr
Copy link
Contributor

Google's OR-Tools are maintained by Google but I'm not sure how easily they play with conda, so we might want to add `em.

@sodre
Copy link
Member

sodre commented Apr 27, 2017

@pmlandwehr you can assign it to me.

sodre added a commit to sodre/staged-recipes that referenced this issue Apr 27, 2017
@jGaboardi
Copy link
Member

@sodre Has there been any more progress on this? I was looking this issue earlier when a friend pointed me to this thread.

@sodre
Copy link
Member

sodre commented Aug 14, 2018

@jGaboardi, I haven't made any more progress on this since 2017. I was trying to cover all the architectures at that time and failed to complete the macOS and Windows builds.

If you are only interested in Linux, I think it is possible for us to rebase the current branch and only enable the build for Linux. Let me know if you're interested on that.

@jGaboardi
Copy link
Member

jGaboardi commented Aug 14, 2018

@sodre thanks for the super quick reply! And yes, I am very interested. The current OS I need a conda build of ortools for is RHEL 6.10, so this would fit my needs perfectly. Much good karma to you if we can get it up on the conda-forge channel.

EDIT
I have just been informed by the main person in charge of approving software install requests that only conda-forge packages from the following two channels mirrored by the US Census Bureau (which is the environment I am working in for this specific project):

@sodre
Copy link
Member

sodre commented Aug 18, 2018

Okay, I'll continue the work this weekend.

@jGaboardi
Copy link
Member

@sodre Also, I am working in python 2.7 due to a legacy code base.

Although I have never built conda package for or done a conda-forge release, I am happy to help you with this in any way that I am able.

@jGaboardi
Copy link
Member

@sodre I wondering if anything more has been done in regards to getting the Linux build up on conda-forge? Please don't think I am trying to be pushy, I am in a time-sensitive situation for a particular project that I need to either be able to ask the person in charge of software to do conda install -c conda-forge ortools (easy option) or go to a committee for approval of pip install ortools (difficult option) . If this is still a work-in-progress, is there any timeline when it would be available? Either way, I want to reiterate that I am not trying to be a demanding jerk and really appreciate the work you are doing. Regardless of whether the conda-forge build will be completed for me to use for this particular project, I do believe the community would greatly benefit from ortools being available through conda.

sodre added a commit to sodre/staged-recipes that referenced this issue Aug 26, 2018
@lperron
Copy link

lperron commented Sep 7, 2018

Just to comment, as the creator of or-tools, thanks and tell me if you need anything.

@astrojuanlu
Copy link
Member

Left a comment in google/or-tools#92 (comment) about repackaging the official wheels instead of building from source. Perhaps it's a good first step before trying to compile from source. In any case, ortools supports make builds without Bazel, as far as I understand.

@cdeil
Copy link
Contributor

cdeil commented Apr 9, 2020

I'm also very interested in this addition.

@sodre - What's the next step to make this happen?

Is sodre@5b61e96 a good starting point for a pull request?

If the build fails on some platforms for now, maybe just activate that one platform, and leave the others for later? Something is better than nothing, no?

Thanks for your work on conda packaging!

@wolfv
Copy link
Member

wolfv commented Apr 9, 2020

I have a working conda build locally. The first step would be to split the existing packages for the coin tools here: https://github.com/conda-forge/coincbc-feedstock (issue is open here: conda-forge/coincbc-feedstock#37)

I can upload the recipes later...

@wolfv
Copy link
Member

wolfv commented Apr 9, 2020

@lperron the cmake of ortools is quite opinionated -- could you add hooks to also use pkgconfig to find dependencies such as protobuf or the coin libraries?

@lperron
Copy link

lperron commented Apr 9, 2020 via email

@wolfv
Copy link
Member

wolfv commented Apr 10, 2020

@cdeil do you want to take over these recipes: https://github.com/wolfv/staged-recipes/tree/add_coin_or/recipes

It has working recipes for or-tools (with tests) at least on linux :)

The remaining issue is that we should probably use pgkconfig / autotools to build the coin dependencies and need to patch or-tools cmake to find the packages using pkgconfig (as we don't want to statically link the dependencies, except for Abseil where we have to as or-tools is using an "unreleased" version (abseil doesn't really seem to have releases)).

The second part of the previous point is that the coincbc package in conda-forge already vendors a bunch of libraries that will collide with the splitted libraries. I talked with @scopatz and he's fine with splitting although we haven't figured out how to do it.

The first part would be to re-open a PR here and then we need to make the existing coincbc a meta-package to the new, correct one.

@cdeil
Copy link
Contributor

cdeil commented Apr 10, 2020

@wolfv - I'm just a user that tried out OR-Tools and other packages yesterday for the first time (see python-discrete-optimization-examples). I don't feel qualified to maintain the conda recipes for these large and complex packages. If you think it's useful to add me as a co-maintainer for simple tasks such as e.g. reviewing version update PRs, that's fine.

@tkralphs or anyone from the COIN project - would you be willing to co-maintain conda-forge recipes for the COIN codes or review these changes? (following up on #10935 (comment))

@clemolgat-SBR - Would you or someone from OR-Tools be willing to co-maintain the conda-forge recipe for OR-Tools or review these changes?

@wolfv - I looked a bit at your COIN and OR-tools packaging proposal at: https://github.com/wolfv/staged-recipes/tree/add_coin_or/recipes

Two thoughts:

How did you come up with these version numbers for the COIN packages for OR-Tools
(see here)?
On the other hand within the COIN packages it seems you have no version constraints (see here)?
Is this a good way to maintain versions of dependencies, or will this cause problems in the future?
I see that for Homebrew all the COIN packages are maintained in one git repo by @tkralphs and at https://github.com/Mizux/homebrew-or-tools/blob/master/or-tools.rb @Mizux doesn't have COIN versions pinned for OR tools at all.

Why did you patch to CXX_STANDARD 17 for OR-Tools? Shouldn't it work for CXX_STANDARD 11 and that would be more conservative? (see here

@wolfv
Copy link
Member

wolfv commented Apr 10, 2020

Hi @cdeil, thanks for pinging some more people.

The version numbers of the coin projects come from their releases on the relevant github projects.

google or-tools does not work with the latest release of coin-clp but works with the version just before (there is an issue filed by @Mizux on coin-clp.
The version constraints on the or-tools package are derived from the or-tools cmake (https://github.com/google/or-tools/blob/stable/cmake/dependencies/CMakeLists.txt)
We can assume some level of binary compatibility between releases.

The default CXX standard on conda-forge is C++17 so I had to patch this because of linker and include errors -- the default on conda-forge is C++17 so if you link multiple shared libraries that can lead to inconsistencies for example with std::string_view which exists in C++17 but not in C++11. I faced these problems at least with either protobuf or abseil which has it's own implementation of std::string_view if compiled with C++11 and otherwise uses the C++17 string_view. The issue is that header & library are not having the same interface anymore if compiled with different standards.

On the other hand the coin libraries do not support C++17 and had to be explicitly compiled with C++11 :) but the code is anyways old-school so the aforementioned issues don't play a role.

Sure, I can add you as a maintainer. I'll try to work out a plan with @scopatz to get the split packages into conda-forge.

@Mizux
Copy link

Mizux commented Apr 10, 2020

Hi, I'm currently the maintainer of the python packaging process.

please notice pep 513 doesn't allow a python package to depends on system libraries, see the manylinux policy...
see: https://www.python.org/dev/peps/pep-0513/#the-manylinux1-policy

thus when building the python package we try to embed all required dependencies (as static library if possible)

We can discuss on this OR-Tools discord: https://discord.gg/ENkQrdf

@wolfv
Copy link
Member

wolfv commented Apr 10, 2020

the conda package manager is thousand times nicer than PEP 513, it's cross-platform and language agnostic so we can (and should!) link to shared libraries as if it was a deb package :) We're not bound by the Python packaging process here.

@tkralphs
Copy link

For the version numbers for the COIN-OR projects, one is a release and the others are stables. If you really want to depend on a tagged version, they should all be releases. In general, there is a Dependencies file that is packaged with each version that says what the dependencies are. This is what coinbrew uses to figure out what versions to fetch if you want to build a particular project. Right now, the Cbc stack is still maintained in SVN (and mirrored to Github), so the URLs are actually for the SVN repo. We'll be moving to git permanently relatively soon. It would probably be good to use Dependencies file somehow here. Anyway, I would be willing to help maintain these conda packages.

Keep in mind that there are about 20 packages in the overall COIN-OR Optimization Suite, all of which I would eventually like to have packages for most/all of these. I'm not sure if the OR-Tools can interface to any of the others, but there is at least the potential to do so in the future, so that should be part of the roadmap.

@BastianZim BastianZim mentioned this issue Sep 11, 2021
9 tasks
@BastianZim
Copy link
Member

Hi everybody, as an FYI, I have created a new PR based on @wolfv's work which you can find in #16147

Two questions for the or-tools maintainer, if you're happy to have a look:

  1. When using -BUILD_DEPS=OFFthe builds fail whereas deselecting them individually work. Am I doing something incorrectly here? Detailed logs are in the PR.
  2. Do you think it would be possible to allow building the python, java etc. wrappers without building or-tools itself? That would allow us to build or-tools and then build the wrappers separately with them then depending on or-tools.

@Mizux
Copy link

Mizux commented Sep 14, 2021

  1. Trying help you directly in the issue -> nothing to add here.

  2. For Python according various PEP, PEP 600 and the PEP 513 manylinux Policy a python package can not depends on external deps except glibc etc...
    So it is currently a non goal for us (and we lack of human resource) to have a python package without the native library libortools.so in it.
    If you still want it the roadmap should be:

and few others things I'm missing ;)

  1. For Java and .Net, I try to follow the same convention aka try to depend on nothing except the java JRE/JDK / .Net SDK and glibc.
    Also from my experience lots of user don't know/understand why a platform agnostic language running in a VM like C# and Java (or Python and its interpreter) can be platform dependent i.e. what's a native package. I think it is still a "niche" topic IMHO. (e.g. how to explain to a Java dev user that it needs to install few packages on its macOS/windows when, to begin, it doesn't know what is a package manager or homebrew or vcpkg, or why its windows local stuff doesn't work on its linux powered azure server)
    note: Will be happy to find some equivalent to my pet projects but officially provided/supported by Oracle, Microsoft or the Python Foundation.
    https://github.com/Mizux/python-native
    https://github.com/Mizux/dotnet-native
    https://github.com/Mizux/java-native

@BastianZim
Copy link
Member

BastianZim commented Sep 14, 2021

  1. Thanks again!
  2. Ok yeah seems like quite the undertaking, then it's probably simpler to just build the package a couple of times.
    And agreed, for the average user, it should probably just work.
    The reason I asked is because we would just substitute the packages being built with the ones we built previously. For the end-user, it would still be a single installation but it would pull in the tools from conda-forge and not build them again. So instead of building or-tools 4 times (one standalone and three wrappers), we would only build or-tools once and then add it to the wrappers as a dependency. But this is primarily about saving CI time so it's not super important.
    What we will probably though have to disable is
BUILD_DEPS OFF* Default to ON if BUILD_JAVA=ON or BUILD_PYTHON=ON or BUILD_DOTNET=ON

at https://github.com/google/or-tools/tree/stable/cmake#cmake-options as we are supplying the dependencies of the wrapper builds (ZLIB etc) ourselves, like we do with the original build.

Edit: I should probably clarify one big difference between pip and conda which makes this possible: Conda allows packages to depend also on other packages, written in other languages. So a python package can depend on a java, c++, etc. package and conda will install all of them and not just ask for them to be available. It's basically a cross-language dependency solver.
This is the difference that allows us to have a python wrapper that can pull in a C++ project and probably makes this non-standard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.