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

Sensitivity part II #2251

Merged
merged 1 commit into from
Dec 15, 2020
Merged

Sensitivity part II #2251

merged 1 commit into from
Dec 15, 2020

Conversation

odow
Copy link
Member

@odow odow commented Jun 8, 2020

Closes #2244

This PR is a rewrite of the sensitivity summary to avoid a lot of redundant computations and allocations.

I've tried to comment my understanding of the code, and compared against the solutions reported by Gurobi and Excel.

Points of contention are:

  • What is the sensitivity of a RHS of an Interval constraint? I have left unimplemented.
  • What is the objective sensitivity of a nonbasic variable with fixed bounds? Gurobi seems to report +/-0.0, where as Excel reports +/-Inf. The current behavior is +/-Inf.

I'm not sure I fully understand all of the edge cases, so it will be interesting to see what the coverage is.

cc @geoffleyland: Want to stress-test this?

@codecov
Copy link

codecov bot commented Jun 8, 2020

Codecov Report

Merging #2251 (5c47a44) into master (508d3a5) will increase coverage by 0.24%.
The diff coverage is 99.31%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2251      +/-   ##
==========================================
+ Coverage   91.61%   91.86%   +0.24%     
==========================================
  Files          42       43       +1     
  Lines        4451     4596     +145     
==========================================
+ Hits         4078     4222     +144     
- Misses        373      374       +1     
Impacted Files Coverage Δ
src/JuMP.jl 74.59% <ø> (ø)
src/lp_sensitivity2.jl 99.30% <99.30%> (ø)
src/lp_sensitivity.jl 93.56% <100.00%> (+0.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 508d3a5...5c47a44. Read the comment docs.

@joaquimg
Copy link
Member

joaquimg commented Jun 9, 2020

matrix building is really similar to what is done in:
https://github.com/JuliaOpt/MatrixOptInterface.jl

Matrix building (from MOI) will also be used in the differentiation GSOC.
A more general version of that is replicated in solvers like SCS and ProxSDP.

I think the matrix builder should live in MOIU. Starting with LP, for this use, then do QP (for GSOC) and Conic (also for GSOC, but potentially used in solvers).

This could simplify some first version solver wrappers.
And could be used by students writing their first solvers.

@odow
Copy link
Member Author

odow commented Jun 10, 2020

👍 to adding matrix-type support to MOI as a submodule.

src/lp_sensitivity2.jl Outdated Show resolved Hide resolved
@joaquimg
Copy link
Member

I have seen that Xpress allows you to query rhs and obj sensitivities.
Also, Gurobi and Xpress allow you to pass a vector and they will multiply it by the inverse basis matrix.

@odow
Copy link
Member Author

odow commented Jun 12, 2020

Yeah some solvers do allow you to query the sensitivity. But I'm not sure how that plays with bridges, etc.

@joaquimg
Copy link
Member

Indeed, that is a bit of a pain

@odow
Copy link
Member Author

odow commented Nov 28, 2020

Anyone interested in reviewing this? If not, I'll merge in a few days. It's a significant improvement over what we have currently, which is unusable for anything but toy models.

src/lp_sensitivity2.jl Outdated Show resolved Hide resolved
src/lp_sensitivity2.jl Outdated Show resolved Hide resolved
src/lp_sensitivity2.jl Show resolved Hide resolved
This functionality is intended as a replacement for the previous
lp_xxx_perturbation_range code. The main criticism of the previous
implementation was that it created the basis matrix every function
call, rather than doing it once and caching the result.

Instead of adding complexity by having a field cache the basis, this
PR computes the perturbation for every variable and constraint up-front.
@odow
Copy link
Member Author

odow commented Dec 14, 2020

Bump. Anyone still want to review this/objections to merging?

@odow odow merged commit 52b1b38 into master Dec 15, 2020
@odow odow deleted the od/lpsen branch December 15, 2020 21:31
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.

Improve performance of LP sensitivity summary
5 participants