Diagnostics report for structured flowsheet wrapper#1742
Diagnostics report for structured flowsheet wrapper#1742dangunter wants to merge 16 commits intoIDAES:mainfrom
Conversation
dallan-keylogic
left a comment
dallan-keylogic
left a comment
There was a problem hiding this comment.
We should think about how the Diagnostics Toolbox returns data. In this PR, the text stream from the Diagnostics Toolbox is parsed in order to make the data available through the GUI, but it might make sense for the DiagnosticsToolbox to return StructuralData and NumericalData classes, and then we can either write the report or make the data available in the GUI from that.
@andrewlee94 told me that he was going to open a couple of PRs breaking the diagnostics_toolbox.py file into smaller subfiles and making them compatible with the Pyomo GreyBox interface, so if we elect to change what the DiagnosticsToolbox outputs, then we should wait into those PRs are merged in.
| if "degree" in msg_lower and "freedom" in msg_lower: | ||
| category = "degrees_of_freedom" | ||
| elif "inconsistent units" in msg_lower: | ||
| category = "inconsistent_units" | ||
| elif "structural singularity" in msg_lower: | ||
| category = "structural_singularity" | ||
| elif "evaluation errors" in msg_lower: | ||
| category = "evaluation_errors" | ||
| elif "fixed to 0" in msg_lower: | ||
| category = "fixed_to_zero" | ||
| elif "unused variable" in msg_lower or "unused variables" in msg_lower: | ||
| category = "unused_variables" | ||
|
|
||
| return StructuralIssueData( | ||
| kind=kind, | ||
| category=category, | ||
| count=count, | ||
| message=body, | ||
| details=lines[1:], | ||
| ) |
There was a problem hiding this comment.
It might be better, in the long run, for the Diagnostics Toolbox to return these data objects directly and then generate a report from that rather than writing a report to a stream and parsing it after the fact.
There was a problem hiding this comment.
I agree. In that case it would make more sense to have a separate PR to make those changes, then this PR becomes much simpler -- essentially selecting which tests to run and returning the objects. @dallan-keylogic do you have any time to help with this? I need a partner in crime to at least review changes
| if line in ( | ||
| "No warnings found!", | ||
| "No cautions found!", | ||
| "If you still have issues converging your model consider:", | ||
| ): | ||
| continue |
There was a problem hiding this comment.
So does this section not get shown to the user at all?
There was a problem hiding this comment.
yeah. the idea was it wasn't really that informative
|
@dangunter As @dallan-keylogic said, I have a couple of PRs on the way, of which I will hopefully have the first up today (just waiting for final internal review). |
|
@dangunter, I've given this (and issue #1741) "Normal" priority and on the May release board. Speak up if that's not correct. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1742 +/- ##
==========================================
- Coverage 73.85% 73.77% -0.09%
==========================================
Files 406 407 +1
Lines 66081 66255 +174
Branches 11110 11136 +26
==========================================
+ Hits 48803 48878 +75
- Misses 14741 14833 +92
- Partials 2537 2544 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
4 participants
Fixes
Fixes #1741
Summary/Motivation:
Changes proposed in this PR:
Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: