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

Significant enhancements and stability for highspy #1942

Merged
merged 5 commits into from
Sep 26, 2024

Conversation

mathgeekcoder
Copy link
Contributor

Major highspy update:

  • changed highs_linear_expression to be immutable by default
  • improved callback support
  • improved test coverage (98%)
  • performance and usability enhancements
  • Support __iadd__, __imul__, etc.
  • Updated chain comparison support in immutable setting
  • h.val() can take highs_linear_expression
  • expr == [lb,ub] -> lb <= expr <= ub syntax
  • qsum
  • added pretty print __repr__ and __str__
  • added KeyboardInterrupt support
  • added user interrupt
  • fixed slicing issues with numpy and highs
  • added resetGlobalScheduler
  • released GIL for Presolve
  • fixed issues with deadlock on Windows
  • fixed MIP solution callback issue
  • support getExpr that creates a highs_linear_expression from existing row
  • fixed all issues with static analysis (pyright)
  • improved typing and overloads
  • added new HighspyArray numpy subclass for improved usability

Replaces #1891
Should address multiple issues: #1865, #1882, #1888, #1892, #1903, #1904, and perhaps #1905

Major highspy update:
* changed `highs_linear_expression` to be immutable by default
* improved callback support
* improved test coverage (99%)
* performance and usability enhancements
* Support `__iadd__`, `__imul__`, etc.
* Updated chain comparison support in immutable setting
* `h.val()` can take `highs_linear_expression`
* `expr == [lb,ub]` -> `lb <= expr <= ub` syntax
* `qsum`
* added pretty print `__repr__` and `__str__`
* added KeyboardInterrupt support
* added user interrupt
* fixed slicing issues with numpy and highs
* added `resetGlobalScheduler`
* released GIL for `Presolve`
* fixed issues with deadlock on Windows
* fixed MIP solution callback issue
* support `getExpr` that creates a `highs_linear_expression` from existing row

Should address multiple issues: ERGO-Code#1865, ERGO-Code#1882, ERGO-Code#1888, ERGO-Code#1892, ERGO-Code#1903, ERGO-Code#1904, and perhaps ERGO-Code#1905
* Fixed all static analysis (strict) from pyright
* Updated typing across all highspy, including submodules
* Added numpy subclass HighspyArray. This allows for better typing, fast summation and vectorized linear expressions
* General cleanup, including fixes from ruff linting and formatting
* Added overload functions for improved intellisense
* Additional checks, tests, and code coverage
* Slight updates to cpp bindings to ensure types are available
@mathgeekcoder mathgeekcoder marked this pull request as draft September 21, 2024 02:19
Copy link
Member

Choose a reason for hiding this comment

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

Some constants in this file are needed by Python users, but many others are for development options that I'd rather not encourage any users to play with, and may confuse them.

Shall I try to edit them out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file was actually auto-generated by pybind11-stubgen.

I believe it should be fine to remove any option from the .pyi file. It'll still be accessible by the user if they really really want.

Copy link
Member

Choose a reason for hiding this comment

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

OK. We'll have them for now, and remove some of them later. I've got other pressing issues and don't want to hold up this PR

@jajhall
Copy link
Member

jajhall commented Sep 26, 2024

Closes the original #992, #1044 and #1670

@few
Copy link
Contributor

few commented Sep 26, 2024

There are breaking changes in highspy:
highs_linear_expression had properties LHS, RHS and vars which I used and are now gone. I can easily change my code, just mentioning it in case backwards compatibility is important for you.

@few
Copy link
Contributor

few commented Sep 26, 2024

You mentioned #1905. I don't see new callbacks in this merge request, and highspy still doesn't handle CTRL+C on its own. Am I missing something?

@few
Copy link
Contributor

few commented Sep 26, 2024

Regarding #1882: data_out.mip_solution in the kCallbackMipImprovingSolution callbacks is not an array, but prints as this:
<highspy._core.readonly_ptr_wrapper_double object at 0x7f921d7b9af0>

@galabovaa
Copy link
Contributor

@few

There are breaking changes in highspy: highs_linear_expression had properties LHS, RHS and vars which I used and are now gone. I can easily change my code, just mentioning it in case backwards compatibility is important for you.

Please update your code with the new highs_linear_expression syntax, the highspy modelling language is not very mature yet, and this is indeed one breaking change, but we decided it would be better to have it in this format.

Thank you for your comments, we will look into 1905 and 1882 agian.

@jajhall
Copy link
Member

jajhall commented Sep 26, 2024

Thanks @galabovaa.

Sorry @few if I was a bit enthusiastic about closing issues, but I'm not in a position to assess Python modifications, and took the lead from the comments in the PR

@mathgeekcoder
Copy link
Contributor Author

mathgeekcoder commented Sep 26, 2024

@few:

highs_linear_expression had properties LHS, RHS and vars which I used and are now gone. I can easily change my code, just mentioning it in case backwards compatibility is important for you.

Apologies about the breaking change. I never imagined people would be using the internal LHS/RHS/vars etc. As @galabovaa mentioned, please update your code.

BTW: it's likely that you don't need to reference these internals anymore anyhow.

You mentioned #1905. I don't see new callbacks in this merge request, and highspy still doesn't handle CTRL+C on its own. Am I missing something?

For Ctrl-C support you'll need to set h.HandleKeyboardInterrupt = True. It is false by default, as it needs to spawn a new thread.

For #1905:

  1. Presolve callbacks are not yet implemented the C++ HiGHS code
  2. The python callbacks will automatically enable any callback on subscription. If you disableCallbacks/enableCallbacks it will disable/enable all callbacks that have been subscribed - and will remember past subscriptions - unless they are explicitly cleared.
  3. The "finished" callback shouldn't be required, as I'm using thread sync objects to avoid any issues.

Regarding #1882: data_out.mip_solution in the kCallbackMipImprovingSolution callbacks is not an array, but prints as this:
<highspy._core.readonly_ptr_wrapper_double object at 0x7f921d7b9af0>

Correct. I didn't want to change the internal HiGHS code, but I wanted to give access to the array. This array is guaranteed to be the size of h.numVariables. You can get a copy of the array via:

copy = data_out.mip_solution.to_array(h.numVariables)

But that's not required. You can also access the values with your variables or linear expressions, e.g.,

x = h.addVariables(...)
expr = x[0] + 2*x[1]

def my_callback(e):
   copy = e.val(x)
   result = e.val(expr)
   solution = e.val(h.getVariables())

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.

4 participants