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

feat: discover dynamically linked backends at runtime #69

Open
wants to merge 20 commits into
base: main
Choose a base branch
from

Conversation

tlambert03
Copy link
Member

@tlambert03 tlambert03 commented Dec 15, 2024

This PR converts the backends to shared libraries that are dynamically linked and discovered at runtime, rather than at compile time. This will make it easier to distribute (won't require that gurobi is present to use scip for example)

summary of changes:

  • moves to src layout (makes it harder to "accidentally" import the ilpy directory in the root folder during development without having installed and built the package)
  • support gurobi 11.0.3 specifically (looks like gurobi ≥12 no longer actually ships a dll on conda forge, but rather embeds it into gurobipy directly. that's an issue for us)
  • lots of changes to SolverFactory.cpp, which now does all the work of dynamically loading a backend wrapper, and calling it's createSolverBackend function
  • in order to make it a bit easier to relocate, I also have createSolverBackend require a directory in which the dlls should be found, so that that can be more easily done on the python side at runtime. might be a better way to do this ...

Copy link

codecov bot commented Dec 15, 2024

Codecov Report

Attention: Patch coverage is 63.76812% with 25 lines in your changes missing coverage. Please review.

Project coverage is 59.37%. Comparing base (0fdfae7) to head (acaa34e).
Report is 13 commits behind head on main.

Files with missing lines Patch % Lines
src/ilpy/impl/solvers/SolverFactory.cpp 75.60% 3 Missing and 7 partials ⚠️
src/ilpy/impl/solvers/BackendPreference.h 50.00% 4 Missing ⚠️
src/ilpy/impl/solvers/GurobiBackend.h 0.00% 4 Missing ⚠️
src/ilpy/wrapper.pyx 50.00% 3 Missing ⚠️
src/ilpy/impl/solvers/ScipBackend.h 0.00% 2 Missing ⚠️
src/ilpy/impl/solvers/GurobiBackend.cpp 75.00% 1 Missing ⚠️
src/ilpy/impl/solvers/ScipBackend.cpp 75.00% 1 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (0fdfae7) and HEAD (acaa34e). Click for more details.

HEAD has 5 uploads less than BASE
Flag BASE (0fdfae7) HEAD (acaa34e)
12 7
Additional details and impacted files
@@             Coverage Diff             @@
##             main      #69       +/-   ##
===========================================
- Coverage   75.72%   59.37%   -16.35%     
===========================================
  Files          17       21        +4     
  Lines         828     1253      +425     
  Branches      154      341      +187     
===========================================
+ Hits          627      744      +117     
- Misses        136      477      +341     
+ Partials       65       32       -33     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tlambert03
Copy link
Member Author

@cmalinmayor and @funkey ... how would you guys feel about not supporting windows 😂

i swear to god... i usually get something working pretty quickly for macos and linux ... (and this PR would be a step towards allowing this all to ship on conda forge). but then it takes forever to try various things to get windows working, and it still doesn't work. I'm honestly not entirely sure that gurobi ever worked on windows? @cmalinmayor, do you have any sense for how many windows users you have? Any definitely important users or use cases?

@cmalinmayor
Copy link

@AnniekStok Do you have any windows users in the lab? Yes, right?

@tlambert03
Copy link
Member Author

After chatting with Jan yesterday, we've got a dramatically new possible strategy, which is to statically link scip, and use gurobipy (instead of our c code here). That's a significant rewrite, but would make it much easier to distribute (and build).

So, windows users have no fear :)

@AnniekStok
Copy link

We are mostly MacOS users, but the shared workstations and virtual desktop system are (mostly) running on Windows. So we would be very happy with Windows support if possible :)

@tlambert03 tlambert03 mentioned this pull request Dec 19, 2024
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 this pull request may close these issues.

3 participants