lanl-ansi / Alpine.jl

A Julia/JuMP-based Global Optimization Solver for Non-convex Programs
https://lanl-ansi.github.io/Alpine.jl/latest/
Other
241 stars 39 forks source link

Simplify handling of solver identifiers #213

Closed odow closed 1 year ago

odow commented 1 year ago

Ah. We were working over top of each other: https://github.com/lanl-ansi/Alpine.jl/pull/212/commits/8a1542465bcc81ad6ce9982521b03745735e84b0

odow commented 1 year ago

@harshangrjn take a look. It's a bit wasteful, but the easiest option is to try and instantiate the solver then ask for SolverName.

There's also a bunch of old code floating around that isn't doing what you think.

I see you commented this block:

# if m.mip_solver_id != :Gurobi
   #  Alp.get_option(m, :convhull_warmstart) == false
   #  Alp.get_option(m, :convhull_no_good_cuts) == false
# end

but it was comparing a string against a symbol, so it was always true, and then it was using get_option instead of set_option?

Ideally, you wouldn't need any solver-specific functionality in Alpine.

harshangrjn commented 1 year ago

@odow I was about to delete those lines of get_option as they weren't doing what they had to. Also, the reason we need solver specific functionality is because there are certain features which are accessible by only certain solvers. For example, in collecting solution pool in Gurobi, or in being able to include a multi warm-start in local solver like Ipopt. But I agree that eventually it is better to have solver-independent methods in Alpine. Are you ok if I merge these changes with #212, or if we bring in the ones from there, that's fine too.

codecov[bot] commented 1 year ago

Codecov Report

Merging #213 (5552797) into solver_identifier_issue (8a15424) will increase coverage by 0.05%. The diff coverage is 100.00%.

@@                     Coverage Diff                     @@
##           solver_identifier_issue     #213      +/-   ##
===========================================================
+ Coverage                    86.38%   86.43%   +0.05%     
===========================================================
  Files                           16       16              
  Lines                         3195     3193       -2     
===========================================================
  Hits                          2760     2760              
+ Misses                         435      433       -2     
Impacted Files Coverage Δ
src/MOI_wrapper/MOI_wrapper.jl 86.47% <ø> (ø)
src/algorithm.jl 89.16% <100.00%> (ø)
src/utility.jl 84.46% <100.00%> (+0.63%) :arrow_up:

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

odow commented 1 year ago

Are you ok if I merge these changes with https://github.com/lanl-ansi/Alpine.jl/pull/212, or if we bring in the ones from there, that's fine too.

Yes, it should be set up for you to merge this into #212