-
Notifications
You must be signed in to change notification settings - Fork 42
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
UPD: update to the NonlinearExpr syntax of JuMP v1.15 #454
base: main
Are you sure you want to change the base?
Conversation
JuMP.@constraint($model, $expr) | ||
end | ||
end) | ||
end |
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.
This PR is breaking because it removes this macro which was documented.
@@ -133,7 +133,7 @@ function constraint_mc_current_balance(pm::RectangularVoltageExplicitNeutralMode | |||
ungrounded_terminals = [(idx,t) for (idx,t) in enumerate(terminals) if !grounded[idx]] | |||
|
|||
for (idx, t) in ungrounded_terminals | |||
@smart_constraint(pm.model, [cr, crd, crg, crs, crsw, crt, vr], | |||
JuMP.@constraint(pm.model, |
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.
As currently formatted, the body of the constraint is indented by a lot. (It used to match the opening [
of the second argument. I can either leave to minimize the diff of this PR, or fix. Not sure which is best.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #454 +/- ##
==========================================
- Coverage 75.01% 73.64% -1.37%
==========================================
Files 73 71 -2
Lines 16541 16264 -277
==========================================
- Hits 12408 11978 -430
- Misses 4133 4286 +153 ☔ View full report in Codecov by Sentry. |
@ccoffrin (or anyone): are the failing tests global solutions (and there's a bug somewhere)? Or are these local minima, and by changing the order of constraints etc we now find new solutions? |
Good question. I am not that familiar with how stable the tests are in terms of local minima. The failing tests do appear to be consolidated in the non-convex formulations, so it is a reasonable hypothesis. We will need one of the primary developers to look into it to assess. |
Hey @pseudocubic any chance you could take a look at this? Or point me in the right direction? |
@odow we have been looking at this but still haven't identified an obvious pattern in the failing tests, and based on the changes in PMD and JuMP 1.15 don't understand what would have caused all the failures we're seeing. |
Closes #453
The tests haven't finished running locally, so there might be some more changes needed, but this is a start. I'll comment in-line about some formatting changes we could do here, or in a separate PR.