Add support for isothermal conditions in control volumes. by andrewlee94 · Pull Request #1558 · IDAES/idaes-pse

andrewlee94 · 2025-01-15T16:37:40Z

Fixes None

Summary/Motivation:

WateTAP has a number of cases where they would like to use isothermal conditions in place of an energy balance. Currently, they use a mix-in to support this, but this adds an addition argument that is effectively redundant. Instead, it would be better to have explicit support for isothermal conditions as part of the core ControlVolume classes.

However, isothermal conditions do not make sense for all unit models, and there are a few cases where unit models have effectively implemented their own isothermal conditions (notable PressureChanger). Thus, to avoid potential edge cases in unit models until they can be updated, a new ExtendedControlVolume0D and ExtenededControlVolume1D class have been created as an interim solution. It is envisioned that unit models will gradually be updated to be more explicit about what balance types make sense, at which point the new and old control volumes can be merged.

Changes proposed in this PR:

Added new isothermal option to EnergyBalanceType Enum.
Added BalanceTypeNotSupported exceptions to ControlVolume0D and ControlVolume1D if isothermal option is selected, with pointer to the new control volume classes.
New ExtendedControlVolume0D and ExtendedControlVolume1D classes with support for isothermal balances.
Docs and tests.

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.

docs/reference_guides/core/control_volume_0d.rst

docs/reference_guides/core/control_volume_1d.rst

Co-authored-by: Adam Atia <[email protected]>

adam-a-a · 2025-01-15T17:07:14Z

idaes/core/base/extended_control_volume1d.py

+            return (
+                b.properties[t, b.length_domain.prev(x)].temperature
+                == b.properties[t, x].temperature


It looks like the constraint would be skipped at the outlet when using forward scheme, but what does this mean for temperature at the outlet?

Actually, most of this is not needed as there is no partial derivative. The way this is written, it has to skip the first point no matter what.

I might be misunderstanding, but as written, it appears that for backwards transformation, temperature at the inlet (x=0) would be set equal to the temperature of the next element, and this equality would be propagated across adjacent pairs for the whole length domain. So the temperature for each element would be covered.

However, for the forward transformation case, it seems that all elements except the last element would be covered by the equality constraint.

OK, now I think the issue is fixed.

Can we also rename enthalpy_balances to isothermal_constraint or something here?

adam-a-a · 2025-01-15T17:17:57Z

idaes/core/base/tests/test_control_volume_0d.py

+    m.fs = Flowsheet(dynamic=False)
+    m.fs.pp = PhysicalParameterTestBlock()
+    m.fs.rp = ReactionParameterTestBlock(property_package=m.fs.pp)
+    m.fs.pp.del_component(m.fs.pp.phase_equilibrium_idx)


Why is this necessary?

Suggested change

m.fs.pp.del_component(m.fs.pp.phase_equilibrium_idx)

m.fs.pp.del_component(m.fs.pp.phase_equilibrium_idx)

It isn't - just left over from copying an old test (where it was likely not needed either).

adam-a-a · 2025-01-15T17:22:04Z

idaes/core/base/tests/test_extended_control_volume_0d.py

+            == m.fs.cv.properties_out[t].temperature
+        )
+
+    assert_units_consistent(m.fs.cv)


Interesting- I thought I remember the units check failing for dynamic cases, or is that only when has_holdup=True as well?

It will fail for anything with a partial derivative. Here we get aways with it because we don't have a derivative.

Not necessarily. Generally for spatial domains we normalize the ContinuousSet to run from 0 to 1, adding in the flow length later. Those should pass. Any ContinuousSet with dimensions, however, will cause it to fail.

adam-a-a · 2025-01-15T17:23:20Z

idaes/core/base/tests/test_extended_control_volume_0d.py

+        ConfigurationError,
+        match="fs.cv: isothermal energy balance option requires that has_heat_transfer is False. "
+        "If you are trying to solve for heat duty to achieve isothermal operation, please use "
+        "a full energy balance as add a constraint to equate inlet and outlet temperatures.",


This error message needs some revision: "a full energy balance as add a constraint to equate inlet and outlet temperatures."

Fix: "and add"

adam-a-a · 2025-01-15T17:24:16Z

idaes/core/base/tests/test_extended_control_volume_0d.py

+        ConfigurationError,
+        match="fs.cv: isothermal energy balance option requires that has_work_transfer is False. "
+        "If you are trying to solve for work under isothermal operation, please use "
+        "a full energy balance as add a constraint to equate inlet and outlet temperatures.",


Same issue- just need to revise this segment of the error message and propagate through.

adam-a-a · 2025-01-15T17:27:03Z

idaes/core/base/tests/test_extended_control_volume_1d.py

+    m.fs.cv.add_isothermal_constraint()
+
+    assert isinstance(m.fs.cv.enthalpy_balances, Constraint)
+    assert len(m.fs.cv.enthalpy_balances) == 1  # x==0 is skipped


I thought the length of enthalpy_balances would be fine_elements - 1. Maybe I am missing something (or the test is failing).

The model has not been discretised, so there are only 2 points.

Oh! I see now. Didn't catch that--didn't apply transformation.

adam-a-a · 2025-01-15T17:28:11Z

idaes/core/base/tests/test_extended_control_volume_1d.py

+    m.fs.cv.add_isothermal_constraint()
+
+    assert isinstance(m.fs.cv.enthalpy_balances, Constraint)
+    assert len(m.fs.cv.enthalpy_balances) == 4  # x==0 is skipped


Similarly, would expect this to be of length equal to (finite_elements - 1) * len(time_set)

It is, it is just (2-1)*4 = 4.

finite elements=10 not 2 here

OK, just saw your previous response. Any reason not to apply transformation and check?

Reduce test overhead.

Could be good just to do a transformation with, say, 3 finite elements.

I debated it, but decided that was as much a test of Pyomo functionality as of IDAES (and thus I was going to trust Pyomo).

I realised there is a reason to test this, but not for the obvious reason - when using the domain.prev() method it works on the domain as it appears at the time of the call, meaning that it will always create T[0, 0] == T[0, 1] and then fill in intermediate points. In this case, it should not matter, but it needs to be tested.

adam-a-a

LGTM - I don't think it would hurt too much to add a test with transformation applied to 1d CV (with 2 - 3 finite elements), but I'll leave that to other reviewers and @andrewlee94

dallan-keylogic

Some minor changes, otherwise it looks good.

dallan-keylogic · 2025-01-16T15:24:34Z

idaes/core/base/extended_control_volume0d.py

+        has_heat_of_reaction=False,
+        has_heat_transfer=False,
+        has_work_transfer=False,
+        has_enthalpy_transfer=False,
+        custom_term=None,


Should we add Type Hints here?

Might as well - if we don't do it now it probably won't get done.

dallan-keylogic · 2025-01-16T15:32:46Z

idaes/core/base/extended_control_volume0d.py

+        @self.Constraint(self.flowsheet().time, doc="Energy balances")
+        def enthalpy_balances(b, t):
+            return b.properties_in[t].temperature == b.properties_out[t].temperature


Can we rename this constraint to something that better describe what it does? Otherwise you might, for example, get enthalpy scaling on a constraint that should have temperature scaling.

I debated whether the change the name - my reason for keeping it the same was for consistency of naming between different configurations (so that the "energy balance" would always have the same name no matter what option you chose).

However, I did not think about scaling, and having a descriptive name would be far more useful there for people doing custom scaling. I'll change it.

dallan-keylogic · 2025-01-16T15:41:45Z

idaes/core/base/extended_control_volume1d.py

+            return (
+                b.properties[t, b.length_domain.prev(x)].temperature
+                == b.properties[t, x].temperature


Can we also rename enthalpy_balances to isothermal_constraint or something here?

dallan-keylogic

A few more documentation quibbles.

dallan-keylogic · 2025-01-16T16:33:06Z

docs/reference_guides/core/control_volume_0d.rst

+`enthalpy_balances(t)`:
+
+.. math:: T_{in, t} == T_{out, t}


This is now outdated

dallan-keylogic · 2025-01-16T16:34:41Z

docs/reference_guides/core/control_volume_1d.rst

+`enthalpy_balances(t)`:
+
+.. math:: T_{t, x-1} == T_{t, x}


dallan-keylogic · 2025-01-16T16:43:23Z

idaes/core/base/extended_control_volume0d.py

+                Method should accept time and phase list as arguments.
+
+        Returns:
+            Constraint object representing enthalpy balances


Might be good to update this to reflect isothermal constraint.

dallan-keylogic · 2025-01-16T16:43:30Z

idaes/core/base/extended_control_volume1d.py

+                Method should accept time and phase list as arguments.
+
+        Returns:
+            Constraint object representing enthalpy balances


Might be good to update this to reflect isothermal constraint.

dallan-keylogic

LGTM

andrewlee94 added 3 commits January 3, 2025 09:24

Extended control volumes

1a58c7c

Extended CV with isothermal option

93c41b8

Docs for extended CVs

eaeabc4

andrewlee94 requested review from agarciadiego, bpaul4, dallan-keylogic, dangunter and lbianchi-lbl as code owners January 15, 2025 16:37

Black and pylint

70c3475

andrewlee94 self-assigned this Jan 15, 2025

andrewlee94 added enhancement New feature or request Priority:Normal Normal Priority Issue or PR core Issues dealing with core modeling components backward-compat Affects backward compatibility WaterTAP labels Jan 15, 2025

adam-a-a requested a review from bknueven January 15, 2025 16:55