UPD: update to the NonlinearExpr syntax of JuMP v1.15 #454
Conversation
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. |
|
Now we're getting somewhere. These two failures just look flakey? One looks like a tolerance issue, and the other is Ipopt not succeeding. |
|
Given all tests are passing, is there any reason not to merge and tag? |
|
From my side, I'm good. |
Project.toml
Outdated
| Dates = "1.6" | ||
| FilePaths = "0.8.3" | ||
| Glob = "1.3" | ||
| Graphs = "1" | ||
| InfrastructureModels = "0.7.3, 0.7.5" | ||
| Ipopt = "0.9, 1.0.2, 1.1" | ||
| Ipopt = "1" | ||
| JSON = "0.18, 0.19, 0.20, 0.21" | ||
| JuMP = "0.22, 0.23, 1" | ||
| JuMP = "1.23.2" | ||
| LoggingExtras = "0.4.7, 1" | ||
| PolyhedralRelaxations = "0.3.5" | ||
| SCS = "0.9, 1.0, 1.1" | ||
| SparseArrays = "1.6" | ||
| SpecialFunctions = "2" | ||
| Test = "1.6" |
There was a problem hiding this comment.
Is there a reason Dates, SparseArrays and Test were added to compat? My impression was that these don't need to be explicitly listed because of the julia compat entry.
|
I've added the CHANGELOG entry. For the compat bounds: future versions of Julia are going to decouple stdlib versions from the Julia version, so best practice is to add compat bounds. But there was a bug in Julia 1.6 that prevented them from enforcing this rule all packages at registration time. I've removed for now, but in some future version, we'll likely need to add them back. (Probably when/if you set |
|
Looks good! Thanks for the info about stdlib versions getting decoupled, I didn't know that, so I'll keep an eye out in the future. Thanks again for the contribution! |
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.