Skip to content

Conversation

@odow
Copy link
Member

@odow odow commented Apr 1, 2022

Closes #392

@codecov
Copy link

codecov bot commented Apr 1, 2022

Codecov Report

Merging #393 (a3395eb) into master (e962188) will decrease coverage by 0.19%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #393      +/-   ##
==========================================
- Coverage   50.76%   50.57%   -0.20%     
==========================================
  Files           7        7              
  Lines        3936     3937       +1     
==========================================
- Hits         1998     1991       -7     
- Misses       1938     1946       +8     
Impacted Files Coverage Δ
src/MOI/MOI_wrapper.jl 91.99% <100.00%> (-0.25%) ⬇️
src/gen1210/libcpx_api.jl 13.92% <0.00%> (-0.20%) ⬇️
src/gen2010/libcpx_api.jl 13.92% <0.00%> (-0.20%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e962188...a3395eb. Read the comment docs.


"""
Optimizer(env::Union{Nothing, Env} = nothing)
Optimizer(env::Union{Nothing, Env} = nothing; pass_names::Bool = false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not an AbstractOptimizerAttribute?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this is really a CPLEX.jl hack, rather than an option supported by CPLEX.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still, it would make sense to have attributes of the wrapper. I don't consider a hack, but an actual option of the wrapper.

The attribute (with a supports function) path is more generic, for users calling multiple solvers in a more abstract manner.

In fact, Xpress will require this eventually.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with @joaquimg. It could be optimizer_with_attributes(CPLEX.Optimizer, CPLEX.PassNames() => true)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I'll make a new PR.

@odow odow merged commit f79a6ed into master Apr 3, 2022
@odow odow deleted the od/names branch April 3, 2022 21:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Slow solution of large LPs w/o direct model

5 participants