Scaler object for Helmholtz Properties #1688
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## scaling_toolbox #1688 +/- ##
===================================================
+ Coverage 77.33% 77.36% +0.02%
===================================================
Files 395 395
Lines 64358 64390 +32
Branches 10798 10808 +10
===================================================
+ Hits 49774 49813 +39
+ Misses 12081 12077 -4
+ Partials 2503 2500 -3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| assert sb.scaling_hint[sb.dens_mol_phase["Vap"]] == 1 | ||
| assert sb.scaling_hint[sb.energy_internal_mass] == 1e-3 | ||
| assert sb.scaling_hint[sb.flow_vol] == pytest.approx( | ||
| 1 / (137 * 18.015 / 1000 * pyo.sqrt(5 * 1000)), rel=1e-3 |
There was a problem hiding this comment.
This is very specific, what is this from?
There was a problem hiding this comment.
1 / 137 is the scaling factor for the molar flow rate. 18.015/1000 is the molar mass of water in kg. The density of steam at ~ 1 atm and 100 C is ~ 5 kg/m^3, and the density of water is ~1000 kg/m^3. We take the geometric mean of those density values because we don't know which phase is going to be present.
| model.flow_vol, 1 / (nom_flow_mass * nom_dens_mass), overwrite=overwrite | ||
| ) | ||
|
|
||
| for varname in self.default_scaling_factors.keys(): |
There was a problem hiding this comment.
So it seems like this for loop would only set the scaling factor for flow_vol if the code in Lines 140-141 hasn't already set it. Is that correct? And do we want to give priority to the method above (i.e. if both methods are trying to set the scaling factor, is there a reason we should defer value given by the default scaling factor?)
There was a problem hiding this comment.
You're right, I should skip flow_vol in this loop.
We want to defer to the value given by the default scaling factor because we don't know which phase the user expects (therefore we use the geometric mean of vapor and liquid densities) while the user, hopefully, knows which phase they expect and can choose an appropriate scaling factor. If the user has a unit operation that expects a vapor phase and encounters a liquid phase, or one that expects a liquid phase and encounters a vapor phase, they should expect their performance equations to go haywire anyway. That's partially why cavitation in pumps and droplets in turbines are such a big deal.
| import re | ||
|
|
There was a problem hiding this comment.
Seems like there's no comparison of the condition number before vs after scaling. Is this intentional?
There was a problem hiding this comment.
Yes. There are zero constraints most of the time, and with TPX state variables there is only a single active constraint, so the condition number is trivially 1.
| StateVars, | ||
| ) | ||
| from idaes.core import FlowsheetBlock | ||
| from idaes.core.util.scaling import get_jacobian, jacobian_cond |
There was a problem hiding this comment.
These imports aren't needed. I'm assuming you accidentally left them while looking into my former comment
dallan-keylogic
merged commit 47f5f84
into
IDAES:scaling_toolbox
3 participants
Summary/Motivation:
Adds a Scaler object for the Helmholtz properties.
Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: