-
Notifications
You must be signed in to change notification settings - Fork 120
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
Test conic forms for constant values #617
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #617 +/- ##
==========================================
+ Coverage 97.75% 97.79% +0.04%
==========================================
Files 88 88
Lines 5073 5075 +2
==========================================
+ Hits 4959 4963 +4
+ Misses 114 112 -2 ☔ View full report in Codecov by Sentry. |
Okay. I've simplified the test handling, but now it requires #623 |
Urgh. I borked that. Let's just wait and I'll rebase on |
context = Convex.Context{value_type}(MOI.Utilities.Model{value_type}) | ||
atom = _to_constant(build_fn(context)) | ||
if Convex.iscomplex(atom) || any(Convex.iscomplex, atom.children) | ||
return |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll fix this in a separate PR.
Addresses #616 in the real case only. In the complex case unfortunately I found many issues, too many to address in this PR. Some would be addressed similarly to the change for
new_conic_form!(context::Context{T}, e::RelativeEntropyAtom)
, whereGenericConstraint
nicely handles the constant case, but directMOI_add_constraint
does not.(The issues here are not really correctness bugs, but rather errors one would get if for some reason they had constant values rather than variables in some atoms, such as if you
fix!
'd a variable).