BUG: Fix highspy __getitem__ speed#1438
Closed
HaoZeke wants to merge 4 commits intoERGO-Code:masterfrom
Closed
Conversation
__getitem__ method for 9799 vars: 0.0956 Copy-first method for 9799 vars: 0.0010
__getitem__ method for 9799 vars: 0.0064 Copy-first method for 9799 vars: 0.0015 ERGO-Code#1277
HaoZeke
added a commit
to HaoZeke/HiGHS
that referenced
this pull request
Sep 30, 2023
HaoZeke
added a commit
to HaoZeke/HiGHS
that referenced
this pull request
Sep 30, 2023
Contributor
Author
|
One of the biggest issues with this is that now users need to be careful to cast: # Before:
lp.col_cost_ = c
# After:
vector_double_c = highspy._highs.VectorDouble(c.tolist())
lp.col_cost_ = vector_double_cThis can be sidestepped with the NumPy approach (or just documented). Alternatively, since for the modeling API we have the Python wrapper over the C++ internals anyway, we can just extract it into a list before operating on it (as is done in the test case). |
Contributor
Author
|
I don't like any of these here actually. They're a mess for downstream library integration (e.g. SciPy). The solution should be to rework the |
HaoZeke
added a commit
to HaoZeke/HiGHS
that referenced
this pull request
Feb 3, 2024
HaoZeke
added a commit
to HaoZeke/HiGHS
that referenced
this pull request
Feb 3, 2024
HaoZeke
added a commit
to HaoZeke/HiGHS
that referenced
this pull request
Feb 3, 2024
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1277. For this test case (adapted from here):
Where the
80bau3b.mpsis part of this repo, we have:As a first approximation, we can use the buff protocol and
numpy:However, this isn't equivalent to
readwriteand isn't very nice conceptually, so we can try to use an opaque type:This can be optimized a bit more:
Note that this method has the caveats listed in the documentation. I think the standard opaque type is probably about as far as can be optimized safely, though there seem to be some micro-optimizations discussed on Gitter (which I admit are beyond me).