Various improvements to the HiGHS interface (#418)

* highs: fix var_get_index for error case

* extend tests for var_by_name

* extend test_objective: add & rm terms

Before, only coefficients were changed. This now fails on HiGHS, which doesn't
properly reset the objective before setting a new one.

* fix SolverHighs.set_objective

* highs: add comments about confusion between BINARY and INTEGER

* add test_verbose

* fix SolverHighs._{get,set}_bool_option_value

Used incorrect types before.

* skip tests with HiGHS based on .gz files

* Return -1 if constraint not found

* substitute highsbox for highspy

* check status on Highs_getColIntegrality (fail some tests)

* work around unexpected Highs_getColIntegrality error

* upgrade python for doing the math fork to python3.13

* highs.py: formatting with black

* pre-commit: update flake8 version to avoid error

---------

Co-authored-by: BernardZweers <bernard@doingthemath.nl>
Co-authored-by: Miguel Hisojo <miguel.hisojo@alliander.com>

Author: 3 people

.github/workflows/github-ci.yml

@@ -15,7 +15,7 @@ jobs:
       - name: Set up Python
         uses: actions/setup-python@v5
         with:
-          python-version: 3.11
+          python-version: 3.13
 
       - name: Upgrade pip
         run: python -m pip install --upgrade pip
@@ -57,7 +57,15 @@ jobs:
       with:
         python-version: ${{ matrix.python-version }}
         architecture: x64
-        cache: 'pip'
+
+    - name: Cache pip dependencies
+      uses: actions/cache@v4
+      with:
+        path: ~/.cache/pip
+        key: ${{ runner.os }}-pip-${{ matrix.python-version }}-${{ hashFiles('**/pyproject.toml') }}
+        restore-keys: |
+          ${{ runner.os }}-pip-${{ matrix.python-version }}-
+          ${{ runner.os }}-pip-
 
     - name: Check python version
       run: python -c "import sys; import platform; print('Python %s implementation %s on %s' % (sys.version, platform.python_implementation(), sys.platform))"

.pre-commit-config.yaml

@@ -7,7 +7,7 @@ repos:
     - id: black
       args: [--line-length=89]
 -   repo: https://github.com/pycqa/flake8
-    rev: 4.0.1
+    rev: 7.3.0
     hooks:
     - id: flake8
       args: [--select=F401, --exclude=__init__.py]

mip/highs.py

@@ -1,6 +1,5 @@
 "Python-MIP interface to the HiGHS solver."
 
-import glob
 import numbers
 import logging
 import os.path
@@ -22,30 +21,23 @@
         libfile = os.environ[ENV_KEY]
         logger.debug("Choosing HiGHS library {libfile} via {ENV_KEY}.")
     else:
-        # try library shipped with highspy packaged
-        import highspy
+        # Try library from highsbox, which is optional dependency.
+        import highsbox
 
-        pkg_path = os.path.dirname(highspy.__file__)
+        root = highsbox.highs_dist_dir()
 
-        # need library matching operating system
+        # Need library matching operating system.
+        # Following: PyOptInterface/src/pyoptinterface/_src/highs.py
         platform = sys.platform.lower()
         if "linux" in platform:
-            patterns = ["highs_bindings.*.so", "_core.*.so"]
+            libfile = os.path.join(root, "lib", "libhighs.so")
         elif platform.startswith("win"):
-            patterns = ["highs_bindings.*.pyd", "_core.*.pyd"]
+            libfile = os.path.join(root, "bin", "highs.dll")
         elif any(platform.startswith(p) for p in ("darwin", "macos")):
-            patterns = ["highs_bindings.*.so", "_core.*.so"]
+            libfile = os.path.join(root, "lib", "libhighs.dylib")
         else:
             raise NotImplementedError(f"{sys.platform} not supported!")
-
-        # there should only be one match
-        matched_files = []
-        for pattern in patterns:
-            matched_files.extend(glob.glob(os.path.join(pkg_path, pattern)))
-        if len(matched_files) != 1:
-            raise FileNotFoundError(f"Could not find HiGHS library in {pkg_path}.")
-        [libfile] = matched_files
-        logger.debug("Choosing HiGHS library {libfile} via highspy package.")
+        logger.debug("Choosing HiGHS library {libfile} via highsbox package.")
 
     highslib = ffi.dlopen(libfile)
     has_highs = True
@@ -687,6 +679,7 @@
 def check(status):
     if status == STATUS_ERROR:
         raise mip.InterfacingError("Unknown error in call to HiGHS.")
+    return status
 
 
 class SolverHighs(mip.Solver):
@@ -720,7 +713,7 @@ def __init__(self, model: mip.Model, name: str, sense: str):
         # Buffer string for storing names
         self._name_buffer = ffi.new(f"char[{self._lib.kHighsMaximumStringLength}]")
 
-        # type conversion maps
+        # type conversion maps (can not distinguish binary from integer!)
         self._var_type_map = {
             mip.CONTINUOUS: self._lib.kHighsVarTypeContinuous,
             mip.BINARY: self._lib.kHighsVarTypeInteger,
@@ -760,8 +753,8 @@ def _get_double_option_value(self: "SolverHighs", name: str) -> float:
         )
         return value[0]
 
-    def _get_bool_option_value(self: "SolverHighs", name: str) -> float:
-        value = ffi.new("bool*")
+    def _get_bool_option_value(self: "SolverHighs", name: str) -> int:
+        value = ffi.new("int*")
         check(
             self._lib.Highs_getBoolOptionValue(self._model, name.encode("UTF-8"), value)
         )
@@ -779,7 +772,7 @@ def _set_double_option_value(self: "SolverHighs", name: str, value: float):
             )
         )
 
-    def _set_bool_option_value(self: "SolverHighs", name: str, value: float):
+    def _set_bool_option_value(self: "SolverHighs", name: str, value: int):
         check(
             self._lib.Highs_setBoolOptionValue(self._model, name.encode("UTF-8"), value)
         )
@@ -815,6 +808,8 @@ def add_var(
         if name:
             check(self._lib.Highs_passColName(self._model, col, name.encode("utf-8")))
         if var_type != mip.CONTINUOUS:
+            # Note that HiGHS doesn't distinguish binary and integer variables
+            # by type. There is only a boolean flag for "integrality".
             self._num_int_vars += 1
             check(
                 self._lib.Highs_changeColIntegrality(
@@ -1035,6 +1030,18 @@ def set_start(self: "SolverHighs", start: List[Tuple["mip.Var", numbers.Real]]):
         self._lib.Highs_setSolution(self._model, cval, ffi.NULL, ffi.NULL, ffi.NULL)
 
     def set_objective(self: "SolverHighs", lin_expr: "mip.LinExpr", sense: str = ""):
+        # first reset old objective (all 0)
+        n = self.num_cols()
+        costs = ffi.new("double[]", n)  # initialized to 0
+        check(
+            self._lib.Highs_changeColsCostByRange(
+                self._model,
+                0,  # from_col
+                n - 1,  # to_col
+                costs,
+            )
+        )
+
         # set coefficients
         for var, coef in lin_expr.expr.items():
             check(self._lib.Highs_changeColCost(self._model, var.idx, coef))
@@ -1323,7 +1330,11 @@ def remove_constrs(self: "SolverHighs", constrsList: List[int]):
 
     def constr_get_index(self: "SolverHighs", name: str) -> int:
         idx = ffi.new("int *")
-        self._lib.Highs_getRowByName(self._model, name.encode("utf-8"), idx)
+        status = self._lib.Highs_getRowByName(self._model, name.encode("utf-8"), idx)
+        if status == STATUS_ERROR:
+            # This means that no constraint with that name was found. Unfortunately,
+            # Highs: getRowByName doesn't assign a value to idx in that case.
+            return -1
         return idx[0]
 
     # Variable-related getters/setters
@@ -1422,12 +1433,15 @@ def var_set_obj(self: "SolverHighs", var: "mip.Var", value: numbers.Real):
         check(self._lib.Highs_changeColCost(self._model, var.idx, value))
 
     def var_get_var_type(self: "SolverHighs", var: "mip.Var") -> str:
+        # Highs_getColIntegrality only works if some variable is not continuous.
+        # Since we want this method to always work, we need to catch this case first.
+        if self._num_int_vars == 0:
+            return mip.CONTINUOUS
+
         var_type = ffi.new("int*")
-        ret = self._lib.Highs_getColIntegrality(self._model, var.idx, var_type)
+        check(self._lib.Highs_getColIntegrality(self._model, var.idx, var_type))
         if var_type[0] not in self._highs_type_map:
-            raise ValueError(
-                f"Invalid variable type returned by HiGHS: {var_type[0]} (ret={ret})"
-            )
+            raise ValueError(f"Invalid variable type returned by HiGHS: {var_type[0]}.")
         return self._highs_type_map[var_type[0]]
 
     def var_set_var_type(self: "SolverHighs", var: "mip.Var", value: str):
@@ -1518,7 +1532,11 @@ def remove_vars(self: "SolverHighs", varsList: List[int]):
 
     def var_get_index(self: "SolverHighs", name: str) -> int:
         idx = ffi.new("int *")
-        self._lib.Highs_getColByName(self._model, name.encode("utf-8"), idx)
+        status = self._lib.Highs_getColByName(self._model, name.encode("utf-8"), idx)
+        if status == STATUS_ERROR:
+            # This means that no var with that name was found. Unfortunately,
+            # HiGHS::getColByName doesn't assign a value to idx in that case.
+            return -1
         return idx[0]
 
     def get_problem_name(self: "SolverHighs") -> str:

pyproject.toml

@@ -22,7 +22,7 @@ classifiers = [
     "Development Status :: 5 - Production/Stable",
     "License :: OSI Approved :: Eclipse Public License 2.0 (EPL-2.0)",
     "Operating System :: OS Independent",
-    "Programming Language :: Python :: 3",
+    "Programming Language :: Python :: 3.13",
     "Programming Language :: Python :: Implementation :: CPython",
     "Programming Language :: Python :: Implementation :: PyPy",
     "Topic :: Scientific/Engineering :: Mathematics"
@@ -38,7 +38,7 @@ numpy = [
     "numpy==1.21.*; python_version=='3.7'"
 ]
 gurobi = ["gurobipy>=8"]
-highs = ["highspy>=1.5.3; python_version<='3.11'"]
+highs = ["highsbox>=1.10.0"]
 test = [
     "pytest>=7.4",
     "networkx==2.8.8; python_version>='3.8'",

test/mip_files_test.py

@@ -139,6 +139,9 @@ def test_mip_file(solver: str, instance: str):
 
     max_dif = max(max(abs(ub), abs(lb)) * 0.01, TOL)
 
+    if solver == HIGHS and instance.endswith(".gz"):
+        pytest.skip("HiGHS does not support .gz files.")
+
     m.read(instance)
     if bas_file:
        m.verbose = True

test/mip_test.py

@@ -556,11 +556,21 @@ def test_constr_by_name_rhs(self, solver):
         assert model.constr_by_name("row0").rhs == val
 
     @pytest.mark.parametrize("solver", SOLVERS)
-    def test_var_by_name_rhs(self, solver):
+    def test_var_by_name_valid(self, solver):
         n, model = self.build_model(solver)
 
-        v = model.var_by_name("x({},{})".format(0, 0))
+        name = "x({},{})".format(0, 0)
+        v = model.var_by_name(name)
         assert v is not None
+        assert isinstance(v, mip.Var)
+        assert v.name == name
+
+    @pytest.mark.parametrize("solver", SOLVERS)
+    def test_var_by_name_invalid(self, solver):
+        n, model = self.build_model(solver)
+
+        v = model.var_by_name("xyz_invalid_name")
+        assert v is None
 
     @pytest.mark.parametrize("solver", SOLVERS)
     def test_obj_const1(self, solver: str):

test/test_model.py

@@ -1294,6 +1294,21 @@ def test_copy(solver):
     assert id(term.expr) != id(term_copy.expr)
 
 
+@skip_on(NotImplementedError)
+@pytest.mark.parametrize("solver", SOLVERS)
+def test_verbose(solver):
+    # set and get verbose flag
+    m = Model(solver_name=solver)
+
+    # active
+    m.verbose = 1
+    assert m.verbose == 1
+
+    # inactive
+    m.verbose = 0
+    assert m.verbose == 0
+
+
 @skip_on(NotImplementedError)
 @pytest.mark.parametrize("solver", SOLVERS)
 def test_constraint_with_lin_expr_and_lin_expr(solver):
@@ -1356,6 +1371,7 @@ def test_objective(solver):
     m = Model(solver_name=solver, sense=MAXIMIZE)
     x = m.add_var(name="x", lb=0, ub=1)
     y = m.add_var(name="y", lb=0, ub=1)
+    z = m.add_var(name="z", lb=0, ub=1)
 
     m.objective = x - y + 0.5
     assert m.objective.x is None
@@ -1374,13 +1390,13 @@ def test_objective(solver):
 
 
     # Test changing the objective
-    m.objective = x + y + 1.5
+    m.objective = y + 2*z + 1.5
     m.sense = MINIMIZE
     # TODO: assert m.objective.sense == MINIMIZE
 
     assert len(m.objective.expr) == 2
-    assert m.objective.expr[x] == 1
     assert m.objective.expr[y] == 1
+    assert m.objective.expr[z] == 2
     assert m.objective.const == 1.5
 
     status = m.optimize()