Skip to content
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.

Sign up for GitHub

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

Fix some bugs in the modular properties implementation #1425

Merged
merged 3 commits into from
Jun 3, 2024
Merged

Fix some bugs in the modular properties implementation #1425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f361917
Fixing bug in modular phase equilibrium list
Jun 3, 2024
929cc16
Add regression tests
Jun 3, 2024
e17d298
Fix unrelated typo
Jun 3, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion idaes/core/util/model_serializer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -819,7 +819,7 @@ def _read_component(sd, o, wts, lookup=None, suffixes=None, root_name=None):
if isinstance(o, Suffix):
if wts.suffix_filter is None or oname in wts.suffix_filter:
suffixes[odict["__id__"]] = odict["data"] # is populated
else: # read non-sufix component data
else: # read non-suffix component data
_read_component_data(odict["data"], o, wts, lookup=lookup, suffixes=suffixes)


Expand Down
2 changes: 1 addition & 1 deletion idaes/models/properties/modular_properties/base/generic_property.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ def build(self):
j,
) in self._phase_component_set:
# Component j is in both phases, in equilibrium
pe_dict["PE" + str(counter)] = {j: (pp[0], pp[1])}
pe_dict["PE" + str(counter)] = [j, (pp[0], pp[1])]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we don't expect these objects to be mutated, wouldn't it be better to have them as tuples?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is from deeper in the framework so now would not be the time to change this. That said, a tuple might make sense here.

pe_set.append("PE" + str(counter))
counter += 1

Expand Down
36 changes: 32 additions & 4 deletions idaes/models/properties/modular_properties/base/tests/test_generic_property.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
from pyomo.util.check_units import assert_units_equivalent

from idaes.models.properties.modular_properties.base.generic_property import (
GenericParameterBlock,
GenericParameterData,
GenericStateBlock,
_initialize_critical_props,
Expand All @@ -34,18 +35,25 @@
from idaes.core import (
declare_process_block_class,
Component,
FlowsheetBlock,
Phase,
LiquidPhase,
VaporPhase,
MaterialBalanceType,
MaterialFlowBasis,
Solvent,
PhaseType as PT,
)
from idaes.core.util.exceptions import ConfigurationError, PropertyPackageError
from idaes.models.properties.modular_properties.phase_equil.henry import HenryType
from idaes.core.base.property_meta import UnitSet
from idaes.core.initialization import BlockTriangularizationInitializer

from idaes.models.properties.modular_properties.phase_equil.henry import HenryType
from idaes.models.properties.modular_properties.examples.BT_ideal import (
configuration as BTconfig,
)
from idaes.models.unit_models.flash import Flash

import idaes.logger as idaeslog


Expand Down Expand Up @@ -494,9 +502,9 @@ def test_phases_in_equilibrium(self):

assert isinstance(m.params.phase_equilibrium_list, dict)
assert m.params.phase_equilibrium_list == {
"PE1": {"a": ("p1", "p2")},
"PE2": {"b": ("p1", "p2")},
"PE3": {"c": ("p1", "p2")},
"PE1": ["a", ("p1", "p2")],
"PE2": ["b", ("p1", "p2")],
"PE3": ["c", ("p1", "p2")],
}

@pytest.mark.unit
Expand Down Expand Up @@ -1962,3 +1970,23 @@ def build_critical_properties(b, *args, **kwargs):
"Component declarations.",
):
_initialize_critical_props(m.props[1])


@pytest.mark.integration
def test_phase_component_flash():
# Regression test for issue #1423
m = ConcreteModel()
m.fs = FlowsheetBlock(dynamic=False)

m.fs.props = GenericParameterBlock(**BTconfig)

m.fs.flash = Flash(
property_package=m.fs.props,
material_balance_type=MaterialBalanceType.componentPhase,
)

assert m.fs.props.phase_equilibrium_list == {
"PE1": ["benzene", ("Vap", "Liq")],
"PE2": ["toluene", ("Vap", "Liq")],
}
assert isinstance(m.fs.flash, Flash)
4 changes: 2 additions & 2 deletions idaes/models/properties/modular_properties/base/tests/test_noncondense.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,8 +243,8 @@ def test_build(self):
assert i in ["PE1", "PE2"]

assert model.params.phase_equilibrium_list == {
"PE1": {"benzene": ("Vap", "Liq")},
"PE2": {"toluene": ("Vap", "Liq")},
"PE1": ["benzene", ("Vap", "Liq")],
"PE2": ["toluene", ("Vap", "Liq")],
}

assert model.params.pressure_ref.value == 1e5
Expand Down
4 changes: 2 additions & 2 deletions idaes/models/properties/modular_properties/base/tests/test_noncondense_PR.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,8 @@ def test_build(self):
assert i in ["PE1", "PE2"]

assert model.params.phase_equilibrium_list == {
"PE1": {"benzene": ("Vap", "Liq")},
"PE2": {"toluene": ("Vap", "Liq")},
"PE1": ["benzene", ("Vap", "Liq")],
"PE2": ["toluene", ("Vap", "Liq")],
}

assert model.params.pressure_ref.value == 1e5
Expand Down
4 changes: 2 additions & 2 deletions idaes/models/properties/modular_properties/base/tests/test_nonvap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,8 +244,8 @@ def test_build(self):
assert i in ["PE1", "PE2"]

assert model.params.phase_equilibrium_list == {
"PE1": {"benzene": ("Vap", "Liq")},
"PE2": {"toluene": ("Vap", "Liq")},
"PE1": ["benzene", ("Vap", "Liq")],
"PE2": ["toluene", ("Vap", "Liq")],
}

assert model.params.pressure_ref.value == 1e5
Expand Down
4 changes: 2 additions & 2 deletions idaes/models/properties/modular_properties/base/tests/test_nonvap_PR.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,8 @@ def test_build(self):
assert i in ["PE1", "PE2"]

assert model.params.phase_equilibrium_list == {
"PE1": {"benzene": ("Vap", "Liq")},
"PE2": {"toluene": ("Vap", "Liq")},
"PE1": ["benzene", ("Vap", "Liq")],
"PE2": ["toluene", ("Vap", "Liq")],
}

assert model.params.pressure_ref.value == 1e5
Expand Down
16 changes: 8 additions & 8 deletions idaes/models/properties/modular_properties/base/tests/test_vle.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,8 +209,8 @@ def test_build(self, model):
assert i in ["PE1", "PE2"]

assert model.params.phase_equilibrium_list == {
"PE1": {"A": ("Vap", "Liq")},
"PE2": {"B": ("Vap", "Liq")},
"PE1": ["A", ("Vap", "Liq")],
"PE2": ["B", ("Vap", "Liq")],
}

assert model.params.pressure_ref.value == 1e5
Expand Down Expand Up @@ -513,9 +513,9 @@ def test_build(self, model):
assert i in ["PE1", "PE2", "PE3"]

assert model.params.phase_equilibrium_list == {
"PE1": {"A": ("Vap", "Liq")},
"PE2": {"B": ("Vap", "Liq")},
"PE3": {"C": ("Vap", "Liq")},
"PE1": ["A", ("Vap", "Liq")],
"PE2": ["B", ("Vap", "Liq")],
"PE3": ["C", ("Vap", "Liq")],
}

assert model.params.pressure_ref.value == 1e5
Expand Down Expand Up @@ -692,9 +692,9 @@ def test_build(self, model):
assert i in ["PE1", "PE2", "PE3"]

assert model.params.phase_equilibrium_list == {
"PE1": {"A": ("Vap", "Liq")},
"PE2": {"B": ("Vap", "Liq")},
"PE3": {"C": ("Vap", "Liq")},
"PE1": ["A", ("Vap", "Liq")],
"PE2": ["B", ("Vap", "Liq")],
"PE3": ["C", ("Vap", "Liq")],
}

assert model.params.pressure_ref.value == 1e5
Expand Down
6 changes: 3 additions & 3 deletions idaes/models/properties/modular_properties/examples/tests/test_ASU_PR.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,9 +121,9 @@ def test_build(self):
assert i in ["PE1", "PE2", "PE3"]

assert model.params.phase_equilibrium_list == {
"PE1": {"nitrogen": ("Vap", "Liq")},
"PE2": {"argon": ("Vap", "Liq")},
"PE3": {"oxygen": ("Vap", "Liq")},
"PE1": ["nitrogen", ("Vap", "Liq")],
"PE2": ["argon", ("Vap", "Liq")],
"PE3": ["oxygen", ("Vap", "Liq")],
}

assert model.params.pressure_ref.value == 101325
Expand Down
6 changes: 3 additions & 3 deletions idaes/models/properties/modular_properties/examples/tests/test_ASU_PR_Dowling_2015.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,9 +126,9 @@ def test_build(self):
assert i in ["PE1", "PE2", "PE3"]

assert model.params.phase_equilibrium_list == {
"PE1": {"nitrogen": ("Vap", "Liq")},
"PE2": {"argon": ("Vap", "Liq")},
"PE3": {"oxygen": ("Vap", "Liq")},
"PE1": ["nitrogen", ("Vap", "Liq")],
"PE2": ["argon", ("Vap", "Liq")],
"PE3": ["oxygen", ("Vap", "Liq")],
}

assert model.params.pressure_ref.value == 101325
Expand Down
4 changes: 2 additions & 2 deletions idaes/models/properties/modular_properties/examples/tests/test_BTIdeal.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ def test_build(self):
assert i in ["PE1", "PE2"]

assert model.params.phase_equilibrium_list == {
"PE1": {"benzene": ("Vap", "Liq")},
"PE2": {"toluene": ("Vap", "Liq")},
"PE1": ["benzene", ("Vap", "Liq")],
"PE2": ["toluene", ("Vap", "Liq")],
}

assert model.params.pressure_ref.value == 1e5
Expand Down
4 changes: 2 additions & 2 deletions idaes/models/properties/modular_properties/examples/tests/test_BTIdeal_FPhx.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,8 @@ def test_build(self):
assert i in ["PE1", "PE2"]

assert model.params.phase_equilibrium_list == {
"PE1": {"benzene": ("Vap", "Liq")},
"PE2": {"toluene": ("Vap", "Liq")},
"PE1": ["benzene", ("Vap", "Liq")],
"PE2": ["toluene", ("Vap", "Liq")],
}

assert model.params.pressure_ref.value == 1e5
Expand Down
4 changes: 2 additions & 2 deletions idaes/models/properties/modular_properties/examples/tests/test_BTIdeal_FcPh.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,8 +237,8 @@ def test_build(self):
assert i in ["PE1", "PE2"]

assert model.params.phase_equilibrium_list == {
"PE1": {"benzene": ("Vap", "Liq")},
"PE2": {"toluene": ("Vap", "Liq")},
"PE1": ["benzene", ("Vap", "Liq")],
"PE2": ["toluene", ("Vap", "Liq")],
}

assert model.params.pressure_ref.value == 1e5
Expand Down
4 changes: 2 additions & 2 deletions idaes/models/properties/modular_properties/examples/tests/test_BTIdeal_FcTP.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,8 +234,8 @@ def test_build(self):
assert i in ["PE1", "PE2"]

assert model.params.phase_equilibrium_list == {
"PE1": {"benzene": ("Vap", "Liq")},
"PE2": {"toluene": ("Vap", "Liq")},
"PE1": ["benzene", ("Vap", "Liq")],
"PE2": ["toluene", ("Vap", "Liq")],
}

assert model.params.pressure_ref.value == 1e5
Expand Down
2 changes: 1 addition & 1 deletion idaes/models/properties/modular_properties/examples/tests/test_CO2_H2O_Ideal_VLE.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ def test_build(self):
for i in model.params.phase_equilibrium_idx:
assert i in ["PE1"]

assert model.params.phase_equilibrium_list == {"PE1": {"H2O": ("Vap", "Liq")}}
assert model.params.phase_equilibrium_list == {"PE1": ["H2O", ("Vap", "Liq")]}

assert model.params.pressure_ref.value == 101325
assert model.params.temperature_ref.value == 298.15
Expand Down
2 changes: 1 addition & 1 deletion idaes/models/properties/modular_properties/examples/tests/test_CO2_bmimPF6_PR.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def test_build(self):
assert i in ["PE1"]

assert model.param.phase_equilibrium_list == {
"PE1": {"carbon_dioxide": ("Vap", "Liq")}
"PE1": ["carbon_dioxide", ("Vap", "Liq")]
}

assert model.param.pressure_ref.value == 101325
Expand Down
22 changes: 11 additions & 11 deletions idaes/models/properties/modular_properties/examples/tests/test_HC_PR.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,17 +197,17 @@ def test_build(self):
]

assert model.params.phase_equilibrium_list == {
"PE1": {"ethane": ("Vap", "Liq")},
"PE2": {"propane": ("Vap", "Liq")},
"PE3": {"nbutane": ("Vap", "Liq")},
"PE4": {"ibutane": ("Vap", "Liq")},
"PE5": {"ethylene": ("Vap", "Liq")},
"PE6": {"propene": ("Vap", "Liq")},
"PE7": {"butene": ("Vap", "Liq")},
"PE8": {"pentene": ("Vap", "Liq")},
"PE9": {"hexene": ("Vap", "Liq")},
"PE10": {"heptene": ("Vap", "Liq")},
"PE11": {"octene": ("Vap", "Liq")},
"PE1": ["ethane", ("Vap", "Liq")],
"PE2": ["propane", ("Vap", "Liq")],
"PE3": ["nbutane", ("Vap", "Liq")],
"PE4": ["ibutane", ("Vap", "Liq")],
"PE5": ["ethylene", ("Vap", "Liq")],
"PE6": ["propene", ("Vap", "Liq")],
"PE7": ["butene", ("Vap", "Liq")],
"PE8": ["pentene", ("Vap", "Liq")],
"PE9": ["hexene", ("Vap", "Liq")],
"PE10": ["heptene", ("Vap", "Liq")],
"PE11": ["octene", ("Vap", "Liq")],
}

assert model.params.pressure_ref.value == 101325
Expand Down
6 changes: 3 additions & 3 deletions idaes/models/properties/modular_properties/state_definitions/FTPx.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,8 @@ def state_initialization(b):
K = None
break

# Default is no initialization of VLE
vap_frac = None
Comment on lines +472 to +473
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change because of PyLint?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No - this was from issue #1424. Later steps expect vap_frac to exist, so this is making sure it will exist no matter what course gets taken in the if/else block.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I see. I looked at the different branches of the inner if statement, but forgot it was nested inside an outer if statement.

if init_VLE:
raoult_init = False
if tdew is not None and b.temperature.value > tdew:
Expand All @@ -490,9 +492,7 @@ def state_initialization(b):
l_only_comps,
v_only_comps + henry_conc + henry_other,
)
else:
# No way to estimate phase fraction
vap_frac = None
# else: No way to estimate phase fraction, do nothing

if vap_frac is not None:
b.phase_frac[v_phase] = vap_frac
Expand Down
Loading
Loading