Skip to content

Conversation

@matbesancon
Copy link
Contributor

Merge MOI._constant and MOIU.getconstant. Fixes #618

@blegat
Copy link
Member

blegat commented May 2, 2019

We might also add a way to get zero for the constant of SingleVariable and VectorOfVariables to solve the use case in:
https://github.com/JuliaOpt/MathOptInterface.jl/blob/c1d8f6a6949a39091c89d8c4053d51a7f6cc8d03/src/Bridges/scalarizebridge.jl#L7-L10
For that we need to know what is the coefficient type. We could allow a second argument which is the coefficient type but is optional for every set/function except SingleVariable/VectorOfVariables.

For the name, getconstant does not satisfy JuMP style guide, it should be constant, get_constant, or set_constant/function_constant. The second choice is inconsistent with the naming in the rest of JuMP/MOI but the first and third choices works (the first one is better IMO since the fact that it is a setorfunction` is already in the signature so adding it in the name is redundant).

@codecov-io
Copy link

codecov-io commented May 2, 2019

Codecov Report

Merging #723 into master will not change coverage.
The diff coverage is 84.61%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #723   +/-   ##
=======================================
  Coverage   93.83%   93.83%           
=======================================
  Files          54       54           
  Lines        5677     5677           
=======================================
  Hits         5327     5327           
  Misses        350      350
Impacted Files Coverage Δ
src/Bridges/quadtosocbridge.jl 93.44% <100%> (ø) ⬆️
src/sets.jl 94.11% <100%> (+0.36%) ⬆️
src/Bridges/vectorizebridge.jl 85.71% <100%> (ø) ⬆️
src/Utilities/sets.jl 100% <100%> (ø) ⬆️
src/Bridges/scalarizebridge.jl 53.12% <50%> (+0.34%) ⬆️
src/functions.jl 95.16% <77.77%> (-3.12%) ⬇️
src/Utilities/functions.jl 91.86% <83.33%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c1d8f6a...8b9fdd6. Read the comment docs.

@blegat
Copy link
Member

blegat commented May 2, 2019

@matbesancon
Copy link
Contributor Author

I put it after output_dimension

@blegat blegat added this to the v0.9 milestone May 2, 2019
@matbesancon
Copy link
Contributor Author

ok this looks good

@matbesancon
Copy link
Contributor Author

@blegat seems good here

@blegat blegat merged commit 88656c6 into jump-dev:master May 3, 2019
coroa added a commit to coroa/JuMP.jl that referenced this pull request Jul 8, 2019
coroa added a commit to coroa/JuMP.jl that referenced this pull request Aug 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Refactor MOI._constant and MOIU.getconstant

3 participants