Modular properties fixes and enhancements by dallan-keylogic · Pull Request #1656 · IDAES/idaes-pse

dallan-keylogic · 2025-09-03T15:45:27Z

Fixes

Modular properties initializer now correctly uses the passed log level
Updated initialization to ensure we have 0 degrees of freedom for FpcTP initialization (addressing Error in initialization of control volume out properties when initializing a unit model for the FpcTP state definition when using modular properties. #1646)
Updated initialization to ensure that the postcheck doesn't raise an error because the VLE constraints are not satisfied
VLE in Ideal equation of state now uses equilibrium temperature rather than actual temperature
RPP5 now responds to configuration about whether or not to include enthalpy of formation in enthalpy calculations
Made a change to InitializerBase to decrease the scope of try/except block (hasattr is implemented internally with a try/except block)
Removed legacy CubicEoS

Enhancements

Log fugacity in Ideal now uses log-form variables for mole fraction
Antoine equation now returns log expression directly for RPP5

Legal Acknowledgement

By contributing to this software project, I agree to the following terms and conditions for my contribution:

I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
I represent I am authorized to make the contributions and grant the license. If my employer has rights to intellectual property that includes these contributions, I represent that I have received permission to make contributions and grant the required license on behalf of that employer.

dallan-keylogic · 2025-09-03T18:14:57Z

idaes/core/initialization/initializer_base.py

        """
-        try:
+        if hasattr(model, "fix_initialization_states"):
            model.fix_initialization_states()
-        except AttributeError:
+        else:
            _log.info_high(
                f"Model {model.name} does not have a fix_initialization_states method - attempting to continue."
            )


This change revealed that there was an error in the legacy Cubic EoS initialization---the fix_initialization_states method was trying to deactivate sum_mol_frac_out, but that constraint doesn't appear to exist (at least, not with that name), which was raising an AttributeError that was swallowed in the try/except block. That illustrates why this change was necessary.

Since we said that we were going to remove the legacy CubicEoS in the May release, the straightforward solution is to just remove it now.

dallan-keylogic · 2025-09-03T18:50:57Z

idaes/models/properties/modular_properties/base/utility.py

+    if not log:
+        try:
+            mthd = c_arg.return_expression
+        except AttributeError:
+            mthd = c_arg
+    else:
+        mthd = c_arg.return_log_expression


Do we want to require the user to add a return_log_expression function to their implementation of Antoine's law?

codecov · 2025-09-03T19:30:04Z

Codecov Report

❌ Patch coverage is 80.00000% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.06%. Comparing base (d96090c) to head (d396282).
⚠️ Report is 40 commits behind head on main.

Files with missing lines	Patch %	Lines
.../models/properties/modular_properties/eos/ideal.py	74.57%	8 Missing and 7 partials ⚠️
...erties/modular_properties/base/generic_property.py	68.42%	3 Missing and 3 partials ⚠️
.../models/properties/modular_properties/pure/RPP5.py	84.61%	1 Missing and 1 partial ⚠️
...rties/modular_properties/phase_equil/bubble_dew.py	66.66%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1656      +/-   ##
==========================================
+ Coverage   76.95%   77.06%   +0.10%     
==========================================
  Files         397      395       -2     
  Lines       63580    62761     -819     
  Branches    10367    10233     -134     
==========================================
- Hits        48929    48366     -563     
+ Misses      12213    11985     -228     
+ Partials     2438     2410      -28

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

dallan-keylogic · 2025-09-03T21:08:04Z

The test failure in Examples requires a fix to be pushed to the Examples repo.

agarciadiego · 2025-09-11T13:31:33Z

idaes/models/properties/modular_properties/base/generic_property.py

+        scaler_block = [blk for blk in model.values()][0]
+
+        if hasattr(scaler_block, "inherent_equilibrium_constraint") and (
+            not scaler_block.params._electrolyte  # TODO why do we need this check?


is TODO necessary? I beleive it creates confusion on who we are talking to

Would you like me to remove the TODO or the entire comment?

agarciadiego · 2025-09-11T13:33:11Z

idaes/models/properties/modular_properties/base/generic_property.py

+            or scaler_block.params.config.state_components == StateIndex.true
+        ):
+            init_log.debug(
+                "Cannot converge inherent reaction constraints "


This warning may be confusing. When we say "they need to be solved at the control volume level," the user may not know if that is something they must now do.

agarciadiego · 2025-09-11T13:33:44Z

idaes/models/properties/modular_properties/base/generic_property.py

+                or "mole_frac_phase_comp" in k.define_state_vars()
+            ):
+                k.equilibrium_constraint.deactivate()
+            # TODO Inherent reactions with a true component basis will fail here too


Do not understant TODO message

agarciadiego · 2025-09-11T13:34:09Z

idaes/models/properties/modular_properties/base/generic_property.py

+            for idx in self.fug_phase_comp:
+                sf_x = iscale.get_scaling_factor(
+                    self.mole_frac_phase_comp[idx],
+                    default=1e3,  # I'd prefer 10, but this is consistent with existing scaling


Remove "I prefer" comment or change wording

agarciadiego · 2025-09-11T13:35:09Z

idaes/models/properties/modular_properties/eos/ideal.py

+    def log_fug_phase_comp_eq(b, p, j, pp):
+        if (p, j) not in b.phase_component_set:
+            raise KeyError(f"Component {j} is not present in phase {p}.")
+        # TODO does allowing a calculation with


Change wording on TODO message

agarciadiego · 2025-09-11T13:36:59Z

idaes/models/properties/modular_properties/eos/ideal.py

-        cobj = b.params.get_component(j)
+        if b.params.has_inherent_reactions:
+            raise PropertyNotSupportedError(
+                "Bubble/dew calculations for systems with inherent "


This is only a question. What would be required to support this?

It depends. If all we want to do is have any temperature at which we can solve the VLE equations for SmoothVLE, then we can probably just use the current system composition in the bubble/dew calculations. However, if we want temperature_bubble and temperature_dew to be the system's actual bubble and dew temperatures, we would need to write inherent reaction equilibrium equations not just at the system's temperature, but also at the bubble/dew temperature. That adds a lot of complexity and overhead, so I'd rather punt on the issue until we have a specific problem we're trying to solve.

We don't have this issue with the CubicComplementarityVLE because the equilibrium temperature there is not directly related to the bubble/dew temperatures.

Of course, we're not going to be using CubicComplementarityVLE with the Ideal EoS anyway.

agarciadiego

Just a couple of comments about commented code

dallan-keylogic added 3 commits September 3, 2025 08:47

rescue files from feature creep

104491a

not enough syntactic sugar

b6b1aa8

test initialization

b1e4af2

dallan-keylogic requested review from agarciadiego, bpaul4 and lbianchi-lbl as code owners September 3, 2025 15:45

dallan-keylogic added 3 commits September 3, 2025 11:47

run black

ed01d78

forgot a file

758bf85

remove old cubic eos

7741873

dallan-keylogic commented Sep 3, 2025

View reviewed changes

dallan-keylogic added 2 commits September 3, 2025 14:42

remove legacy cubic eos from docs

39ce8dc

revert reintroduced bug

fa5625c

dallan-keylogic requested a review from dangunter as a code owner September 3, 2025 18:44

dallan-keylogic changed the title ~~Modular properties fixes~~ Modular properties fixes and enhancements Sep 3, 2025

dallan-keylogic commented Sep 3, 2025

View reviewed changes

dallan-keylogic added the CI:run-integration triggers_workflow: Integration label Sep 3, 2025

dallan-keylogic self-assigned this Sep 3, 2025

idaes-build removed the CI:run-integration triggers_workflow: Integration label Sep 3, 2025

dallan-keylogic mentioned this pull request Sep 3, 2025

Fix in HDA initializer IDAES/examples#147

Merged

dallan-keylogic added 5 commits September 5, 2025 10:24

increase test coverage of ideal property changes

e90cd9d

add back henry support non bubble/dew methods

bab1b8a

covering more bases

83eef3a

add tests for RPP5

f15cae7

invalid phase tests

ce2d56a

dallan-keylogic requested a review from tannerpolley September 9, 2025 17:11

tannerpolley approved these changes Sep 9, 2025

View reviewed changes

agarciadiego reviewed Sep 11, 2025

View reviewed changes

rescue test_cubic_root from deletion

9f3ccb9

agarciadiego reviewed Sep 11, 2025

View reviewed changes

address review comments

7a8835e

ksbeattie added the Priority:High High Priority Issue or PR label Sep 11, 2025

Merge branch 'main' into modular_properties_fixes

d396282

agarciadiego approved these changes Sep 15, 2025

View reviewed changes

dallan-keylogic merged commit 00c61dc into IDAES:main Sep 15, 2025
43 checks passed

dallan-keylogic deleted the modular_properties_fixes branch September 15, 2025 15:56

Comments

Conversation

dallan-keylogic commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fixes

Enhancements

Legal Acknowledgement

Uh oh!

dallan-keylogic Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Sep 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

dallan-keylogic commented Sep 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agarciadiego left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

dallan-keylogic commented Sep 3, 2025 •

edited

Loading

dallan-keylogic Sep 3, 2025 •

edited

Loading

codecov bot commented Sep 3, 2025 •

edited

Loading