Skip to content
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

LQOI #146

Closed
wants to merge 43 commits into from
Closed

LQOI #146

wants to merge 43 commits into from

Conversation

joaquimg
Copy link
Member

@joaquimg joaquimg commented Oct 1, 2017

Changes might be to aggressive.

Is enough for MathOptInterfaceCPLEX, with LinQuadOptInterface

How do we proceed?

@joaquimg
Copy link
Member Author

I removed stuff from MOI which is lives now in MathOptInterfaceCPLEX.

This is needed to have MathOptInterfaceCPLEX working. It is passing CPLEX.jl tests.
Should I test anything else?

Is the current organization ok?

cc @mlubin @odow @blegat

@odow
Copy link
Member

odow commented Nov 15, 2017

Callbacks are still unresolved. I'm not in a rush to have this merged. It almost seems like there should be a CPLEXBase.jl with this version of the C wrapper to go with MathOptInterfaceCPLEX.jl. Then CPLEX.jl can stay with the MPB version and callbacks.

@joaquimg
Copy link
Member Author

I am not very found of having both CPLEXBase.jl and CPLEX.jl unless the second is completely discontinued and turned into legacy.

@odow
Copy link
Member

odow commented Apr 29, 2018

This PR, and #139 are both pretty unwieldy.

I propose closing both, and starting again from the latest master.

That new PR would

  • add the code from MathOptInterfaceCPLEX.jl
  • add all of the new cpx_xxx.jl files
  • retain all of the old code, except, where the arguments are the same, deprecate in favour of the new named functions. e.g., read_model becomes cpx_readcopyprob.
    An alternative to the last point:
  • the various add_constrs! add_var! methods leave a high surface area for bugs. Leave all of the function definitions, but replace the bodies with an appropriate error.

@joaquimg
Copy link
Member Author

joaquimg commented May 2, 2018

I am ok with that

@odow
Copy link
Member

odow commented Sep 3, 2018

Closing in favor of #181

@odow odow closed this Sep 3, 2018
This pull request was closed.
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.

2 participants