-
Notifications
You must be signed in to change notification settings - Fork 46
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
Use HiGHS solver by default, with Big M, and End-of-Life for v1 and v2 #575
Conversation
Bill-Becker
commented
Mar 30, 2024
- See PR Use HiGHS solver by default and on production, include other OS solvers #574
mining operations may use > 1,000 GWh
Use HiGHS solver by default and on production, include other OS solvers
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.
This builds locally and I was able to run test_job_endpoint just as a quick check. Thanks @Bill-Becker!
CHANGELOG.md
Outdated
- See updates from REopt.jl v0.44.0: https://github.com/NREL/REopt.jl/releases/tag/v0.44.0 | ||
- HiGHS, Cbc, and SCIP solvers use Big M notation constraints only in REopt.jl | ||
#### Deprecated | ||
- End-of-Life for v1 and v2 of the API for external/public interfacing from NREL servers |
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.
@Bill-Becker should we include a bit more detail about what this means for users? E.g., Will users no longer be able to access results using v1 and v2 endpoints, or just will no longer be able to post to these endpoints?
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 made a discussion forum post about this, and added a link in both the API response error message and in the Changelog in 089153d
@@ -296,7 +296,7 @@ class SOLVERS(models.TextChoices): | |||
|
|||
solver_name = models.TextField( | |||
blank=True, | |||
default=SOLVERS.XPRESS, | |||
default=SOLVERS.HIGHS, |
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.
This might not be critical for this PR, but if you use an empty string (below) for the solver_name input, it does not automatically select HiGHS and the error under "messages" is just Unexpected Error...
post_1["Settings"]["solver_name"] = '' or ""
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, yeah I'll punt on this for now. I don't think the user would enter an empty string accidentally in most cases.
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.
My proposal is (i) agreeing to punt for now, and (ii) making a fix for this on the REopt.jl side to allow empty strings as input which default to HiGHS longer-term.
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.
@Bill-Becker this look good to me! I just had two minor comments that could be worth addressing depending on priority. I spun up this branch locally and tested all of the solvers as well as the error messages from the v1 and v2 endpoints.
Thanks Amanda! I addressed one of the two items, and punted on the other. |