Skip to content

Refactor .objective field of Utilities.Model into a struct#1485

Merged
odow merged 5 commits intomasterfrom
od/model-objective
Jul 28, 2021
Merged

Refactor .objective field of Utilities.Model into a struct#1485
odow merged 5 commits intomasterfrom
od/model-objective

Conversation

@odow
Copy link
Copy Markdown
Member

@odow odow commented Jul 22, 2021

This simplifies the Utilities.Model quite a bit.

Currently needs test_broken due to the isapprox issue #1484

@odow odow added the Submodule: Utilities About the Utilities submodule label Jul 22, 2021
@odow odow requested a review from blegat July 22, 2021 02:53
@odow odow force-pushed the od/model-objective branch from 21d37a1 to 9942bd2 Compare July 22, 2021 04:47
@blegat
Copy link
Copy Markdown
Member

blegat commented Jul 22, 2021

Could we do something like:

struct UnionScalarFunction{T}
    single_variable::Union{Nothing,MOI.SingleVariable}
    scalar_affine::Union{Nothing,MOI.ScalarAffineFunction{T}}
    scalar_quadratic::Union{Nothing,MOI.ScalarQuadraticFunction{T}}
end

and have

mutable struct ObjectiveFunctionContainer{T,F} <: MOI.ModelLike
    ...
    objective_function::Union{Nothing,F}
end

and MOI.Utilities.Model{T} would use ObjectiveFunctionContainer{T,UnionScalarFunction{T}} while Clp and may others would use ObjectiveFunctionContainer{T,MOI.ScalarAffineFunction{T}} and COSMO would use ObjectiveFunctionContainer{T,MOI.ScalarQuadraticFunction{T}} for their default cache (#1381).
Note that in recent Julia version, MOI.Utilities.Model{T} just prints as MOI.Utilities.Model{T} if I'm not mistaken so the printing argument for avoiding parametrized types does not hold anymore :)

@odow odow force-pushed the od/model-objective branch from 44ad7a3 to 3d297ff Compare July 27, 2021 00:04
@odow odow added this to the v0.10 milestone Jul 27, 2021
@odow odow added the breaking label Jul 27, 2021
@odow
Copy link
Copy Markdown
Member Author

odow commented Jul 27, 2021

Coverage failure is because of poor testing of filter_variables from Utilities/functions.jl.

Copy link
Copy Markdown
Member

@blegat blegat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a nice improvement but it does not closes #1470. #1485 (comment) would close it but it can be done in a separate PR.

@odow odow merged commit d2ee9f3 into master Jul 28, 2021
@odow odow deleted the od/model-objective branch July 28, 2021 10:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking Submodule: Utilities About the Utilities submodule

Development

Successfully merging this pull request may close these issues.

2 participants