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.

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

1D Membrane Model for CO2 Capture and Utilization #1378

Merged
merged 43 commits into from
Nov 27, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
43 commits
Select commit Hold shift + click to select a range
2affa77
Add CCUS file structure
Morgan88888888 Mar 18, 2024
b0a336a
One-dimensional membrane model
Morgan88888888 Mar 18, 2024
775b27a
add the unit test file
Morgan88888888 Mar 18, 2024
dd437b7
reformatted by black
Morgan88888888 Mar 18, 2024
693fd68
reformat using black
Morgan88888888 Mar 18, 2024
5505b1f
fixed the path issue in testing
Morgan88888888 Mar 18, 2024
f91e825
added the missing heading
Morgan88888888 Mar 18, 2024
446951b
refined the workspace name
Morgan88888888 Mar 19, 2024
db77a65
formatting issue resolved for GitHub test
Morgan88888888 Mar 19, 2024
508f10c
fixed typo
Morgan88888888 Mar 19, 2024
d31a4cb
fixed the unit model importing issue
Morgan88888888 Mar 29, 2024
e228592
fixed linter warnings
Morgan88888888 Mar 29, 2024
feda25c
resolved the comments
Morgan88888888 Mar 29, 2024
054808f
fix linter issues
Morgan88888888 Mar 29, 2024
f6dc03b
formated
Morgan88888888 Mar 29, 2024
a6c9697
Merge branch 'IDAES:main' into co2_membrane
Morgan88888888 May 2, 2024
14d3856
remove the unit level property config
Morgan88888888 May 13, 2024
e8bc928
save changes
Morgan88888888 May 14, 2024
3d8e068
Merge branch 'IDAES:main' into co2_membrane
Morgan88888888 May 28, 2024
81c3fa3
Merge branch 'IDAES:main' into co2_membrane
Morgan88888888 Jun 4, 2024
35a8ee4
Merge branch 'IDAES:main' into co2_membrane
Morgan88888888 Jul 5, 2024
53a17b5
Merge branch 'co2_membrane' of https://github.com/Morgan88888888/idae…
Morgan88888888 Jul 11, 2024
97abbe7
Merge branch 'IDAES:main' into co2_membrane
Morgan88888888 Nov 5, 2024
b795061
Merge branch 'co2_membrane' of https://github.com/Morgan88888888/idae…
Morgan88888888 Nov 5, 2024
e721692
added test for different configs and added stream table display
Morgan88888888 Nov 5, 2024
2cd61a4
added more docs to explain the models and settings
Morgan88888888 Nov 5, 2024
3e3dda8
Addressed the comments to support different property packages
Morgan88888888 Nov 5, 2024
278345b
add linebreak
Morgan88888888 Nov 5, 2024
c30f6f9
corrected copyright info
Morgan88888888 Nov 5, 2024
0b8a57d
reformatted file to pass test
Morgan88888888 Nov 5, 2024
d826433
fixed copyright info
Morgan88888888 Nov 5, 2024
093de8e
added material conservation test
Morgan88888888 Nov 5, 2024
448394f
reformatted
Morgan88888888 Nov 5, 2024
d6a977c
address comments
Morgan88888888 Nov 5, 2024
75cfb24
added basic documentation
Morgan88888888 Nov 5, 2024
c6d3677
fix pylint test
Morgan88888888 Nov 5, 2024
83529cf
fix doc strings
Morgan88888888 Nov 5, 2024
80befee
fix the doc string
Morgan88888888 Nov 5, 2024
6f3f8a6
added what the inputs/degrees of freedom
Morgan88888888 Nov 5, 2024
93038d4
fix pytest
Morgan88888888 Nov 5, 2024
d44f95e
Merge branch 'main' into co2_membrane
ksbeattie Nov 14, 2024
410c036
Merge branch 'main' into co2_membrane
ksbeattie Nov 21, 2024
b1051bf
Merge branch 'main' into co2_membrane
lbianchi-lbl Nov 27, 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
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
This directory contains the unit models for Carbon Capture and Utilization
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#################################################################################
# The Institute for the Design of Advanced Energy Systems Integrated Platform
# Framework (IDAES IP) was produced under the DOE Institute for the
# Design of Advanced Energy Systems (IDAES).
#
# Copyright (c) 2018-2023 by the software owners: The Regents of the
Morgan88888888 marked this conversation as resolved.
Show resolved Hide resolved
# University of California, through Lawrence Berkeley National Laboratory,
# National Technology & Engineering Solutions of Sandia, LLC, Carnegie Mellon
# University, West Virginia University Research Corporation, et al.
# All rights reserved. Please see the files COPYRIGHT.md and LICENSE.md
# for full copyright and license information.
#################################################################################
from .membrane_1d import Membrane1D, MembraneFlowPattern
Original file line number Diff line number Diff line change
@@ -0,0 +1,316 @@
#################################################################################
# The Institute for the Design of Advanced Energy Systems Integrated Platform
Morgan88888888 marked this conversation as resolved.
Show resolved Hide resolved
# Framework (IDAES IP) was produced under the DOE Institute for the
# Design of Advanced Energy Systems (IDAES).
#
# Copyright (c) 2018-2023 by the software owners: The Regents of the
Morgan88888888 marked this conversation as resolved.
Show resolved Hide resolved
# University of California, through Lawrence Berkeley National Laboratory,
# National Technology & Engineering Solutions of Sandia, LLC, Carnegie Mellon
# University, West Virginia University Research Corporation, et al.
# All rights reserved. Please see the files COPYRIGHT.md and LICENSE.md
# for full copyright and license information.
#################################################################################

"""
One-dimensional membrane class for CO2 gas separation
"""

# pylint: disable=unused-import
Copy link
Member

Choose a reason for hiding this comment

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

Why is this being disabled? Enum is being used, so there should not be a warning issued by this.

Copy link
Member

Choose a reason for hiding this comment

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

Again, why is this disabled? This implies you have an unused import which should be removed.

Copy link
Member Author

Choose a reason for hiding this comment

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

It is disabled to pass the pylint test.

Copy link
Member

Choose a reason for hiding this comment

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

You miss my point. I am asking why you are disabling the test, as you generally should not turn off pytest checks.

from enum import Enum
from pyomo.common.config import Bool, ConfigDict, ConfigValue, In
from pyomo.environ import (
Constraint,
Param,
Var,
units,
Expression,
)
from pyomo.network import Port

from idaes.core import (
FlowDirection,
UnitModelBlockData,
declare_process_block_class,
useDefault,
MaterialFlowBasis,
)
from idaes.core.util.config import is_physical_parameter_block
from idaes.models.unit_models.mscontactor import MSContactor
from idaes.core.util.exceptions import ConfigurationError

__author__ = "Maojian Wang"


class MembraneFlowPattern(Enum):
"""
Enum of supported flow patterns for membrane.
So far only support countercurrent and cocurrent flow
"""

COUNTERCURRENT = 1
COCURRENT = 2
CROSSFLOW = 3


@declare_process_block_class("Membrane1D")
class Membrane1DData(UnitModelBlockData):
"""Standard Membrane 1D Unit Model Class."""

CONFIG = UnitModelBlockData.CONFIG()

Stream_Config = ConfigDict()

Stream_Config.declare(
"property_package",
ConfigValue(
default=useDefault,
domain=is_physical_parameter_block,
description="Property package to use for given stream",
doc="""Property parameter object used to define property calculations for given stream,
**default** - useDefault.
**Valid values:** {
**useDefault** - use default package from parent model or flowsheet,
**PhysicalParameterObject** - a PhysicalParameterBlock object.}""",
),
)
Stream_Config.declare(
"property_package_args",
ConfigDict(
implicit=True,
description="Dict of arguments to use for constructing property package",
doc="""A ConfigDict with arguments to be passed to property block(s)
and used when constructing these,
**default** - None.
**Valid values:** {
see property package for documentation.}""",
),
)
Stream_Config.declare(
"has_energy_balance",
ConfigValue(
default=True,
domain=Bool,
doc="Bool indicating whether to include energy balance for stream. Default=True.",
),
)
Stream_Config.declare(
"has_pressure_balance",
ConfigValue(
default=True,
domain=Bool,
doc="Bool indicating whether to include pressure balance for stream. Default=True.",
),
)

Morgan88888888 marked this conversation as resolved.
Show resolved Hide resolved
CONFIG.declare(
Morgan88888888 marked this conversation as resolved.
Show resolved Hide resolved
"sweep_flow",
Morgan88888888 marked this conversation as resolved.
Show resolved Hide resolved
ConfigValue(
default=True,
domain=Bool,
doc="Bool indicating whether there is a sweep flow in the permeate side.",
description="Bool indicating whether stream has a feed Port and inlet "
"state, or if all flow is provided via mass transfer. Default=True.",
),
)
CONFIG.declare(
"finite_elements",
ConfigValue(
default=5,
domain=int,
description="Number of finite elements in length domain",
doc="""Number of finite elements to use when discretizing length
domain (default=5)""",
),
)
CONFIG.declare(
"flow_type",
Morgan88888888 marked this conversation as resolved.
Show resolved Hide resolved
ConfigValue(
default=MembraneFlowPattern.COUNTERCURRENT,
domain=In(MembraneFlowPattern),
description="Flow configuration of membrane",
doc="""Flow configuration of membrane
- MembraneFlowPattern.COCURRENT: feed and sweep flows from 0 to 1
- MembraneFlowPattern.COUNTERCURRENT: feed side flows from 0 to 1
sweep side flows from 1 to 0 (default)""",
),
)
CONFIG.declare(
Morgan88888888 marked this conversation as resolved.
Show resolved Hide resolved
"property_package",
ConfigValue(
default=None,
domain=is_physical_parameter_block,
description="Property package to use for control volume",
doc="""Property parameter object used to define property
calculations
(default = 'use_parent_value')
- 'use_parent_value' - get package from parent (default = None)
- a ParameterBlock object""",
),
)
CONFIG.declare(
"property_package_args",
ConfigValue(
default={},
description="Arguments for constructing property package",
doc="""A dict of arguments to be passed to the PropertyBlockData
and used when constructing these
(default = 'use_parent_value')
- 'use_parent_value' - get package from parent (default = None)
- a dict (see property package for documentation)""",
),
)

for side_name in ["feed", "sweep"]:
Morgan88888888 marked this conversation as resolved.
Show resolved Hide resolved
CONFIG.declare(
side_name + "_side",
Stream_Config(),
)

def build(self):
super().build()

if self.config.property_package is not None:
if self.config.feed_side.property_package == useDefault:
self.config.feed_side.property_package = self.config.property_package
Morgan88888888 marked this conversation as resolved.
Show resolved Hide resolved
if self.config.sweep_side.property_package == useDefault:
self.config.sweep_side.property_package = self.config.property_package

feed_dict = dict(self.config.feed_side)
sweep_dict = dict(self.config.sweep_side)

feed_dict["flow_direction"] = FlowDirection.forward
if self.config.flow_type == MembraneFlowPattern.COCURRENT:
sweep_dict["flow_direction"] = FlowDirection.forward

Check warning on line 183 in idaes/models_extra/co2_capture_and_utilization/unit_models/membrane_1d.py

View check run for this annotation

Codecov / codecov/patch

idaes/models_extra/co2_capture_and_utilization/unit_models/membrane_1d.py#L183

Added line #L183 was not covered by tests
elif self.config.flow_type == MembraneFlowPattern.COUNTERCURRENT:
sweep_dict["flow_direction"] = FlowDirection.backward
else:
raise ConfigurationError(

Check warning on line 187 in idaes/models_extra/co2_capture_and_utilization/unit_models/membrane_1d.py

View check run for this annotation

Codecov / codecov/patch

idaes/models_extra/co2_capture_and_utilization/unit_models/membrane_1d.py#L187

Added line #L187 was not covered by tests
f"{self.name} Membrane1D only supports cocurrent and "
"countercurrent flow patterns, but flow_type configuration"
" argument was set to {config.flow_type}."
)

if self.config.sweep_flow is False:
sweep_dict["has_feed"] = False

Check warning on line 194 in idaes/models_extra/co2_capture_and_utilization/unit_models/membrane_1d.py

View check run for this annotation

Codecov / codecov/patch

idaes/models_extra/co2_capture_and_utilization/unit_models/membrane_1d.py#L194

Added line #L194 was not covered by tests

streams_dict = {"feed_side": feed_dict, "sweep_side": sweep_dict}
self.mscontactor = MSContactor(
streams=streams_dict,
Morgan88888888 marked this conversation as resolved.
Show resolved Hide resolved
number_of_finite_elements=self.config.finite_elements,
)

self.feed_side_inlet = Port(extends=self.mscontactor.feed_side_inlet)
self.feed_side_outlet = Port(extends=self.mscontactor.feed_side_outlet)
if self.config.sweep_flow is True:
self.sweep_side_inlet = Port(extends=self.mscontactor.sweep_side_inlet)
self.sweep_side_outlet = Port(extends=self.mscontactor.sweep_side_outlet)

self._make_geometry()
self._make_performance()

def _make_geometry(self):

self.area = Var(initialize=100, units=units.cm**2, doc="The membrane area")
Morgan88888888 marked this conversation as resolved.
Show resolved Hide resolved

self.length = Var(initialize=100, units=units.cm, doc="The membrane length")
self.cell_length = Expression(expr=self.length / self.config.finite_elements)

self.cell_area = Var(
initialize=100, units=units.cm**2, doc="The membrane area"
Morgan88888888 marked this conversation as resolved.
Show resolved Hide resolved
)

@self.Constraint()
def area_per_cell(self):
return self.cell_area == self.area / self.config.finite_elements

def _make_performance(self):
feed_side_units = (
self.config.feed_side.property_package.get_metadata().derived_units
)

self.permeance = Var(
self.flowsheet().time,
self.mscontactor.elements,
Copy link
Member

Choose a reason for hiding this comment

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

To check, does this need to be indexed by time and finite element? Whilst there are cases where you would want this degree of flexibility, is it something you want to support right now?

Also, I do not think this should be indexed by the feed side component list. Either you should have a single property package for the unit model, in which case you should use the uni level package, or it you want to support separate property packages you should use the intersection of the feed and sweep side component lists (i.e. only create a term for those species which appear in both property packages). Otherwise, you run the risk of trying to write constraints involving species which do not exist on the sweep side.

Copy link
Member Author

Choose a reason for hiding this comment

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

In the simple assumption, it isn't necessary. However, retaining it isn't particularly harmful either.
The example you gave about indexing could occur in some membrane separations but isn't common in this application. I'll keep this idea in mind for future improvements if the need arises.

Copy link
Member

Choose a reason for hiding this comment

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

It is better to fix these issues now rather than wait until later when you have forgotten how the model works.

You need to make the decision to either use a single property package for everything or not, and if you decide to allow for multiple property packages then you need to go the full way to supporting them.

Copy link
Member

Choose a reason for hiding this comment

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

What happens here if the two property packages have different components? Should this instead use the intersection of the two component lists (which is easy to do)?

self.mscontactor.feed_side.component_list,
initialize=1,
doc="Values in Gas Permeance Unit(GPU)",
Copy link
Member

Choose a reason for hiding this comment

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

Missing space "Unit (GPU)"

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

units=units.dimensionless,
)

self.gpu_factor = Param(
default=10e-8 / 13333.2239,
units=units.m / units.s / units.Pa,
mutable=True,
)

self.selectivity = Var(
self.flowsheet().time,
self.mscontactor.elements,
self.mscontactor.feed_side.component_list,
Copy link
Member

Choose a reason for hiding this comment

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

Do you need the full factorial combination of these? I suspect you do not need the combination where both components are the same, and that S[i, j] == 1/S[j, i]. Looking below, I think this is confirmed by the following constraint.

Thus, my question becomes how do you intend for a user to use these? If a user were to naively fix all the selectivity, then the problem would be over-specified.

Copy link
Member

Choose a reason for hiding this comment

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

How are you handling the possible over-specification here?

self.mscontactor.feed_side.component_list,
initialize=1,
units=units.dimensionless,
)

@self.Constraint(
self.flowsheet().time,
self.mscontactor.elements,
self.mscontactor.feed_side.component_list,
self.mscontactor.feed_side.component_list,
doc="permeance calculation",
)
def permeance_calculation(self, t, e, a, b):
return (
self.permeance[t, e, a] * self.selectivity[t, e, a, b]
== self.permeance[t, e, b]
)
Copy link
Member

Choose a reason for hiding this comment

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

This looks like it results in an over-specified problem. You write a constraint for a, b and b, a which are reflections of each other. If the user specifies all the selectivities they will have a degree of freedom issue.

I think you really need to only cover the unique pairs (and exclude a=b).


p_units = feed_side_units.PRESSURE

@self.Constraint(
self.flowsheet().time,
self.mscontactor.elements,
self.mscontactor.feed_side.component_list,
doc="permeability calculation",
)
def permeability_calculation(self, t, s, m):
feed_side_state = self.mscontactor.feed_side[t, s]
if feed_side_state.get_material_flow_basis() is MaterialFlowBasis.molar:
mb_units = feed_side_units.FLOW_MOLE
rho = self.mscontactor.feed_side[t, s].dens_mol
elif feed_side_state.get_material_flow_basis() is MaterialFlowBasis.mass:
mb_units = feed_side_units.FLOW_MASS
rho = self.mscontactor.feed_side[t, s].dens_mass

Check warning on line 283 in idaes/models_extra/co2_capture_and_utilization/unit_models/membrane_1d.py

View check run for this annotation

Codecov / codecov/patch

idaes/models_extra/co2_capture_and_utilization/unit_models/membrane_1d.py#L282-L283

Added lines #L282 - L283 were not covered by tests
else:
raise TypeError("Undefined flow basis, please define the flow basis")

Check warning on line 285 in idaes/models_extra/co2_capture_and_utilization/unit_models/membrane_1d.py

View check run for this annotation

Codecov / codecov/patch

idaes/models_extra/co2_capture_and_utilization/unit_models/membrane_1d.py#L285

Added line #L285 was not covered by tests
Morgan88888888 marked this conversation as resolved.
Show resolved Hide resolved

return self.mscontactor.material_transfer_term[
t, s, "feed_side", "sweep_side", m
] == -units.convert(
(
rho
* self.gpu_factor
* self.permeance[t, s, m]
* self.cell_area
* (
self.mscontactor.feed_side[t, s].pressure
* self.mscontactor.feed_side[t, s].mole_frac_comp[m]
- units.convert(
self.mscontactor.sweep_side[t, s].pressure, to_units=p_units
)
* self.mscontactor.sweep_side[t, s].mole_frac_comp[m]
)
),
to_units=mb_units,
)

@self.Constraint(
self.flowsheet().time,
self.mscontactor.elements,
doc="Energy balance",
)
def energy_transfer(self, t, s):
Copy link
Member

Choose a reason for hiding this comment

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

For clarity, I would call this an isothermal constraint. It is not an energy balance.

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

Copy link
Member

Choose a reason for hiding this comment

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

I would also suggest changing the name of the constraint (i.e. the name of the rule/function).

Copy link
Member Author

Choose a reason for hiding this comment

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

changed

return (
self.mscontactor.feed_side[t, s].temperature
== self.mscontactor.sweep_side[t, s].temperature
)
Loading
Loading