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 SCIP optimization suite #13520

Merged
merged 14 commits into from
Feb 10, 2021
Merged

Add SCIP optimization suite #13520

merged 14 commits into from
Feb 10, 2021

Conversation

AntoinePrv
Copy link
Contributor

@AntoinePrv AntoinePrv commented Dec 19, 2020

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml"
  • License file is packaged (see here for an example)
  • Source is from official source
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged)
  • If static libraries are linked in, the license of the static library is packaged.
  • Build number is 0
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details)
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there
  • When in trouble, please check our knowledge base documentation before pinging a team.

This PR brings the SCIP optimization suite to conda-forge.
Some context:

  • scip and soplex are package under the ZIB Academic (non commercial license) that is not listed in the SPDX License List. Perhaps a PR could be added there to add it.
  • bliss-automorphism is a fork, as a certain number of patches needed to be applied from the original source that seems to be unmaintained.
  • bliss-automorphism, soplex, papilo are typically build as static library. One reason for this is ease of packaging, but since dynamic libraries seem to be more appropriate for conda, I wonder if it may be worth building them as dynamic libraries. I do not know if it would negatively impact performance.
  • These libraries are available on Windows. However, I must admit that having no expertise in Windows, just getting started with conda-build, and a laptop that has troubles building this recipes in VM, I would very much prefer getting them merged without Windows in a first time.
  • Pinging (non exhaustively) @ambros-gleixner @mattmilten @fschloesser @pfetsch and @pokutta for comment, review, and for any volunteer for recipe co-maintainers.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/bliss-automorphism, recipes/papilo, recipes/pyscipopt, recipes/scip, recipes/soplex) and found some lint.

Here's what I've got...

For recipes/bliss-automorphism:

  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

Documentation on acceptable licenses can be found here.

For recipes/papilo:

  • The test section contained an unexpected subsection name. script is not a valid subsection name.

For recipes/papilo:

  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

Documentation on acceptable licenses can be found here.

For recipes/scip:

  • The recipe license should not include the word "License".
  • The test section contained an unexpected subsection name. script is not a valid subsection name.

For recipes/scip:

  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

Documentation on acceptable licenses can be found here.

For recipes/soplex:

  • The recipe license should not include the word "License".
  • The test section contained an unexpected subsection name. script is not a valid subsection name.

For recipes/soplex:

  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

Documentation on acceptable licenses can be found here.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/bliss-automorphism, recipes/papilo, recipes/pyscipopt, recipes/scip, recipes/soplex) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipes/scip:

  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

Documentation on acceptable licenses can be found here.

For recipes/soplex:

  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

Documentation on acceptable licenses can be found here.

@pfetsch
Copy link

pfetsch commented Dec 19, 2020

@AntoinePrv This is great!

With respect to static vs. shared linking:

  • SoPlex can be linked both shared and statically. When linking the shared version one needs to use -fPIC, which - as far as I know - causes a very minor performance degradation, since it adds indirect addressing for some part of the code/constant data. So, if a shared version has advantages, you can just use it.
  • Linking Papilo with SCIP does not need any libraries.
  • For bliss the same comment as for SoPlex hold. Default is to build the static library.

@AntoinePrv
Copy link
Contributor Author

@isuruf thank you for looking at this. How can I move this PR forward?

@AntoinePrv AntoinePrv mentioned this pull request Jan 14, 2021
3 tasks
@fschloesser
Copy link

Hey, what is the status here, can we merge this? While figuring out the future of bliss for now we want to keep the packages in this pull request as the bliss-automorphism.

@isuruf
Copy link
Member

isuruf commented Jan 22, 2021

We are not going to merge this while bliss-automorphism clearly conflicts with bliss. If bliss-automorphism renames the header and library name to not conflict with bliss, then we can merge it.

@AntoinePrv
Copy link
Contributor Author

That's a good point, I did not though of the headers and libraries clash when suggesting a second package.
We're getting synchronized with the current repository to merge all our changes, and provide a well maintained fork mkoeppe/bliss#3.

In the meantime, I still believe vendoring Bliss inside SCIP is a good way to go. This is a private dependency for SCIP, so if we
build it statically we should leave no trace of it. I can add a test that no Bliss files are included when building the recipe. Would that work?

@isuruf
Copy link
Member

isuruf commented Jan 22, 2021

Sure. Remember to package the license of bliss in SCIP when you do that.

@conda-forge-linter
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/pyscipopt, recipes/scipoptsuite) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipes/scipoptsuite:

  • It looks like the 'scip' output doesn't have any tests.
  • It looks like the 'soplex' output doesn't have any tests.
  • License is not an SPDX identifier (or a custom LicenseRef) nor an SPDX license expression.

Documentation on acceptable licenses can be found here.

@AntoinePrv
Copy link
Contributor Author

@isuruf I've made some major change to the scip recipe, using the single/grouped scipoptsuite release and then splitting it into different packages.
With regard to the top level requirements and outputs requirements, I'm having a little trouble understanding what will be available at compile time. Could you please provide some feedback as to whether it looks sound in the recipe?

I added the Bliss fork privately, but not test since with the outputs section all files are named.

@AntoinePrv AntoinePrv requested a review from isuruf February 9, 2021 16:13
@AntoinePrv AntoinePrv requested a review from isuruf February 10, 2021 15:35
@fschloesser
Copy link

Please add me as maintainer for scipoptsuite, pyscipopt

@isuruf isuruf merged commit aa394bb into conda-forge:master Feb 10, 2021
@AntoinePrv
Copy link
Contributor Author

Thanks a lot @isuruf !

@pokutta
Copy link

pokutta commented Feb 11, 2021

Please add me as maintainer for [scipoptsuite] [pyscipopt]

@AntoinePrv AntoinePrv deleted the scipopt branch February 11, 2021 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants