Error when solving non-DCP problems #523
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #523 +/- ##
==========================================
+ Coverage 92.26% 92.30% +0.03%
==========================================
Files 86 86
Lines 5300 5301 +1
==========================================
+ Hits 4890 4893 +3
+ Misses 410 408 -2 ☔ View full report in Codecov by Sentry. |
ericphanson
changed the title
|
That's a good idea, I've made those changes here. I've still left an opt-out in case there are rare cases in which it can be useful. I chose not to mention the opt-out in the error message since I don't want folks turning to it without understanding all the implications. |
src/Convex.jl
Outdated
|
|
||
| Convex.emit_dcp_warnings() = false | ||
| to (globally) allow non-DCP problems to be solved. Note that if a problem is not DCP, the solution obtained by Convex.jl can be wildly incorrect (even if the solution status is `OPTIMAL`!). |
There was a problem hiding this comment.
Is there a practical use-case for allowing this? cc @mlubin
There was a problem hiding this comment.
I remember someone using it for alternating minimization. It could be something where they were going to fix a variable but hadn’t yet and the error occurs too early (when you construct the expression not when you solve the problem). The early errors should give more useful stacktraces though.
There was a problem hiding this comment.
actually something weird with the current place the errors are thrown is that it is only when you call vexity. This happens during problem solving so we at least always throw then. But it doesn't happen when you construct the expression, unless you show it, which does check the vexity (to print it). So I think this isn't ideal, since you end up with different-seeming behavior in the REPL (as it shows things).
There was a problem hiding this comment.
What about just erroring on optimize!?
There was a problem hiding this comment.
ok, I've made that change, and removed the opt-out
Co-authored-by: Oscar Dowson <odow@users.noreply.github.com>
ericphanson
changed the title
docs/examples_literate/supplemental_material/Convex.jl_intro_ISMP2015.jl
Outdated
Show resolved
Hide resolved
odow
changed the title
src/solution.jl
Outdated
| @@ -52,6 +52,10 @@ function solve!(p::Problem, optimizer_factory; silent_solver = false) | |||
| MOI.set(model, MOI.Silent(), true) | |||
| end | |||
|
|
|||
| if vexity(p) == NotDcp() | |||
There was a problem hiding this comment.
I think this isn’t quite right actually, we need to check for eg concave minimization problems
There was a problem hiding this comment.
Or at least, this is right, but there are other cases.
Noticed
while running the tests for #522. Using method invalidation for this is silly as there's no reason to recompile code; checking a global ref is very fast compared to everything else Convex is doing. Also, this fix is non-breaking as the previous advice to invalidate the method will continue to work here.