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 highs support in MindtPy #2971

Merged
merged 19 commits into from
May 7, 2024
Merged

Conversation

ZedongPeng
Copy link
Contributor

Fixes #2951 and #2874.

Since the Highs team has fixed the column integrality bug ERGO-Code/HiGHS#1386, mindtpy can now support appsi_highs as the mip_solver.

Summary/Motivation:

  1. Add the support of appsi_highs in MindtPy.
  2. Change the mip_solver from glpk to appsi_highs in MindtPy tests.

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.

@ZedongPeng
Copy link
Contributor Author

I think the appsi_highs needs to be updated to the latest version for MindtPy tests to pass.

@ZedongPeng
Copy link
Contributor Author

Let's wait for the next release (1.5.4) of Highs.
https://github.com/ERGO-Code/HiGHS/releases

@blnicho blnicho marked this pull request as draft September 12, 2023 18:52
@blnicho blnicho changed the title Add highs support in MindtPy [WIP] Add highs support in MindtPy Sep 12, 2023
@ZedongPeng
Copy link
Contributor Author

Highs 1.60 was just released. @blnicho

https://github.com/ERGO-Code/HiGHS/releases/tag/v1.6.0

@blnicho
Copy link
Member

blnicho commented Nov 13, 2023

This is waiting on highspy 1.6.0 to be released on PyPI. Relevant issue: ERGO-Code/HiGHS#925 and discussion: ERGO-Code/HiGHS#1441

@ZedongPeng
Copy link
Contributor Author

Highspy 1.7.1 is available.
https://pypi.org/project/highspy/1.7.1.dev1/

@ZedongPeng ZedongPeng marked this pull request as ready for review April 16, 2024 18:58
@ZedongPeng
Copy link
Contributor Author

Hi @blnicho. The tests failed after I changed the highs version check from ==1.7.1 to >=1.7.1 in the GitHub workflow. Any suggestions to fix this?

@jsiirola
Copy link
Member

jsiirola commented Apr 17, 2024

You need to put in quotes (otherwise the shell interprets the > as output redirection):

- $PYTHON_EXE -m pip install --cache-dir cache/pip highspy>=1.7.1.dev1 \
+ $PYTHON_EXE -m pip install --cache-dir cache/pip "highspy>=1.7.1.dev1" \

@blnicho blnicho changed the title [WIP] Add highs support in MindtPy Add highs support in MindtPy Apr 23, 2024
@blnicho blnicho linked an issue Apr 23, 2024 that may be closed by this pull request
Copy link
Member

@blnicho blnicho left a comment

Choose a reason for hiding this comment

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

@ZedongPeng I made a minor change in how the APPSI Highs interface gets the solver version. Hopefully this will resolve the Jenkins test failures. Assuming everything passes I think this is finally ready to be merged. Thanks for your patience!

@blnicho
Copy link
Member

blnicho commented May 6, 2024

@jsiirola @mrmundt @emma58 could one of you be the second reviewer on this?

Copy link

codecov bot commented May 7, 2024

Codecov Report

Attention: Patch coverage is 69.56522% with 7 lines in your changes are missing coverage. Please review.

Project coverage is 88.47%. Comparing base (e77be8c) to head (c610a20).

Files Patch % Lines
pyomo/contrib/mindtpy/algorithm_base_class.py 61.11% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2971      +/-   ##
==========================================
+ Coverage   88.45%   88.47%   +0.01%     
==========================================
  Files         850      850              
  Lines       95525    95544      +19     
==========================================
+ Hits        84500    84531      +31     
+ Misses      11025    11013      -12     
Flag Coverage Δ
linux 86.41% <56.52%> (-0.01%) ⬇️
osx ?
other 86.61% <56.52%> (+<0.01%) ⬆️
win 83.91% <56.52%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

Comment on lines +615 to +616
echo "NOTE: temporarily pinning to highspy pre-release for testing"
$PYTHON_EXE -m pip install --cache-dir cache/pip "highspy>=1.7.1.dev1" \
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we remove 'temporarily' if this is how we're leaving it?

Copy link
Member

Choose a reason for hiding this comment

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

I think we should leave it for now. When they cut a new (non-dev) highspy release we can remove the version requirement.

@ZedongPeng
Copy link
Contributor Author

Thank you all! @blnicho @emma58 @mrmundt @jsiirola

@blnicho blnicho merged commit 6e08021 into Pyomo:main May 7, 2024
31 of 33 checks passed
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.

Feasibility pump still fails when using appsi_highs in MindtPy Support more milp solver in mindtpy
6 participants