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 qp support for highs #3531

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

Conversation

quantresearch1
Copy link
Contributor

Fixes #3381 .

Summary/Motivation:

HiGHS can solve quadratic programming (QP) models, which contain an objective term x.T @ Q @ x where the Hessian matrix Q is positive semi-definite.

Changes proposed in this PR:

  • Allow quadratic terms in the objective by changing quadratic to True in generate_standard_repn

Legal Acknowledgement

By contributing to this software project, I have read the contribution guide and agree to the following terms and conditions for my contribution:

  1. I agree my contributions are submitted under the BSD license.
  2. I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

Copy link
Contributor

@emma58 emma58 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@quantresearch1, thank you for the contribution! As written, this does not yet add QP support for Highs, though: By changing the quadratic flag to True in generate_standard_repn, you move the nonlinear term in your objective from repn.nonlinear_expr to repn.quadratic_vars and repn.quadratic_coefs (and hence bypass the error in line 622 of appsi/solvers/highs.py) . To properly send quadratic expressions to Highs, you will need to parse this part of the StandardRepn object and send the result to Highs.

(Your test appears to get lucky--it is actually sending a constant objective to Highs, but the variables happen to land on the values you are expecting. You can see this by adding an assertion that the results object returned by opt.solve(m) agrees with your expected objective value: self.assertEqual(results.best_feasible_objective, 2).)

@quantresearch1
Copy link
Contributor Author

@emma58 Thanks for the advice. Looking into the Highs C++ code, I did not find a quadratic equivalent to changeColCost so I think each coefficient update would have to be a call to passHessian which is not ideal in a persistent setting (assuming many updates). I will think about this a bit more and reraise if I can come up with a decent solution

raise unittest.SkipTest


class TestBugs(unittest.TestCase):
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mrmundt I have copied the test cases that are passing from the legacy appsi_highs. I am not sure if you wanted to leave the test cases for a later stage, let me know if I should remove them (I do need the new qp test cases though)


self._solver_model.changeObjectiveSense(sense)
self._solver_model.changeColsCost(n, indices, costs)
self._mutable_objective = _MutableObjective(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the idea here is we collect all the mutable objective terms and update them once through _mutable_objective.update()

self.col_idx = col_idx


class _MutableObjective:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@emma58 I have tried to emulate the new gurobi_persistent and use a similar objective data structure. However, highs quadratic objective handling is too different so I could not make it fit as nicely as I would have hoped. To avoid always calling passHessian at each update I first check against the last set of coefficients, let me know if you think there's a better way

@quantresearch1
Copy link
Contributor Author

I need to look into the highs unit tests failure, they look genuine

@blnicho blnicho requested a review from michaelbynum March 25, 2025 19:13
@quantresearch1
Copy link
Contributor Author

quantresearch1 commented Mar 27, 2025

I need to look into the highs unit tests failure, they look genuine

I have had a look at the remaining failures and I think it's unrelated to my PR:

  • osx
    INFO: The following extensions were downloaded:
    [FAIL] gjh
    [ OK ] mcpp
  • linux
    Message: 'Significant delay observed waiting to join reader threads, possible output stream deadlock'
    Arguments: ()
    Significant delay observed waiting to join reader threads, possible output stream deadlock
    ------- generated xml file: /home/runner/work/pyomo/pyomo/TEST-pyomo.xml -------
    =========================== short test summary info ============================
    FAILED pyomo/contrib/solver/tests/solvers/test_solvers.py::TestSolvers::test_domain_0_gurobi - RuntimeError: TeeStream: deadlock observed joining reader threads

@jsiirola
Copy link
Member

I need to look into the highs unit tests failure, they look genuine

I have had a look at the remaining failures and I think it's unrelated to my PR:

I believe that those two test failures will be resolved by #3537.

@quantresearch1 quantresearch1 requested a review from emma58 March 28, 2025 00:04
@quantresearch1
Copy link
Contributor Author

@jsiirola Thanks, I have synced with main and that has helped with a few failures. I am just left with this one failure for python 3.13 on linux. Not clear to me if this branch is the culprit but will try to install linux over the weekend and debug. Let me know if you think it's unrelated to the branch
self = <pyomo.common.tee.capture_output object at 0x7f900cbbd6d0>, et = None
ev = None, tb = None

def __exit__(self, et, ev, tb):
    # Check that we were nested correctly
    FAIL = []
    if self.tee.STDOUT is not sys.stdout:
        FAIL.append('Captured output does not match sys.stdout.')
    if self.tee.STDERR is not sys.stderr:
        FAIL.append('Captured output does not match sys.stderr.')
    # Exit all context managers.  This includes
    #  - Restore any file descriptors we commandeered
    #  - Close / join the TeeStream
    #  - Close any opened files
    FAIL.extend(self._exit_context_stack(et, ev, tb))
    sys.stdout, sys.stderr = self.old
    self.old = None
    self.tee = None
    self.output_stream = None
    if FAIL:
      raise RuntimeError("\n".join(FAIL))

E RuntimeError: TeeStream: deadlock observed joining reader threads

pyomo/common/tee.py:380: RuntimeError
----------------------------- Captured stderr call -----------------------------
Significant delay observed waiting to join reader threads, possible output stream deadlock
TeeStream: deadlock observed joining reader threads
------- generated xml file: /home/runner/work/pyomo/pyomo/TEST-pyomo.xml -------
=========================== short test summary info ============================
FAILED pyomo/contrib/solver/tests/solvers/test_solvers.py::TestSolvers::test_equality_4_highs - RuntimeError: TeeStream: deadlock observed joining reader threads

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.

Cannot solve quadratic problem with Highs solver
3 participants