-
Notifications
You must be signed in to change notification settings - Fork 238
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
Infeasibility diagnostic tool #1409
Infeasibility diagnostic tool #1409
Conversation
The spell checker is hitting false positives on "mis" and "MIS". |
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.
Only a few minor requests, of which half are spell checker issues.
idaes/core/util/model_diagnostics.py
Outdated
ConfigValue( | ||
default=1e-6, | ||
domain=float, | ||
description="Feasiblity tolerance for idenifying infeasible constraint and bounds", |
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.
Typo: "idenifying"
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.
Also "feasiblty" I believe
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 should probably also be constraints (plural).
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.
idaes/core/util/model_diagnostics.py
Outdated
@@ -3887,6 +3930,51 @@ def b_rule(b, i): | |||
return ill_cond | |||
|
|||
|
|||
def compute_infeasibility_explanation( |
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.
Does this need to be done as a separate method, or could it just be written directly into the toolbox class?
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 agree -- I couldn't decide at what level things should be integrated. Implemented this suggestion in 40670e4.
idaes/core/util/model_diagnostics.py
Outdated
@@ -3887,6 +3930,51 @@ def b_rule(b, i): | |||
return ill_cond | |||
|
|||
|
|||
def compute_infeasibility_explanation( | |||
model, solver, tee=False, tolerance=1e-6, stream=None |
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 we should have solver=None
here, and a check for:
if solver is None:
solver = get_solver()
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.
Done in 40670e4
idaes/core/util/model_diagnostics.py
Outdated
that removing any single constraint or variable bound would result in a | ||
feasible subsystem. | ||
|
||
This function is a wrapper for the same capability in pyomo.contrib.iis.mis. |
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.
You will need to add a few exclusions to the spell-checker file as well.
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.
Done in 34668b0
@bknueven You can add exclusions for the spell checker here: https://github.com/IDAES/idaes-pse/blob/main/.github/workflows/typos.toml |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1409 +/- ##
==========================================
- Coverage 77.89% 77.88% -0.01%
==========================================
Files 394 394
Lines 65053 65069 +16
Branches 14383 14385 +2
==========================================
+ Hits 50670 50680 +10
- Misses 11793 11798 +5
- Partials 2590 2591 +1 ☔ View full report in Codecov by Sentry. |
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.
Looks good to me, thanks Ben!
Fixes #1405
Summary/Motivation:
See discussion on #1405
Changes proposed in this PR:
pyomo.contrib.iis.mis.compute_infeasibility_explanation
toidaes.core.util.model_diagnositics
(6099416)compute_infeasibility_explanation
into theDiagnosticsToolbox
(a931a77)Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: