Adding diagnostics tools for ill conditioning #1299

andrewlee94 · 2023-12-08T16:12:24Z

Fixes None

Summary/Motivation:

This PR adds new diagnostics capabilities for detecting near-parallel variables and constraints and for finding sources of ill conditioning in models (similar to DegeneracyHunter).

Changes proposed in this PR:

Adds method to check for near parallel rows and columns in Jacobians
Add hook for checking for parallel rows and columns in DiagnosticsToolbox
Adds beta method to analyze Jacobian for near singularity based on rows or columns (with logger warning about future changes)

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

…nditioning2

bknueven

I left a few comments while my experience with the ill conditioning tool is still fresh.

idaes/core/util/model_diagnostics.py

Co-authored-by: bknueven <[email protected]>

codecov · 2023-12-08T19:19:56Z

Codecov Report

Attention: 16 lines in your changes are missing coverage. Please review.

Comparison is base (7ee5489) 77.41% compared to head (cc51ef0) 77.43%.

Files	Patch %	Lines
idaes/core/util/model_diagnostics.py	85.45%	6 Missing and 10 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1299      +/-   ##
==========================================
+ Coverage   77.41%   77.43%   +0.01%     
==========================================
  Files         390      390              
  Lines       63649    63758     +109     
  Branches    11705    11737      +32     
==========================================
+ Hits        49276    49370      +94     
- Misses      11835    11845      +10     
- Partials     2538     2543       +5

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

bknueven

The below set of changes avoids cubic time complexity in the check_ill_conditioning model construction.

idaes/core/util/model_diagnostics.py

Co-authored-by: bknueven <[email protected]>

…ctors

Robbybp

A few small comments, then I've opened a PR into this branch where I do the following:

Clean up variable names in some of the code I wrote for the parallel vector checker
Implement a relative tolerance check for parallel vectors.

The PR is here: andrewlee94#19. Let me know what you think about the changes.

idaes/core/util/model_diagnostics.py

idaes/core/util/tests/test_model_diagnostics.py

Use a relative tolerance when checking parallel rows/cols

build ill conditioning problem iterating over non-zeros

andrewlee94 · 2023-12-13T21:14:05Z

@Robbybp @bknueven Do you think this PR is ready to merge now, or should we hold off? I will be out of the country from the end of the week until basically the beginning of February.

Robbybp

A few minor comments. These do not necessarily need to happen now, as this is beta functionality, but I would like them to be considered before this is made more permanent. In addition, I think a more permanent implementation should be more configurable to incorporate a DegeneracyHunter-like option.

Robbybp · 2023-12-13T22:02:48Z

idaes/core/util/model_diagnostics.py

+    return parallel
+
+
+def check_ill_conditioning(


I think this name is a too vague, as there are many different ways of checking for ill-conditioning. Maybe compute_ill_conditioning_certificate or compute_worst_conditioned_subset?

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.

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?

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.

Robbybp · 2023-12-13T22:03:30Z

idaes/core/util/model_diagnostics.py

+        jac = jac.tocsr()
+
+    # Build test problem
+    inf_prob = ConcreteModel()


Why is this called inf_prob?

probably should just go with m?

I like m. I'm fine with another name too, I just don't know what "inf" is.

"Infeasibility" of the linear dependence constraint?

Robbybp · 2023-12-13T22:12:02Z

idaes/core/util/model_diagnostics.py

+    solver.options["primalT"] = target_feasibility_tol * 1e-1
+    solver.options["dualT"] = target_feasibility_tol * 1e-1
+
+    results = solver.solve(inf_prob, tee=False)


This does not have to happen now, but I generally like splitting these types of functions into two functions: One that simply builds the model, and one that just builds the model (by calling the first function), then solves the model. That way an advanced user can build the model and inspect it, apply a transformation, or use a custom solver. It also makes it easier to see what the default solver and options are.

I'm open to arguments that this is a bad idea, and again, this does not need to happen now.

We actually solve two problems at the moment -- we minimize the norm of the RHS, then we fix the norm of the RHS and minimize the 1-norm of the vector. That makes a cleaner workflow more difficult.

I also imagine eventually trying to re-solve with tighter tolerances if the answer is nonsensical, e.g., the norm of the RHS is negative. These things can be done automatically much more straightforwardly if we pick a single solver.

I did not realize that we solve two problems. I see the difficulty in splitting this up in that case.

Robbybp · 2023-12-13T22:18:30Z

idaes/core/util/model_diagnostics.py

+    # Objective -- minimize residual
+    inf_prob.min_res = Objective(expr=inf_prob.res_norm)
+
+    solver = SolverFactory("cbc")  # TODO: Consider making this an option


It would be nice to default to HiGHS or SCIP, although I understand if that can't happen for the time being.

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 with idaes get-extensions.

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.

Robbybp

Other than the name of check_ill_conditioning, I think this is ready.

I'd still like to see this merged with DegeneracyHunter eventually, because I think having two different optimization-based degeneracy/ill-conditioning detectors will be confusing for users. This does not have to hold up this PR.

Robbybp · 2024-01-25T18:34:58Z

idaes/core/util/model_diagnostics.py

+    return parallel
+
+
+def check_ill_conditioning(


I think check_ill_conditioning is too vague of a name here. I suggest compute_ill_conditioning_certificate

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.

I've updated the name to compute_ill_conditioning_certificate

Robbybp

Looks good

andrewlee94 · 2024-01-26T14:34:59Z

@bknueven Would you have time to review this? I think it is just waiting on a second reviewer.

andrewlee94 added 4 commits December 7, 2023 10:36

Initial version of ill conditioning and angle checks

0942f99

Adding tests for ill conditioning checks

b1e5c9b

Merge branch 'main' of https://github.com/IDAES/idaes-pse into ill_co…

ba6c08c

…nditioning2

Adding acknowledgements and test for ok reports

34d942d

andrewlee94 changed the title ~~[WIP] Adding diagnsotics tools for ill conditioning~~ [WIP] Adding diagnostics tools for ill conditioning Dec 8, 2023

andrewlee94 requested review from Robbybp, bknueven, adowling2 and jsiirola December 8, 2023 16:12

andrewlee94 self-assigned this Dec 8, 2023

andrewlee94 added enhancement New feature or request Priority:Normal Normal Priority Issue or PR core Issues dealing with core modeling components WIP diagnostics labels Dec 8, 2023

andrewlee94 added 2 commits December 8, 2023 11:21

Fixing typo

b637be8

Adding tolerance to ill conditioning tests

48a1557

bknueven reviewed Dec 8, 2023

View reviewed changes

andrewlee94 and others added 2 commits December 8, 2023 12:56

Apply suggestions from code review

3349904

Co-authored-by: bknueven <[email protected]>

Fixing minor bug in suggestions

c1d9db6

bknueven reviewed Dec 11, 2023

View reviewed changes

idaes/core/util/model_diagnostics.py Outdated Show resolved Hide resolved

idaes/core/util/model_diagnostics.py Outdated Show resolved Hide resolved

idaes/core/util/model_diagnostics.py Outdated Show resolved Hide resolved

idaes/core/util/model_diagnostics.py Outdated Show resolved Hide resolved

andrewlee94 and others added 3 commits December 11, 2023 11:52

Apply suggestions from code review

181c5e0

Co-authored-by: bknueven <[email protected]>

use more descriptive names and use relative tolerance for parallel ve…

7b2c2ac

…ctors

update tests for parallel-checker that uses relative tolerance

85eb74e

Robbybp reviewed Dec 11, 2023

View reviewed changes

idaes/core/util/model_diagnostics.py Outdated Show resolved Hide resolved

idaes/core/util/tests/test_model_diagnostics.py Outdated Show resolved Hide resolved

idaes/core/util/tests/test_model_diagnostics.py Show resolved Hide resolved

andrewlee94 and others added 2 commits December 11, 2023 15:11

Merge pull request #19 from Robbybp/ill_conditioning2-reltol

db228a3

Use a relative tolerance when checking parallel rows/cols

Addressing reviews

447055c

andrewlee94 marked this pull request as ready for review December 11, 2023 20:26

andrewlee94 requested a review from eslickj as a code owner December 11, 2023 20:26

andrewlee94 requested a review from lbianchi-lbl as a code owner December 11, 2023 20:26

build ill conditioning problem iterating over non-zeros

b0d39d8

bknueven mentioned this pull request Dec 11, 2023

build ill conditioning problem iterating over non-zeros andrewlee94/idaes-pse#20

Merged

bknueven and others added 2 commits December 12, 2023 15:21

return component instead of its name

5316a01

Merge pull request #20 from bknueven/ill_cond_sparse

c4c9f5d

build ill conditioning problem iterating over non-zeros

andrewlee94 changed the title ~~[WIP] Adding diagnostics tools for ill conditioning~~ Adding diagnostics tools for ill conditioning Dec 13, 2023

andrewlee94 removed the WIP label Dec 13, 2023

Robbybp reviewed Dec 13, 2023

View reviewed changes

Robbybp requested changes Jan 25, 2024

View reviewed changes

andrewlee94 added 2 commits January 25, 2024 14:12

Merging main

1e92d90

Updating name of check_ill_conditioning

f08c298

Robbybp approved these changes Jan 25, 2024

View reviewed changes

Merge branch 'main' into ill_conditioning2

cc51ef0

bknueven approved these changes Jan 26, 2024

View reviewed changes

andrewlee94 enabled auto-merge (squash) January 26, 2024 16:44

andrewlee94 merged commit 190451a into IDAES:main Jan 26, 2024
41 checks passed

andrewlee94 deleted the ill_conditioning2 branch January 26, 2024 17:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding diagnostics tools for ill conditioning #1299

Adding diagnostics tools for ill conditioning #1299

andrewlee94 commented Dec 8, 2023

bknueven left a comment

codecov bot commented Dec 8, 2023 •

edited

Loading

bknueven left a comment

Robbybp left a comment

andrewlee94 commented Dec 13, 2023

Robbybp left a comment

Robbybp Dec 13, 2023

bknueven Dec 13, 2023

Robbybp Dec 13, 2023

bknueven Dec 14, 2023

Robbybp Dec 13, 2023

bknueven Dec 13, 2023

Robbybp Dec 13, 2023

Robbybp Dec 13, 2023

Robbybp Dec 13, 2023

bknueven Dec 13, 2023

Robbybp Dec 13, 2023

Robbybp Dec 13, 2023

bknueven Dec 13, 2023

andrewlee94 Dec 14, 2023

Robbybp left a comment

Robbybp Jan 25, 2024

bknueven Jan 25, 2024

andrewlee94 Jan 25, 2024

Robbybp left a comment

andrewlee94 commented Jan 26, 2024

Adding diagnostics tools for ill conditioning #1299

Adding diagnostics tools for ill conditioning #1299

Conversation

andrewlee94 commented Dec 8, 2023

Fixes None

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

bknueven left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 8, 2023 • edited Loading

Codecov Report

bknueven left a comment

Choose a reason for hiding this comment

Robbybp left a comment

Choose a reason for hiding this comment

andrewlee94 commented Dec 13, 2023

Robbybp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Robbybp left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Robbybp left a comment

Choose a reason for hiding this comment

andrewlee94 commented Jan 26, 2024

codecov bot commented Dec 8, 2023 •

edited

Loading