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

support for solver with direct model? #24

Closed
xhub opened this issue May 17, 2020 · 10 comments
Closed

support for solver with direct model? #24

xhub opened this issue May 17, 2020 · 10 comments

Comments

@xhub
Copy link

xhub commented May 17, 2020

Right now the tests only work with a solver specified as an optimizer_factory for a JuMP.Model call.
This precludes using this nice package with a solver in direct mode.

Is it planned to support specifying the solver in direct mode?
My guess is that the main hurdle is that one needs to use JuMP.direct_model to create such solver, rather than JuMP.Model.

@ccoffrin
Copy link
Collaborator

As I understand it, any solver that supports direct mode should also support the cacheing mode that is used in these tests.

Thoughts @odow, @blegat?

@odow
Copy link
Member

odow commented May 17, 2020

any solver that supports direct mode should also support the cacheing mode that is used in these tests.

Correct. What is the motivation for testing in direct mode?

@xhub
Copy link
Author

xhub commented May 17, 2020

To test the direct mode codepath :)

Any solver would be able to use this package via any mode, I would find it nice to be able to also test the direct mode. Nothing critical, would just be nice to have. "No" is a perfectly fine answer

@odow
Copy link
Member

odow commented May 17, 2020

Which solver? Have you implemented a specialized copy_to in addition to the direct interface?

@ccoffrin
Copy link
Collaborator

I think it would be in scope of this repo to add one "direct mode" test to the suite. Thoughts on that @odow?

@xhub
Copy link
Author

xhub commented May 17, 2020

https://github.com/xhub/ReSHOP.jl

I did not implement a copy_to.
I was playing with direct_model a few month ago and I was struggling to find what should be implemented for the direct interface. So I was looking to do some TDD, and this repo is quite nice for that purpose. Maybe things have changed in the documentation, would love to learn more about it (on the proper channel? drifting towards offtopic I feel :))

@odow
Copy link
Member

odow commented May 17, 2020

Since you have this line:
https://github.com/xhub/ReSHOP.jl/blob/f42d87d0f4bf96339e2377b4520477340224bfc4/src/MOI_wrapper.jl#L129-L132
the current tests will have the same coverage as if you used direct_mode.

Model(optimizer) results in the following steps:

Direct mode skips the JuMP cache and copy_to, and goes straight to add_variable and add_constraint.

I don't think we need any changes to MINLPTests.jl.

@xhub
Copy link
Author

xhub commented May 17, 2020

It dates back from February, but this is what I recall. When playing with direct_model, I ended up implementing some functions, like function MOI.get(model::Optimizer, loc::MOI.ListOfConstraints) or MOI.is_valid(model::Optimizer, ci::MOI.ConstraintIndex) which are not required for a classical MOI solver. For instance KNITRO has no MOI.is_valid, and is (I believe) fully functional as a solver. By looking at MOSEK, I saw a MOI.transform that I may want to also add. So coverage with in direct model is higher.

Correct me if I am wrong, but it is not documented which functions needs to be implemented so I was just trying with some toy models. Hence, the TDD approach. There is an extensive list in apireference.md, it would be great to just have a table of functions that needs to be implemented for a complete direct interface

@odow
Copy link
Member

odow commented May 17, 2020

If KNITRO doesn't implement MOI.is_valid, then that is a bug.

In fact, it looks like it is missing a lot of the interface, because most tests are excluded:
https://github.com/JuliaOpt/KNITRO.jl/blob/60d24774ca1ed9f998c572d35988c2ff23468ab3/test/MOI_wrapper.jl#L68-L82

and it doesn't implement

MOI.Test.basic_constraint_tests(model, config)

or

MOI.Test.unittest(model, config)

If ReSHOP passed the following, plus MINLPTests, you're doing pretty well:

MOI.Test.unittest(model, config)
MOI.Test.basic_constraint_tests(model, config)
MOI.Test.contlineartest(model, config)

@xhub
Copy link
Author

xhub commented May 17, 2020

Thanks for the pointers, I didn't have

MOI.Test.unittest(model, config) 
MOI.Test.basic_constraint_tests(model, config)

and I am still skipping some tests in contlineartest, will work on that.

Closing since the MOIT seems to cover quite a lot already

@xhub xhub closed this as completed May 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants