Move code to MOI and add JuMP layer #19
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19 +/- ##
==========================================
- Coverage 97.05% 89.91% -7.15%
==========================================
Files 5 7 +2
Lines 1054 1388 +334
==========================================
+ Hits 1023 1248 +225
- Misses 31 140 +109 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
@odow Thoughts? |
src/ModelAnalyzer.jl
Outdated
|
|
||
| This variant allows summarizing a single issue instance of type `AbstractIssue`. | ||
| The model tha led to the issue can be provided to `name_source`, it will be used |
There was a problem hiding this comment.
| The model tha led to the issue can be provided to `name_source`, it will be used | |
| The model that led to the issue can be provided to `name_source`, it will be used |
There was a problem hiding this comment.
Why not just ; model::Union{Nothing,JuMP.AbstractModel} = nothing?
There was a problem hiding this comment.
Fixed!
I don't want to type this because I want to have JuMP as a weakdep
| @@ -107,9 +109,11 @@ julia> ModelAnalyzer.summarize(ModelAnalyzer.Numerical.DenseConstraint) | |||
| ``` | |||
| """ | |||
| struct DenseConstraint <: AbstractNumericalIssue | |||
| ref::JuMP.ConstraintRef | |||
| ref::MOI.ConstraintIndex | |||
There was a problem hiding this comment.
Open question whether we should add {F,S} in many places. Not sure
There was a problem hiding this comment.
I am not sure as well, so I was keeping it simpler for now.
src/_eval_variables.jl
Outdated
| # TODO: this conversion exists in JuMP, but not in MOI | ||
| S = Base.promote_op(value_fn, MOI.VariableIndex) | ||
| U = Base.promote_op(*, T, S) | ||
| out = convert(U, f.constant) |
There was a problem hiding this comment.
I don't get it, what does not exist in MOI ?
There was a problem hiding this comment.
There was a problem hiding this comment.
Ah yes, MOI only works if the return type of the function is the same as the type of f.constant. In JuMP, we try to be more general to be more user-friendly.
| data = ModelAnalyzer.analyze( | ||
| ModelAnalyzer.Feasibility.Analyzer(), | ||
| model, | ||
| primal_point = Dict(JuMP.index(x) => 1.0), |
There was a problem hiding this comment.
@odow this might be slightly bad for user.
We could try allowing Dict(x => 1.0): either intercepting the specific kwarg or some sort of map_variables
joaquimg
changed the title
close #6
close #22
ModelAnalyzer.variableand similar methods in tests