Fixing spurious DoF in Gibbs reactor model #1393
Conversation
andrewlee94
added
bug
| if b.control_volume.properties_out[ | ||
| t | ||
| ].config.parameters.element_comp[j][e] | ||
| != 0 |
There was a problem hiding this comment.
Should we use 0, or a very small value? Does the Pyomo solver writer specifically check for "exactly 0" values and filter those variables out?
There was a problem hiding this comment.
This is only for exact zero (and a fixed value at that). The issue is that the solver writer is filtering these out for us, but when we use the degrees_of_freedom function we still see these (and hence see spurious DoF).
This is to catch cases where the multiplier is a constant of 0 and filter them out of the expression.
There was a problem hiding this comment.
I also just worked out a better solution to remove the unneeded multipliers entirely (so that they don't end up as unused variables).
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1393 +/- ##
==========================================
- Coverage 77.63% 77.62% -0.01%
==========================================
Files 391 391
Lines 64375 64386 +11
Branches 14257 14262 +5
==========================================
+ Hits 49978 49981 +3
- Misses 11827 11834 +7
- Partials 2570 2571 +1 ☔ View full report in Codecov by Sentry. |
| for j in self.control_volume.properties_in.component_list: | ||
| if j in self.config.inert_species: | ||
| continue | ||
| elif self.control_volume.properties_out.params.element_comp[j][e] != 0: |
There was a problem hiding this comment.
AttributeError: '_IndexedGenericStateBlock' object has no attribute 'params' is thrown here. I believe a time index is missing, properties_out[0.0],params should be there instead of properties_out.params
There was a problem hiding this comment.
This works for me, and I have created a test specifically for your use case to be sure.
Could you check what version of IDAES you are using? I fixed this issue a while ago and StateBlocks now support .params.
There was a problem hiding this comment.
I am using 2.2.0 dev of IDAES
There was a problem hiding this comment.
You need to update your version of IDAES as you are at least two full releases behind.
There was a problem hiding this comment.
This works fine with the latest version. Looks good.
Fixes #1392
Summary/Motivation:
The current implementation of the Gibbs reactor model includes all elements in the Lagrange multipliers expressions, even if the elemental composition for a given component is 0. This leads to spurious degrees of freedom where the variable appears to be in active constraints but is multiplied y zero (and thus filtered out in the Pyomo solver writers).
Changes proposed in this PR:
Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: