From deacf4c422bc9e50cb347e11a8cbfa0195bd4274 Mon Sep 17 00:00:00 2001 From: John Eslick Date: Thu, 3 Aug 2023 23:19:29 -0500 Subject: [PATCH] Add properties and minor fixes for property sets (#1230) * Update property set and checkes * Add saturation index * Add ionic strength * Run black * Pylint fixes * Revert pytest.ini * Fix index check * Revert pytest.ini * Move properties to electrolyte * Move saturation index * Fix tests * Fix pylint errors * Add act_coeff * Fix a couple properties * Add Prandtl number * Fix pylint errors --- idaes/core/base/property_meta.py | 18 +- idaes/core/base/property_set.py | 198 +++++++++++------- idaes/core/base/tests/test_property_set.py | 25 +-- .../general_helmholtz/helmholtz_functions.py | 2 +- 4 files changed, 147 insertions(+), 96 deletions(-) diff --git a/idaes/core/base/property_meta.py b/idaes/core/base/property_meta.py index 2d120a60ae..b12aee115c 100644 --- a/idaes/core/base/property_meta.py +++ b/idaes/core/base/property_meta.py @@ -51,6 +51,7 @@ def define_metadata(cls, meta): from pyomo.environ import units from pyomo.core.base.units_container import _PyomoUnit, InconsistentUnitsError +from pyomo.common.deprecation import deprecation_warning from idaes.core.util.exceptions import PropertyPackageError from idaes.core.base.property_set import StandardPropertySet, PropertySetBase @@ -519,7 +520,22 @@ def add_properties(self, p: dict): for k, v in p.items(): units = v.pop("units", None) try: - n, i = self._properties.get_name_and_index(k) + try: + n, i = self._properties.get_name_and_index(k) + except ValueError: + msg = ( + f"The property name {k} in property metadata is not a recognized " + "standard property name defined in this PropertySet. Please refer " + "to IDAES standard names in the IDAES documentation. You can use " + "the define_custom_properties() rather than the add_properties() " + "method to define metadata for this property. You can also use a " + "different property set by calling the define_property_set() method." + ) + deprecation_warning( + msg=msg, logger=_log, version="2.0.0", remove_in="3.0.0" + ) + n = k + i = None getattr(self._properties, n)[i].update_property(**v) except AttributeError: # TODO: Deprecate this and make it raise an exception if an unknown property is encountered diff --git a/idaes/core/base/property_set.py b/idaes/core/base/property_set.py index 2140ec419d..70af3f584f 100644 --- a/idaes/core/base/property_set.py +++ b/idaes/core/base/property_set.py @@ -40,25 +40,25 @@ def __init__( required: bool = False, valid_range=None, ): - super().__setattr__("_parent", parent) - super().__setattr__("_idx", idx) - super().__setattr__("_method", method) - super().__setattr__("_supported", supported) - super().__setattr__("_required", required) - - super().__setattr__("_valid_range", None) + self._parent = parent + self._idx = idx + self._method = method + self._supported = supported + self._required = required + self._valid_range = None self._set_valid_range( valid_range ) # This method does some basic validation of input - + self._lock_setattr = True # TODO: For future, this would be the place to store default scaling information, etc. # TODO: Could also define default bounds, nominal values, etc. def __setattr__(self, key, value): - raise TypeError("Property metadata does not support assignment.") + if hasattr(self, "_lock_setattr") and self._lock_setattr is True: + raise TypeError("Property metadata does not support assignment.") + super().__setattr__(key, value) def __repr__(self): - # pylint: disable=E1101 return f"{self._parent._doc} ({self._parent._units}%s%s)" % ( "supported" if self._supported else "", "required" if self._required else "", @@ -70,9 +70,9 @@ def name(self): Name of sub-property as a string """ suffix = "" - if self._idx is not "none": # pylint: disable=E1101 - suffix = f"_{self._idx}" # pylint: disable=E1101 - return f"{self._parent.name}{suffix}" # pylint: disable=E1101 + if self._idx != "none": + suffix = f"_{self._idx}" + return f"{self._parent.name}{suffix}" @property def method(self): @@ -80,7 +80,7 @@ def method(self): Reference to method that can be called to construct this property and associated constraints if using build-on-demand approach. """ - return self._method # pylint: disable=E1101 + return self._method @property def units(self): @@ -88,14 +88,14 @@ def units(self): Units of measurement for this property. This should be a reference to a quantity defined in the UnitSet associated with this property package. """ - return self._parent.units # pylint: disable=E1101 + return self._parent.units @property def supported(self): """ Bool indicating whether this property package supports calculation of this property. """ - return self._supported # pylint: disable=E1101 + return self._supported @property def required(self): @@ -106,7 +106,7 @@ def required(self): This is most commonly used by reaction packages which rely of thermophysical property packages to define other properties. """ - return self._required # pylint: disable=E1101 + return self._required @property def valid_range(self): @@ -119,7 +119,7 @@ def valid_range(self): It is strongly recommended that developers set valid ranges for all state variables in a property package, as well as any other key properties. """ - return self._valid_range # pylint: disable=E1101 + return self._valid_range def set_method(self, meth: str): """ @@ -276,14 +276,16 @@ def __init__( if doc is None: doc = name - super().__setattr__("_name", name) - super().__setattr__("_doc", doc) + self._name = name + self._doc = doc # TODO: Validate units are from UnitSet or dimensionless - needs deprecation - super().__setattr__("_units", units) + self._units = units # Record indices - needed for building instance specific cases - super().__setattr__("_indices", indices) + self._indices = indices + + self._lock_setattr = True # Create entries for indexed sub-properties if indices is None or indices is False: @@ -303,21 +305,22 @@ def __getitem__(self, key: str): try: return getattr(self, "_" + key) except AttributeError: - # pylint: disable=E1101 raise KeyError(f"Property {self._name} does not have index {key}.") def __repr__(self): - return f"{self._doc} ({self._units})" # pylint: disable=E1101 + return f"{self._doc} ({self._units})" def __setattr__(self, key, value): - raise TypeError("Property metadata does not support assignment.") + if hasattr(self, "_lock_setattr") and self._lock_setattr is True: + raise TypeError("Property metadata does not support assignment.") + super().__setattr__(key, value) @property def name(self): """ Doc string for property """ - return self._name # pylint: disable=E1101 + return self._name @property def units(self): @@ -325,7 +328,7 @@ def units(self): Units of measurement for this property. This should be a reference to a quantity defined in the UnitSet associated with this property package. """ - return self._units # pylint: disable=E1101 + return self._units # TODO: An overall update method across multiple indices? @@ -341,10 +344,10 @@ class PropertySetBase: _defined_indices = ["comp", "phase", "phase_comp"] def __init__(self, parent): - super().__setattr__("_parent_block", parent) - super().__setattr__("_defined_properties", []) - super().__setattr__("_defined_indices", copy(self.__class__._defined_indices)) - + self._parent_block = parent + self._defined_properties = [] + self._defined_indices = copy(self.__class__._defined_indices) + self._lock_setattr = True # Find standard properties defined in class and create instance versions for i in dir(self.__class__): if not i.startswith("_"): @@ -354,7 +357,6 @@ def __init__(self, parent): units = iobj.units # Check if units is a placeholder string and update if required if isinstance(units, str): - # pylint: disable=E1101 units = getattr(self._parent_block.default_units, units) self._add_property( name=i, @@ -364,9 +366,11 @@ def __init__(self, parent): ) def __setattr__(self, key, value): - raise TypeError( - "PropertySets do not support direct assignment. Please use define_property" - ) + if hasattr(self, "_lock_setattr") and self._lock_setattr is True: + raise TypeError( + "PropertySets do not support direct assignment. Please use define_property" + ) + super().__setattr__(key, value) def __getitem__(self, key: str): n, i = self.get_name_and_index(key) @@ -376,7 +380,7 @@ def __getitem__(self, key: str): raise KeyError(f"Property {key} is not defined in this PropertySet.") def __iter__(self): - for a in self._defined_properties: # pylint: disable=E1101 + for a in self._defined_properties: yield getattr(self, a) def define_property( @@ -480,7 +484,7 @@ def _add_property( initialize=initialize, ), ) - self._defined_properties.append(name) # pylint: disable=E1101 + self._defined_properties.append(name) def check_required_properties(self, other: "PropertySetBase"): """ @@ -493,7 +497,7 @@ def check_required_properties(self, other: "PropertySetBase"): list of properties required by this package which are not supported by other package """ unsupported = [] - for a in self._defined_properties: # pylint: disable=E1101 + for a in self._defined_properties: aobj = getattr(self, a) # TODO: Should refactor parent so this is not private # pylint: disable-next=protected-access @@ -560,7 +564,7 @@ def unitset(self): """ Reference to UnitSet associated with this PropertySet (via the parent metadata object). """ - return self._parent_block.default_units # pylint: disable=E1101 + return self._parent_block.default_units def get_name_and_index(self, property_name: str): """ @@ -574,40 +578,28 @@ def get_name_and_index(self, property_name: str): Returns: name, index: strings indicating the name of the base property and indexing set. """ - name = None - index = None + root_name = property_name + index_name = None + _defined_indices = {"phase_comp", "phase", "comp"} | set(self._defined_indices) - sep_point = None - if property_name in self._defined_properties: # pylint: disable=E1101 - sep_point = 0 - elif "phase" in property_name and not property_name.startswith("phase"): - if "phase_comp" in property_name: - sep_point = property_name.rindex("phase_comp") - else: - sep_point = property_name.rindex("phase") - elif "comp" in property_name and not property_name.startswith("comp"): - if "phase_comp" in property_name: - sep_point = property_name.rindex("phase_comp") - else: - sep_point = property_name.rindex("comp") - else: - for i in self._defined_indices: - if i in property_name: - sep_point = property_name.rindex(i) + _defined_indices = list(reversed(sorted(_defined_indices, key=len))) - if sep_point is None: + if property_name in self._defined_properties: + return property_name, None + else: + for i in _defined_indices: + if property_name.endswith("_" + i): + nchar = len(i) + 1 + root_name = property_name[:-nchar] + index_name = i + break + + if root_name not in self._defined_properties: raise ValueError( - f"Unhandled property: {property_name}. This is mostly likely due to the " - "property not being defined in this PropertySet." + f"Unhandled property: {property_name}. This is mostly likely due to the property not being defined in this PropertySet." ) - if sep_point > 0: - name = property_name[: sep_point - 1] - index = property_name[sep_point:] - else: - name = property_name - index = None - return name, index + return root_name, index_name class StandardPropertySet(PropertySetBase): @@ -625,6 +617,11 @@ class StandardPropertySet(PropertySetBase): doc="Chemical Activity", units=pyunits.dimensionless, ) + act_coeff = PropertyMetadata( + name="act_coeff", + doc="Chemical Activity Coefficient", + units=pyunits.dimensionless, + ) compress_fact = PropertyMetadata( name="compress_fact", doc="Compressiblity Factor", @@ -680,8 +677,8 @@ class StandardPropertySet(PropertySetBase): doc="Molar Density at Critical Point", units="DENSITY_MOLE", ) - diffus_comp = PropertyMetadata( - name="diffus_comp", + diffus = PropertyMetadata( + name="diffus", doc="Diffusivity Coefficient", units="DIFFUSIVITY", ) @@ -755,13 +752,13 @@ class StandardPropertySet(PropertySetBase): doc="Specific Gibbs Energy (Molar Basis)", units="ENERGY_MOLE", ) - isentropic_speed_sound_phase = PropertyMetadata( - name="isentropic_speed_sound_phase", + isentropic_speed_sound = PropertyMetadata( + name="isentropic_speed_sound", doc="Isentropic Speed of Sound", units="VELOCITY", ) - isothermal_speed_sound_phase = PropertyMetadata( - name="isothermal_speed_sound_phase", + isothermal_speed_sound = PropertyMetadata( + name="isothermal_speed_sound", doc="Isothermal Speed of Sound", units="VELOCITY", ) @@ -796,6 +793,11 @@ class StandardPropertySet(PropertySetBase): doc="Phase Fraction", units=pyunits.dimensionless, ) + prandtl_number = PropertyMetadata( + name="prandtl_number", + doc="Prandtl Number", + units=pyunits.dimensionless, + ) pressure = PropertyMetadata( name="pressure", doc="Pressure", @@ -901,13 +903,18 @@ class StandardPropertySet(PropertySetBase): doc="Log of Activity", units=pyunits.dimensionless, ) + log_act_coeff = PropertyMetadata( + name="log_act_coeff", + doc="Log of Activity Coefficient", + units=pyunits.dimensionless, + ) log_conc_mol = PropertyMetadata( name="log_conc_mol", doc="Log of Molar Concentration", units=pyunits.dimensionless, ) log_mass_frac = PropertyMetadata( - name="log_mass_frac_phase_comp", + name="log_mass_frac", doc="Log of Mass Fractions", units=pyunits.dimensionless, ) @@ -989,7 +996,6 @@ class ElectrolytePropertySet(StandardPropertySet): "phase_comp_apparent", "phase_comp_true", ] - # Definition of additional properties required for electrolyte applications # Log terms log_act_phase_solvents = PropertyMetadata( @@ -997,3 +1003,43 @@ class ElectrolytePropertySet(StandardPropertySet): doc="Log of Activity of Solvents", units=pyunits.dimensionless, ) + ionic_strength = PropertyMetadata( + name="ionic_strength", + doc="Ionic Strength (Molality Basis)", + units="MOLALITY", + ) + pH = PropertyMetadata( + name="pH", + doc="pH, -log10(a_H+)", + units=pyunits.dimensionless, + ) + pK = PropertyMetadata( + name="pK", + doc="pK, -log10(equilibrium constant)", + units=pyunits.dimensionless, + ) + pOH = PropertyMetadata( + name="pOH", + doc="pOH, -log10(a_OH-)", + units=pyunits.dimensionless, + ) + log10_act_coeff = PropertyMetadata( + name="log10_act_coeff", + doc="Log of Activity Coefficient", + units=pyunits.dimensionless, + ) + log10_molality = PropertyMetadata( + name="log10_molality", + doc="Log of Molality", + units=pyunits.dimensionless, + ) + log10_k_eq = PropertyMetadata( + name="log10_k_eq", + doc="Log of equilibrium coefficient", + units=pyunits.dimensionless, + ) + saturation_index = PropertyMetadata( + name="saturation_index", + doc="Saturation Index = log(IAP/Ksp)", + units=pyunits.dimensionless, + ) diff --git a/idaes/core/base/tests/test_property_set.py b/idaes/core/base/tests/test_property_set.py index e487d44108..007eb54eec 100644 --- a/idaes/core/base/tests/test_property_set.py +++ b/idaes/core/base/tests/test_property_set.py @@ -89,7 +89,7 @@ def test_valid_range_verification_invalid(self): p = DummyMeta() with pytest.raises( - ValueError, match="valid_range must be a tuple of length 2 \(got foo\)" + ValueError, match=r"valid_range must be a tuple of length 2 \(got foo\)" ): meta = _PropertyMetadataIndex( parent=p, @@ -106,7 +106,7 @@ def test_valid_range_verification_length(self): with pytest.raises( ValueError, - match="valid_range must be a tuple of length 2 \(got \(1, 2, 3\)\)", + match=r"valid_range must be a tuple of length 2 \(got \(1, 2, 3\)\)", ): meta = _PropertyMetadataIndex( parent=p, @@ -123,8 +123,8 @@ def test_valid_range_verification_values(self): with pytest.raises( ValueError, - match="valid_range must be a 2-tuple with form \(lower, upper\): first value " - "was greater than second value: \(1, 0\)", + match=r"valid_range must be a 2-tuple with form \(lower, upper\): first value " + r"was greater than second value: \(1, 0\)", ): meta = _PropertyMetadataIndex( parent=p, @@ -518,7 +518,7 @@ def test_unitset(self): pset = PropertySetBase(parent=p) - return pset.unitset is p.default_units + assert pset.unitset is p.default_units @pytest.mark.unit def test_get_name_and_index(self): @@ -581,24 +581,13 @@ def test_get_name_and_index_custom(self): p.default_units = "foo" pset = PropertySetBase(parent=p) + pset._defined_properties.append("foo") pset._defined_indices.append("bar") n, i = pset.get_name_and_index("foo_bar") assert n == "foo" assert i == "bar" - n, i = pset.get_name_and_index("foo_bar_phase") - assert n == "foo_bar" - assert i == "phase" - - n, i = pset.get_name_and_index("foo_bar_comp") - assert n == "foo_bar" - assert i == "comp" - - n, i = pset.get_name_and_index("foo_bar_phase_comp") - assert n == "foo_bar" - assert i == "phase_comp" - class DerivedPropertySet(PropertySetBase): foo = PropertyMetadata(name="foo") @@ -702,7 +691,7 @@ def test_get_name_and_index(self): p = DummyMeta() p.default_units = UnitSet() - pset = PropertySetBase(parent=p) + pset = ElectrolytePropertySet(parent=p) pset._defined_properties.append("foo_bar") n, i = pset.get_name_and_index("foo_bar") diff --git a/idaes/models/properties/general_helmholtz/helmholtz_functions.py b/idaes/models/properties/general_helmholtz/helmholtz_functions.py index 91bd832084..c726d506a0 100644 --- a/idaes/models/properties/general_helmholtz/helmholtz_functions.py +++ b/idaes/models/properties/general_helmholtz/helmholtz_functions.py @@ -2395,7 +2395,7 @@ def define_metadata(cls, obj): "method": None, "units": obj.derived_units.ENTROPY_MASS, }, - "speed_sound_phase": { + "speed_sound": { "method": None, "units": obj.derived_units.VELOCITY, },