Closed odow closed 1 month ago
Attention: Patch coverage is 80.61798%
with 69 lines
in your changes are missing coverage. Please review.
Project coverage is 73.64%. Comparing base (
80d6178
) to head (c968b1c
). Report is 2 commits behind head on main.:exclamation: Current head c968b1c differs from pull request most recent head a6b5224. Consider uploading reports for the commit a6b5224 to get more accurate results
Files | Patch % | Lines |
---|---|---|
src/form/acp.jl | 64.64% | 35 Missing :warning: |
src/core/objective.jl | 41.66% | 14 Missing :warning: |
src/form/en_acr.jl | 84.61% | 8 Missing :warning: |
src/form/acr.jl | 93.33% | 4 Missing :warning: |
src/form/bf_fbs.jl | 0.00% | 2 Missing :warning: |
src/form/fotp.jl | 0.00% | 2 Missing :warning: |
src/form/fotr.jl | 0.00% | 2 Missing :warning: |
src/form/ivr.jl | 96.42% | 2 Missing :warning: |
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
@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.
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 julia = "1.10"
.)
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.