-
Notifications
You must be signed in to change notification settings - Fork 122
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
Enable commercial solver use without modifying GenX source code #531
Conversation
Awesome thanks so much.
…On Mon, Aug 21, 2023 at 2:18 PM Oscar Dowson ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In src/case_runners/case_runner.jl
<#531 (comment)>:
> @@ -13,14 +13,14 @@ end
@doc raw"""Run the GenX in the given folder
case - folder for the case
"""
-function run_genx_case!(case::AbstractString)
+function run_genx_case!(case::AbstractString, optimizer::Any=HiGHS.Optimizer)
::Any here is correct, because users can pass one of the following:
- a Type{<:MOI.AbstractOptimizer}, like HiGHS.Optimizer
- a function which when called with zero arguments returns an
optimizer, like () -> Gurobi.Optimizer(env)
—
Reply to this email directly, view it on GitHub
<#531 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHKPARQJY3NU2YY3HAEKFYDXWPGAVANCNFSM6AAAAAA3YXFXJU>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Here were some comments from the group discussion today. I think the sentiment was positive.
|
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 will be really excellent, and a key step for making GenX into a proper package.
- please add to the changelog
- My main recommendation is to chop down the example to something like 24 hours.
- Remove all solver settings other than highs/gurobi/cplex
- suggest calling the example ProprietarySolver or CommercialSolver rather than OneZone_Gurobi
- alter its README to stress what this example is about; forget the generic SmallNewEngland instructions
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.
Please see requested changes about - then let's pull it in!
@lbonaldo This PR needs to be rebased on the latest develop. |
4e3b63e
to
d47683c
Compare
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 believe all changes are in order. @lbonaldo any reason we can't merge this?
No, everything looks ready. We will have a final review tomorrow and then merge it. |
Description
This PR enables users to use a commercial solver without modifying the GenX source code, by adding an optimizer object as an argument to run_genx_case. This will both eliminate the significant annoyance for developers who use Gurobi of needing to update the GenX environment and module every time they pull code changes, and make it possible for users that are simply installing GenX as a package (without the ability to modify the source code) to use solvers that are not HiGHS. It also conforms with best practices suggested by the developers of JuMP (see jump-dev/Gurobi.jl#517).
A couple of notes:
What type of PR is this? (check all applicable)
Related Tickets & Documents
jump-dev/Gurobi.jl#517
Checklist
How this can be tested
Can test using the SmallNewEngland/OneZone_Gurobi test case.
Post-approval checklist for GenX core developers
After the PR is approved