Fixing trying to access metadata via config block

[andrewlee94]

Fixes #1249

Summary/Motivation:

@vova292 reported a bug where an error was being raised by the HX unit model when using a default property package defined at the flowsheet level. Investigation revealed that this was due to the model trying to access the properties metadata via the config block which was pointing to useDefault, and that this was a widespread issue across the code base.

The fix is to access the metadata either via a StateBlock or a ControlVolume (which checks for useDefault and replaces it with the correct reference), or to use the _get_property_block method to update the config block if a control volume is not used. This PR addresses many (hopefully most) of these issues.

Changes proposed in this PR:

Legal Acknowledgement

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

1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
2. 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]

Shouldn't this be added to ProcessBlock.build instead of putting it in each of these blocks?

[andrewlee94]

No, as this changes the config block which we generally avoid doing. Hence, I only used this where necessary (and even then I probably could have found another way to do it, but didn't want to work throguh that mess).

[codecov]

Codecov Report

Patch coverage: 100.00% and project coverage change: -0.01% ⚠️

Comparison is base (1130710) 76.72% compared to head (cb7c78d) 76.72%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1252      +/-   ##
==========================================
- Coverage   76.72%   76.72%   -0.01%     
==========================================
  Files         382      382              
  Lines       61212    61219       +7     
  Branches    11291    11291              
==========================================
+ Hits        46967    46971       +4     
- Misses      11813    11817       +4     
+ Partials     2432     2431       -1     

Files Changed	Coverage Δ
idaes/core/base/control_volume0d.py	89.38% <ø> (ø)
idaes/models/unit_models/heat_exchanger_1D.py	88.98% <ø> (ø)
idaes/models/unit_models/mscontactor.py	91.20% <ø> (ø)
idaes/models/unit_models/shell_and_tube_1d.py	98.54% <ø> (ø)
...models_extra/column_models/plate_heat_exchanger.py	93.79% <ø> (ø)
idaes/core/base/property_base.py	93.86% <100.00%> (+0.07%)	⬆️
idaes/models/unit_models/feed.py	97.67% <100.00%> (+0.05%)	⬆️
idaes/models/unit_models/flash.py	97.80% <100.00%> (ø)
idaes/models/unit_models/gibbs_reactor.py	94.20% <100.00%> (ø)
idaes/models/unit_models/heat_exchanger.py	88.76% <100.00%> (ø)
... and 13 more

... and 1 file with indirect coverage changes

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

[bpaul4]

@andrewlee94 is this something that we test for? If not, do we need to add some so this doesn't occur again?

[andrewlee94]

@bpaul4 Unfortunately, this is very hard to test for as it is an integration issue. The only way to test is to force developers to add a test for their model to ensure it works with a default property package.

[vova292]

If i try an execute the following code using the hx_bug branch i get an error:

import pyomo.environ as pe # Pyomo environment
from idaes.core import FlowsheetBlock, StateBlock
from idaes.models.unit_models import HeatExchangerLumpedCapacitance
from idaes.models.unit_models.heat_exchanger import delta_temperature_amtd_callback
from idaes.models.properties import iapws95

# Create an empty flowsheet and steam property parameter block.
model = pe.ConcreteModel()
property_pack = iapws95.Iapws95ParameterBlock()
model.fs = FlowsheetBlock(dynamic=False, default_property_package=property_pack)
model.fs.properties = property_pack

# Add a Heater model to the flowsheet.
model.fs.heat_exchanger = HeatExchangerLumpedCapacitance(
    delta_temperature_callback=delta_temperature_amtd_callback,
    hot_side_name="shell",
    cold_side_name="tube"
)

I get the following traceback:

Traceback (most recent call last):
  File "/home/shabroz/git/idaes-pse/idaes/core/base/process_block.py", line 41, in _rule_default
    b.build()
  File "/home/shabroz/git/idaes-pse/idaes/models/unit_models/heat_exchanger_lc.py", line 210, in build
    raise ConfigurationError(
idaes.core.util.exceptions.ConfigurationError: fs.heat_exchanger dynamic heat balance cannot be used with a steady-state flowsheet
Traceback (most recent call last):
  File "/home/shabroz/.config/JetBrains/PyCharmCE2023.2/scratches/pyomo_test.py", line 14, in <module>
    model.fs.heat_exchanger = HeatExchangerLumpedCapacitance(
  File "/home/shabroz/.virtualenvs/empc/lib/python3.9/site-packages/pyomo/core/base/block.py", line 580, in __setattr__
    self.add_component(name, val)
  File "/home/shabroz/.virtualenvs/empc/lib/python3.9/site-packages/pyomo/core/base/block.py", line 1185, in add_component
    val.construct(data)
  File "/home/shabroz/.virtualenvs/empc/lib/python3.9/site-packages/pyomo/core/base/block.py", line 2191, in construct
    self._getitem_when_not_present(_idx)
  File "/home/shabroz/.virtualenvs/empc/lib/python3.9/site-packages/pyomo/core/base/block.py", line 2111, in _getitem_when_not_present
    obj = self._rule(_block, idx)
  File "/home/shabroz/.virtualenvs/empc/lib/python3.9/site-packages/pyomo/core/base/initializer.py", line 316, in __call__
    return self._fcn(parent, idx)
  File "/home/shabroz/git/idaes-pse/idaes/core/base/process_block.py", line 41, in _rule_default
    b.build()
  File "/home/shabroz/git/idaes-pse/idaes/models/unit_models/heat_exchanger_lc.py", line 210, in build
    raise ConfigurationError(
idaes.core.util.exceptions.ConfigurationError: fs.heat_exchanger dynamic heat balance cannot be used with a steady-state flowsheet

[dallan-keylogic]

@vova292 Why are you trying to use the HeatExchangerLumpedCapacitance? That's for dynamic model simulation, and it looks like you have a steady state flowsheet. If you really want to use it, you need to set the configuration option dynamic_heat_balance=False.

Now, it looks like that unit model has some issues. It shouldn't be looking to the flowsheet flag for dynamics, but the unit level flag, and it should be able to create the holdup terms by supplying the argument has_holdup=True.

[andrewlee94]

@vova292 @dallan-keylogic In this case, the issue is that the unit model allows the useDefault option for the dynamic heat balance, but never checks to see what to do if that is the case. I have a fix mostly ready, I just need to write some tests.

[dallan-keylogic]

I guess the configs here make sense given that the original heat exchanger (which this model subclasses) gets the dynamic flag for the control volumes from the unit level dynamic flag, but in the bigger picture it gets things backwards. IIRC, for a liquid/liquid heat exchanger over 80% of the holdup comes from metal, and for a gas/gas heat exchanger almost 100% of it comes from the metal.

There's nothing wrong with the code here in question, though.

[andrewlee94]

@dallan-keylogic That might actually be a better solution, but would need us to overload some base methods.