Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add standard_form modifications #1935

Merged
merged 5 commits into from
Apr 9, 2019
Merged

Add standard_form modifications #1935

merged 5 commits into from
Apr 9, 2019

Conversation

odow
Copy link
Member

@odow odow commented Apr 8, 2019

Closes #1890

I have deprecated set_coefficient in favor of set_standard_form_coefficient.

@codecov
Copy link

codecov bot commented Apr 9, 2019

Codecov Report

Merging #1935 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1935      +/-   ##
==========================================
+ Coverage   91.06%   91.07%   +<.01%     
==========================================
  Files          32       32              
  Lines        3950     3954       +4     
==========================================
+ Hits         3597     3601       +4     
  Misses        353      353
Impacted Files Coverage Δ
src/constraints.jl 90.75% <100%> (+0.15%) ⬆️
src/macros.jl 93% <0%> (+0.02%) ⬆️

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 b88eb99...44114e9. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 9, 2019

Codecov Report

Merging #1935 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1935      +/-   ##
==========================================
+ Coverage   91.06%   91.07%   +<.01%     
==========================================
  Files          32       32              
  Lines        3950     4256     +306     
==========================================
+ Hits         3597     3876     +279     
- Misses        353      380      +27
Impacted Files Coverage Δ
src/constraints.jl 91.05% <100%> (+0.45%) ⬆️
src/quad_expr.jl 92.03% <100%> (+1.19%) ⬆️
src/aff_expr.jl 94.08% <100%> (+1.52%) ⬆️
src/operators.jl 82.38% <0%> (-1.66%) ⬇️
src/macros.jl 93% <0%> (+0.02%) ⬆️
src/variables.jl 97.9% <0%> (+0.21%) ⬆️
src/objective.jl 96.29% <0%> (+1.29%) ⬆️
src/Containers/DenseAxisArray.jl 85.98% <0%> (+4.07%) ⬆️

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 b88eb99...8305c2d. Read the comment docs.

Copy link
Member

@mlubin mlubin left a comment

Choose a reason for hiding this comment

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

What do you think about corresponding getters standard_form_coefficient and standard_form_rhs?

@odow odow merged commit 665f3a7 into master Apr 9, 2019
@odow odow deleted the od/rhs branch April 9, 2019 15:48
@blegat
Copy link
Member

blegat commented Apr 9, 2019

I don't support renaming set_coefficient to set_standard_form_coefficient`.
The "standard form" is confusing because of the confusion with https://en.m.wikipedia.org/wiki/Linear_programming#Standard_form

@odow
Copy link
Member Author

odow commented May 10, 2019

Not sure if this counts as breaking because it adds deprecations.

@blegat
Copy link
Member

blegat commented May 10, 2019

Do you think it should be included in v0.19.1 ?

@odow
Copy link
Member Author

odow commented May 10, 2019

I don't mind. It isn't urgent, so it can probably wait. (Selfish reason, I'll have to update SDDP.jl, and relying on a patch 0.19.1 isn't fun.)

@blegat
Copy link
Member

blegat commented May 11, 2019

Even if there is warning, if someone release a package using this function and people use this package, when updating JuMP, the package will throw warning so it is kind of breaking.
I'd like to release MOI v0.9 in a few days so if we give a week for packages to update to it, we can release JuMP v0.20 in about a week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

Modify RHS (and define RHS)
4 participants