Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Adding diagnostics tools for ill conditioning #1299
Adding diagnostics tools for ill conditioning #1299
Changes from 6 commits
0942f99
b1e5c9b
ba6c08c
34d942d
b637be8
48a1557
3349904
c1d9db6
181c5e0
7b2c2ac
85eb74e
db228a3
447055c
b0d39d8
5316a01
c4c9f5d
1e92d90
f08c298
cc51ef0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this name is a too vague, as there are many different ways of checking for ill-conditioning. Maybe
compute_ill_conditioning_certificate
orcompute_worst_conditioned_subset
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
compute_ill_conditioning_certificate
is better.compute_worst_conditioned_subset
would be a bit inaccurate since it might involve solving a MIP version of this problem.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're solving the minimum-residual problem globally, we have the "worst conditioned" combination of rows/columns, right? Then you're saying that there is no reason to believe that the result will be a subset?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so, but it will tend to be a subset because we minimize the 1-norm of the multiplier alongside requiring an optimal basic feasible solution to the LP, i.e., it's just LASSO.
Put another way, we could try to minimize the number of non-zeros in the multiplier, which would give a minimal subset demonstrating the ill conditioning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think
check_ill_conditioning
is too vague of a name here. I suggestcompute_ill_conditioning_certificate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this name better.
If we were honest it might be
try_to_compute_ill_conditioning_certificate
, as this LP itself will also be ill-conditioned. SoPlex has the capability to solve LPs to arbitrary precision, but it's not available currently as a (direct) Pyomo solver. And you cannot access the same options through SCIP, unfortunately.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated the name to
compute_ill_conditioning_certificate
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this called
inf_prob
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably should just go with
m
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like
m
. I'm fine with another name too, I just don't know what "inf" is.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"Infeasibility" of the linear dependence constraint?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to default to HiGHS or SCIP, although I understand if that can't happen for the time being.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The best solver to pick would probably be SoPlex as we can set arbitrary precision and utilize its iterative refinement capability. But I went with
cbc
because it ships withidaes get-extensions
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FYI, for testing at least we do have the AMPL version of SCIP available now. For DegeneracyHunter, I made the solver a user argument with SCIP as the default.