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

OSI interface needs updating #525

Closed
yurivict opened this issue Jun 17, 2021 · 22 comments
Closed

OSI interface needs updating #525

yurivict opened this issue Jun 17, 2021 · 22 comments
Assignees

Comments

@yurivict
Copy link
Contributor

tests unit_tests_all and osi_unit_tests_all fail: the osi_unit_tests executable crashes:

Program received signal SIGSEGV, Segmentation fault.
0x000000080027a6b2 in OsiHiGHSSolverInterface::OsiHiGHSSolverInterface (this=0x7fffffffe5d0) at /disk-samsung/freebsd-ports/math/highs/work/HiGHS-cfe064e/src/interfaces/OsiHiGHSSolverInterfa
ce.cpp:67
(gdb) bt
[0x000000080027a6b2 in OsiHiGHSSolverInterface::OsiHiGHSSolverInterface (this=0x7fffffffe5d0) at /disk-samsung/freebsd-ports/math/highs/work/HiGHS-cfe064e/src/interfaces/OsiHiGHSSo
lverInterface.cpp:67
#1  0x00000000002098a0 in main (argc=1, argv=0x7fffffffe7e8) at /disk-samsung/freebsd-ports/math/highs/work/HiGHS-cfe064e/check/TestOsi.cpp:54

this->highs in OsiHiGHSSolverInterface::OsiHiGHSSolverInterface is nullptr.

rev. cfe064e

@jajhall
Copy link
Sponsor Member

jajhall commented Jun 18, 2021

Sorry about this. We don't have the resources to maintain the OSI interface as we work towards our first release, and we should say so in our documentation. Once the first release is out, we'll restore the OSI interface, as we know that it's valuable for some users.

@odow
Copy link
Collaborator

odow commented Feb 2, 2022

Should this issue be closed or renamed to something like "OSI interface needs updating"?

@jajhall
Copy link
Sponsor Member

jajhall commented Feb 2, 2022

Yes, it was precisely this issue that I knew existed, but couldn't find yesterday.

@jajhall jajhall changed the title 2 tests fail on FreeBSD OSI interface needs updating Feb 2, 2022
@jajhall jajhall self-assigned this Feb 2, 2022
@kkofler
Copy link

kkofler commented Feb 14, 2022

The OSI interface definitely needs adapting to commit 0e81db0: all the status query methods are still the stubs left by that commit, they need to work with the model status (as in the other files in that commit) instead.

@jajhall
Copy link
Sponsor Member

jajhall commented Feb 14, 2022

Sure. Now that we're close to completing the patches to v1.2 resurrecting the OSI interface moves up the list of priorities. We need it to enable people to switch from Cbc to HiGHS

@mkoeppe
Copy link

mkoeppe commented Apr 1, 2022

Do you also plan to add implementations of the tableau access functions (https://github.com/coin-or/Osi/blob/master/src/Osi/OsiSolverInterface.hpp#L1985)?

@jajhall
Copy link
Sponsor Member

jajhall commented Apr 1, 2022

Do you also plan to add implementations of the tableau access functions (https://github.com/coin-or/Osi/blob/master/src/Osi/OsiSolverInterface.hpp#L1985)?

Yes, should be pretty trivial since they exist in the HiGHS class. But first we have to repair the OSI interface build, and we've got other priorities over the next couple of months.

@mkoeppe
Copy link

mkoeppe commented Apr 1, 2022

Do you have a pointer where I find these? My actual goal is to get a pivot-level interface in Python, preferably going through the Cython interface to HiGHS in scipy, for SageMath (https://trac.sagemath.org/ticket/32282)

@jajhall
Copy link
Sponsor Member

jajhall commented Apr 1, 2022

Access using C++ is enabled via Highs::getBasicVariables, Highs::getReducedRow and Highs::getReducedColumn.

Thanks to @michaelbynum we have most of a new Python API to HiGHS in

https://github.com/ERGO-Code/HiGHS/tree/highspy

but the tableau access functions are amongst the missing methods. By pattern-matching we can add them. This will happen before we fix the OSI interface

@mkoeppe
Copy link

mkoeppe commented Apr 1, 2022

Awesome, thanks!

@mkoeppe
Copy link

mkoeppe commented Apr 1, 2022

Quick comment on highspy: Consider adding a pyproject.toml file to declare the build-time dependencies on pybind11 and pyomo

@jajhall
Copy link
Sponsor Member

jajhall commented Apr 1, 2022

Quick comment on highspy: Consider adding a pyproject.toml file to declare the build-time dependencies on pybind11 and pyomo

Means nothing to me, but I'm sure that @michaelbynum can interpret this!

@michaelbynum
Copy link
Contributor

Great suggestion @mkoeppe. I'll add a pyproject.toml shortly. I also need to list NumPy as a dependency...

@yurivict
Copy link
Contributor Author

yurivict commented Feb 5, 2023

I think it has been updated?

@jajhall
Copy link
Sponsor Member

jajhall commented Feb 5, 2023

No, it's still broken. No-one asks for it, so we've not updated it. Just call HiGHS directly, Clp and Cbc are history ;-)

@jajhall
Copy link
Sponsor Member

jajhall commented Feb 7, 2023

HiGHS no longer supports an OSI interface, and we have no plans to do so in the future

@jajhall jajhall closed this as completed Feb 7, 2023
@kkofler
Copy link

kkofler commented Feb 7, 2023

Well, "no-one asks for it" is not strictly true, I did. ;-)

But I can live without OSI support in HiGHS. I think the only thing it is really mandatory for is to use HiGHS for LPs in Cbc, but Cbc will likely still perform poorly even if you swap out Clp under it. (As far as I can tell, Bonmin is not a valid use case for HiGHS OSI because it uses low-level Cbc interfaces in addition to OSI ones, so you cannot just swap the MILP solver under it. It never claimed to support any MILP solver other than Cbc.)

But what I would really need if I were to use HiGHS at my work place (other than as an LP solver for SCIP MILPs) would be working Java bindings. OSI would have been one way to get those done, adding HiGHS to SWIMP. But SWIMP is old unmaintained stuff, so we can use a better binding that I do not have to maintain myself. The problem is, there is currently none for Java. I guess the recommended approach would be to go via the C binding and to just port, e.g., the C# binding from P/Invoke to Java JNA. But the issue I see is that the C binding is also very incomplete, there are lots of things you can only do through the native C++ interface. So I think it might make sense to use SWIG to bind the C++ classes directly in an object-oriented way. (Well, internally, it also goes through C, but the non-OO glue layer is all automatically generated, you see only the OO layer closely matching the native C++ one.)

@jajhall
Copy link
Sponsor Member

jajhall commented Feb 7, 2023

I'm curious to know what's missing in the C API, as I've tended to add to it as the C++ class is extended. Perhaps you don't just mean methods.

@kkofler
Copy link

kkofler commented Feb 7, 2023

It has been a while, so I do not remember what exactly I was missing. It is possible that it was just me missing the obvious and not finding an API that was in fact there. Or it is possible that I was missing something that I found in some internal C++ class and not wrapped in the Highs class (which, I suppose, means it should be added to the Highs C++ class first of all if we want it to be public?). It is good to know that the C API is supposed to be as complete as the C++ one. So basing any Java binding on that is probably the best approach, for consistency with the other bindings.

I will let you know if and when I find something (still) missing in the C API.

@kkofler
Copy link

kkofler commented Feb 7, 2023

Wait, there is one thing I remember already: There is no way in the C API or through the Highs class to set a custom message handler callback. This is actually possible through the OSI interface. But apparently not part of the public C++ API, if the Highs class is supposed to be the sole API class.

This is something that is really hard to handle in a C (or C-style C++) interface. The easiest way is really to do what OSI does and declare an abstract class for the message handler, which can then be implemented in the application. With SWIG cross-language polymorphism (SWIG directors), it can even be implemented in the target language, reversing the usual binding flow.

For SCIP, when I implemented message handlers in JSCIP, I actually changed JSCIP to C++ mode in order to be able to wrap the C++ classes for the callbacks (using SWIG directors).

@kkofler
Copy link

kkofler commented Feb 7, 2023

(For reference, see scipopt/JSCIPOpt#34 (merged) for how I implemented custom message handlers in JSCIP. JSCIP is MIT-licensed, so it should be safe for you to look at the code if you are curious.)

@jajhall
Copy link
Sponsor Member

jajhall commented Feb 7, 2023

There are methods in the Highs class that are only for internal use (in the MIP solver - which uses sub-instances of the Highs class)

As for the issue of call-backs, this is well up the ToDo list. Thanks for the pointers

svigerske added a commit to coin-or/Osi that referenced this issue Aug 13, 2023
- could add this again when it is has become usable
- #180, ERGO-Code/HiGHS#525
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

No branches or pull requests

6 participants