-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
MAINT: optimize.linprog: make HiGHS default and deprecate old methods #15512
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am +1 on reducing our technical debt especially if we have better methods.
In scikit-learn's |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thank you for taking the time to tidy this up.
Great. Let's give it a week for comments and merge on 2/9 if there are no objections. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comment applies in several places of the diff.
`'simplex'`) are legacy methods and will be removed in the second release | ||
after SciPy 1.9.0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"second release after SciPy 1.9.0." is not clear - presumably this doesn't refer to 1.9.2, but 1.11.0? Unless there are concrete plans for a scipy 2.0, I'd just write 1.11.0 here, or at least qualify "second non-patch release after 1.9.0".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks... yeah, it would have been "1.11.0" but for comments in gh-13490. I was thinking about qualifying with "minor" but was worried some users would think that would mean bug-fix release. Is there a standard description other than "non-patch"? "feature" release?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I looked at that discussion, but I kinda fail to see the reason why the minor counter couldn't go to two digits - numpy and cpython do the same...? I'm thinking a putative 2.0 should come with some degree of "large change compared to 1.x
", even if it isn't breaking anything**.
In any case, I'd say it would be simpler to just leave 1.11 here, and if a version 2.0 is decided (should we open an issue to discuss?), then it's a simple matter of searching for "1.10"/"1.11"/etc. and replacing them appropriately.
CC @ev-br @rgommers @tupui @tylerjereddy
Is there a standard description other than "non-patch"? "feature" release?
I wanted to avoid "minor" due to not mapping well to scipy's versioning scheme, but "feature release" would work. That's still worse than using 1.11 IMO, but definitely an improvement over what we have now.
** as an example in terms of the kind of scope for a major bump that I have in mind, a current(ly-talked-about) candidate for cpython 4.0 is if/once the GIL is removed. On the numpy side, although there is an ancient, non-empty milestone, I'm not aware of current plans for 2.0 (though I did once wager that it'll come about eventually 😉) - comprehensively fixing integer promotion issues might be a candidate to go to numpy 2.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say "two releases after N" and leave it at that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say "two releases after N" and leave it at that
That's the part I disagree with. Strictly speaking "two releases after 1.9.0" is "1.9.2", which is clearly not what is intended here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you feel strongly enough to conflate the issue at hand with a decision on the 1.10 vs 2.0 naming, I suggest to separate the latter and discuss it on the mailing list.
My personal opinion is that 2.0 is no special and I'd have a slight preference for 1.9->2.0. That said, it's not quite my hill so I'd go with a majority on this one :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Respectfully, I'm not conflating anything. Avoiding a direct "will be removed in Scipy 1.11.0" and ending up with awkward wording (i.e. the issue at hand) only makes sense if a possible 2.0 release is a possibility. Still, I'll open an issue to discuss the version after 1.9.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still, I'll open an issue to discuss the version after 1.9.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! CC'ing HiGHS upstream for awareness @jajhall
@tupui @mckib2 @Kai-Striega @ev-br I sent the message to the mailing list about one week ago. Would one of you like to merge this tomorrow if no objections come up before then? @h-vetinari I agree "two releases" is more likely to be interpreted as two bug-fix releases. Let's going ahead and merge this as-is and I'll come back and fix this and the gh-13490 wording once gh-15528 is resolved. Sound good? |
Happy to do the honour tomorrow 😉 |
Thanks @mdhaber, quite a few issue closed 😅👍 |
Reference issue
closes gh-15507
closes gh-14471
closes gh-14081
addresses
linprog
part of gh-13846closes gh-13200
closes gh-12485
closes gh-11617
closes gh-11570
closes gh-11515
closes gh-11326
closes gh-11215
closes gh-11201
closes gh-10288
closes gh-9671
closes gh-9536
closes gh-7877
I don't think GitHub will take care of PRs automatically:
closes gh-14488
closes gh-12510
closes gh-11618
What does this implement/fix?
The HiGHS methods for
linprog
have been thoroughly tested in the last few SciPy releases and have demonstrated that they are (much) faster and more robust than the original methods. This PR makesmethod='highs'
the default and deprecatesmethod='simplex'
,method='revised simplex'
, andmethod='interior-point'
.Additional information