-
Notifications
You must be signed in to change notification settings - Fork 234
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
Support calcuation of critical properties of mixtures in Modular Properties #1336
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1336 +/- ##
==========================================
+ Coverage 77.43% 77.50% +0.06%
==========================================
Files 390 390
Lines 63758 63884 +126
Branches 11737 11756 +19
==========================================
+ Hits 49373 49514 +141
+ Misses 11841 11799 -42
- Partials 2544 2571 +27 ☔ View full report in Codecov by Sentry. |
Most of this PR is good, but I think that adding critical properties to the ideal EoS is fundamentally misguided. There's no point at which the vapor and liquid phases become identical for the ideal EoS. I looked up "Kay's Rule" for calculating T_crit and P_crit for mixtures, and it appears to be used for estimating mixture critical properties to use for calculating Z in a corresponding states chart. For the ideal EoS Z=1 if we have a vapor and Z=0 if we have a liquid, there is no middle ground. |
@dallan-keylogic This should be ready for review. I've removed the ideal critical properties and added the cubics. |
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.
The implementation looks largely fine, though I'm wondering why we're maintaining two separate initialization routines for the modular properties. It probably has to do with the old method of initialization vs the new method of initialization, but having one method call the other or extracting the common code into a function would be much cleaner. That impacts this PR only incidentally, though (where the same typos/stylistic choices were made in both parallel initialization methods).
Potentially more critically, we have four critical properties the user can provide, but they're related through an equation. Thus we have only three degrees of freedom. The user could, potentially, provide inconsistent data. The answer is probably "the user is responsible for understanding which properties are used by their EoS and for providing consistent data", but I thought I should raise the issue.
docs/explanations/components/property_package/general/phase_def.rst
Outdated
Show resolved
Hide resolved
docs/explanations/components/property_package/general/phase_def.rst
Outdated
Show resolved
Hide resolved
docs/explanations/components/property_package/general/phase_def.rst
Outdated
Show resolved
Hide resolved
@@ -246,6 +246,7 @@ def build(self): | |||
|
|||
# Create Vars for common parameters | |||
var_dict = { | |||
"compress_fact_crit": pyunits.dimensionless, | |||
"dens_mol_crit": base_units.DENSITY_MOLE, | |||
"omega": pyunits.dimensionless, |
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'd be nice to one day rename this "acentric factor", but that can wait for another day.
"compress_fact_crit": 1, | ||
"dens_mol_crit": 55, | ||
"pressure_crit": 1e5, | ||
"temperature_crit": 500, |
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.
This is a good example of the perils of inconsistent data. We say Z_crit = 1, but P_crit/(rho_mol_critRT_crit) ~= 0.4376. It probably doesn't matter for this test, though.
idaes/models/properties/modular_properties/base/generic_property.py
Outdated
Show resolved
Hide resolved
idaes/models/properties/modular_properties/base/generic_property.py
Outdated
Show resolved
Hide resolved
elif self.params.get_phase(p).is_vapor_phase(): | ||
vap_phase = p |
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.
Perhaps initialize vap_phase
as None
, then if you find a second vapor phase have a warning/exception?
def rule_A_crit(m): | ||
am_crit = getattr(m, "am_crit") | ||
return EoS_param[ctype]["omegaA"] * ( | ||
Cubic.gas_constant(m) * m.temperature_crit | ||
) ** 2 == (am_crit * m.pressure_crit) | ||
|
||
m.add_component("A_crit", Constraint(rule=rule_A_crit)) | ||
|
||
def rule_B_crit(m): | ||
bm_crit = getattr(m, "bm_crit") | ||
return EoS_param[ctype]["coeff_b"] * Cubic.gas_constant( | ||
m | ||
) * m.temperature_crit == (bm_crit * m.pressure_crit) | ||
|
||
m.add_component("B_crit", Constraint(rule=rule_B_crit)) |
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.
Can't these be Expressions
? Or is the idea to avoid divisions.
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.
No, these are the actual constraints we need to solve to find T_c and P_c.
m.params = GenericParameterBlock( | ||
components={ | ||
"a": { | ||
"parameter_data": { | ||
"omega": 0.1, | ||
"compress_fact_crit": 1, | ||
"dens_mol_crit": 1, | ||
"pressure_crit": 1e5, | ||
"temperature_crit": 100, | ||
} | ||
}, | ||
"b": { | ||
"parameter_data": { | ||
"omega": 0.2, | ||
"compress_fact_crit": 1, | ||
"dens_mol_crit": 1, | ||
"pressure_crit": 2e5, | ||
"temperature_crit": 200, | ||
} | ||
}, | ||
"c": { | ||
"parameter_data": { | ||
"omega": 0.3, | ||
"compress_fact_crit": 1, | ||
"dens_mol_crit": 1, | ||
"pressure_crit": 3e5, | ||
"temperature_crit": 300, | ||
} | ||
}, | ||
}, |
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.
Here we might start to run into problems with the critical properties not being consistent. Knowing how the cubic EoS works, I imagine what happens is that T_crit and P_crit influence the mixture critical properties while rho_mol_crit and Z_crit do not.
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.
Z_crit and dens_mol_crit are only used for initialization (at this point) so it won't really matter too much.
Regarding having the two initialization routines, it is backward compatibility for the older approach. As they are in separate classes and the new approach also has a few things the old one does not, I did not want one to call the other. It would be possible to have them use a single, common method for the main initialization routine, but at the time I wanted the new version to package everything it would need neatly. |
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.
LGTM
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.
LGTM
Replaces part of #977
Summary/Motivation:
This PR adds support for calculating the critical properties of mixtures in the Modular Properties framework, which is necessary to support the new Smooth VLE formulation. This is done by adding a new method to EoS classes which contains instructions for how to calculate the mixture critical properties appropriate to the EoS.
When dealing with systems with multiple phases/EoS, only one EoS can be used thus it is necessary to determine which EoS to use. This PR applies the following steps:
Changes proposed in this PR:
MOLAR_VOLUME
unit.compress_fact_crit
anddens_mol_crit
to the standard property set.build_critical_properties
method toEoSBase
.build_critical_properties
method toIdealEoS
. Cubic equivalent WIP.Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: