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.

Sign up for GitHub

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

Jump to bottom

IX model small fix for anions in demo #1420

Merged
merged 7 commits into from
Jun 1, 2024
Merged

IX model small fix for anions in demo #1420

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
06c9888
abs value of charge in eq_mass_removed
kurbansitterley May 30, 2024
0841cde
add anion test for ix demo
kurbansitterley May 30, 2024
a79e67c
Merge branch 'main' into ix_hotfix_for_demo
kurbansitterley May 31, 2024
b6784ba
remove value call inside abs
kurbansitterley May 31, 2024
d404c89
remove value from imports
kurbansitterley May 31, 2024
fb4c55f
add regen block to IX demo
kurbansitterley May 31, 2024
9e06637
add requires_idaes_solver; test less things
kurbansitterley May 31, 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
185 changes: 172 additions & 13 deletions watertap/examples/flowsheets/ion_exchange/tests/test_ion_exchange_demo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,23 +40,22 @@

__author__ = "Kurban Sitterley"

target_ion = "Ca_2+"
ions = [target_ion]
mass_frac = 1e-4
feed_mass_frac = {target_ion: mass_frac}

solver = get_solver()


class TestIXDemo:
@pytest.fixture(scope="class")
def ix_0D(self):
class TestIXDemoCa:

@pytest.fixture(scope="class")
def ix_0D_Ca(self):
target_ion = "Ca_2+"
ions = [target_ion]
m = ixf.ix_build(ions)
return m

@pytest.mark.unit
def test_build_model(self, ix_0D):
m = ix_0D
def test_build_model(self, ix_0D_Ca):
m = ix_0D_Ca

# Test basic build
assert isinstance(m, ConcreteModel)
Expand Down Expand Up @@ -110,9 +109,9 @@ def test_build_model(self, ix_0D):
assert_units_consistent(m)

@pytest.mark.component
def test_specific_operating_conditions(self, ix_0D):
def test_specific_operating_conditions(self, ix_0D_Ca):

m = ix_0D
m = ix_0D_Ca
ixf.set_operating_conditions(m)
ixf.initialize_system(m)
assert degrees_of_freedom(m) == 0
Expand Down Expand Up @@ -230,8 +229,8 @@ def test_specific_operating_conditions(self, ix_0D):

@pytest.mark.component
@pytest.mark.requires_idaes_solver
def test_optimization(self, ix_0D):
m = ix_0D
def test_optimization(self, ix_0D_Ca):
m = ix_0D_Ca
ixf.optimize_system(m)
isinstance(m.fs.obj, Objective)
assert m.fs.obj.expr == m.fs.costing.LCOW
Expand Down Expand Up @@ -423,3 +422,163 @@ def test_main_fun(self):
assert pytest.approx(s, rel=1e-3) == value(mv[i])
else:
assert pytest.approx(r, rel=1e-3) == value(mv)


class TestIXDemoSO4:

@pytest.fixture(scope="class")
def ix_0D_SO4(self):
target_ion = "SO4_2-"
ions = [target_ion]
m = ixf.ix_build(ions)
return m

@pytest.mark.unit
def test_build_model(self, ix_0D_SO4):
m = ix_0D_SO4

# Test basic build
assert isinstance(m, ConcreteModel)
assert isinstance(m.fs, FlowsheetBlock)
assert isinstance(m.fs.properties, MCASParameterBlock)
assert isinstance(m.fs.costing, Block)
assert isinstance(m.fs.feed, Feed)
assert isinstance(m.fs.ion_exchange, IonExchange0D)
assert isinstance(m.fs.product, Product)

# Test port
assert isinstance(m.fs.feed.outlet, Port)
assert isinstance(m.fs.ion_exchange.inlet, Port)
assert isinstance(m.fs.ion_exchange.outlet, Port)
assert isinstance(m.fs.product.inlet, Port)

# # Test consting
assert isinstance(m.fs.ion_exchange.costing, Block)
assert isinstance(m.fs.ion_exchange.costing.capital_cost, Var)
assert isinstance(m.fs.ion_exchange.costing.fixed_operating_cost, Var)

var_str_list = [
"total_capital_cost",
"total_operating_cost",
]
for var_str in var_str_list:
var = getattr(m.fs.costing, var_str)
assert isinstance(var, Var)

# Test arcs
arc_dict = {
m.fs.feed_to_ix: (m.fs.feed.outlet, m.fs.ion_exchange.inlet),
m.fs.ix_to_product: (m.fs.ion_exchange.outlet, m.fs.product.inlet),
}
for arc, port_tpl in arc_dict.items():
assert arc.source is port_tpl[0]
assert arc.destination is port_tpl[1]

# test configrations
assert len(m.fs.ion_exchange.config) == 11
assert not m.fs.ion_exchange.config.dynamic
assert not m.fs.ion_exchange.config.has_holdup
assert m.fs.ion_exchange.config.target_ion == "SO4_2-"
assert m.fs.ion_exchange.ion_exchange_type == IonExchangeType.anion
assert m.fs.ion_exchange.config.regenerant == RegenerantChem.NaCl
assert m.fs.ion_exchange.config.isotherm == IsothermType.langmuir

assert m.fs.ion_exchange.config.property_package is m.fs.properties
assert "H2O" in m.fs.properties.component_list

assert_units_consistent(m)

@pytest.mark.component
def test_specific_operating_conditions(self, ix_0D_SO4):

m = ix_0D_SO4
ixf.set_operating_conditions(m)
ixf.initialize_system(m)
assert degrees_of_freedom(m) == 0

solver = get_solver()
results = solver.solve(m)
assert_optimal_termination(results)
assert value(m.fs.feed.properties[0].flow_vol_phase["Liq"]) == pytest.approx(
0.05, rel=1e-3
)
assert value(m.fs.product.properties[0].flow_vol_phase["Liq"]) == pytest.approx(
0.049995010, rel=1e-3
)
assert value(
sum(
m.fs.feed.properties[0].conc_mass_phase_comp["Liq", j]
for j in m.fs.properties.ion_set
)
) == pytest.approx(0.1, rel=1e-3)
assert value(
sum(
m.fs.product.properties[0].conc_mass_phase_comp["Liq", j]
for j in m.fs.properties.ion_set
)
) == pytest.approx(8.821e-5, rel=1e-3)

results_dict = {
"resin_surf_per_vol": 4285.714,
"c_norm": {"SO4_2-": 0.473},
"bed_vol_tot": 12.0,
"col_height": 3.4887,
"col_diam": 1.498,
"col_height_to_diam_ratio": 2.327,
"number_columns": 4.0,
"t_breakthru": 136055.4,
"ebct": 240.0,
"vel_bed": 0.0070833,
"N_Re": 4.958333,
"N_Sc": {"SO4_2-": 943.3},
"N_Sh": {"SO4_2-": 25.095},
"N_Pe_particle": 0.107827,
"N_Pe_bed": 261.867,
"resin_max_capacity": 3.0,
"resin_eq_capacity": 1.685707,
"resin_unused_capacity": 1.314292,
"langmuir": {"SO4_2-": 0.7},
"mass_removed": {"SO4_2-": 7079.969},
"num_transfer_units": 39.087,
"dimensionless_time": 1.0,
"partition_ratio": 566.397,
"fluid_mass_transfer_coeff": {"SO4_2-": 3.8001e-05},
"pressure_drop": 9.450141,
"separation_factor": {"SO4_2-": 1.4285},
"rate_coeff": {"SO4_2-": 0.000232663},
"HTU": {"SO4_2-": 0.043492261},
}
for v, r in results_dict.items():
ixv = getattr(m.fs.ion_exchange, v)
if ixv.is_indexed():
for i, s in r.items():
assert pytest.approx(s, rel=1e-3) == value(ixv[i])
else:
assert pytest.approx(r, rel=1e-3) == value(ixv)

sys_cost_results = {
"aggregate_capital_cost": 866570.3,
"aggregate_fixed_operating_cost": 5492.468,
"aggregate_variable_operating_cost": 0.0,
"aggregate_flow_electricity": 4.023,
"aggregate_flow_NaCl": 1016854.2,
"aggregate_flow_costs": {"electricity": 2468.7, "NaCl": 92576.0},
"total_capital_cost": 866570.3,
"total_operating_cost": 117029.8,
"aggregate_direct_capital_cost": 433285.1,
"maintenance_labor_chemical_operating_cost": 25997.1,
"total_fixed_operating_cost": 31489.578,
"total_variable_operating_cost": 85540.2,
"total_annualized_cost": 203686.8,
"annual_water_production": 1419950.1,
"LCOW": 0.14344,
"specific_energy_consumption": 0.022353,
}

for v, r in sys_cost_results.items():
mv = getattr(m.fs.costing, v)
if mv.is_indexed():
for i, s in r.items():
assert pytest.approx(s, rel=1e-3) == value(mv[i])
else:
assert pytest.approx(r, rel=1e-3) == value(mv)
3 changes: 2 additions & 1 deletion watertap/unit_models/ion_exchange_0D.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
Param,
Suffix,
log,
value,
units as pyunits,
)
from pyomo.common.config import ConfigBlock, ConfigValue, In
Expand Down Expand Up @@ -1053,7 +1054,7 @@ def eq_partition_ratio(b):
self.target_ion_set, doc="Removed total mass of ion in equivalents"
)
def eq_mass_removed(b, j):
charge = prop_in.charge_comp[j]
charge = abs(value(prop_in.charge_comp[j]))
Copy link
Contributor

Choose a reason for hiding this comment

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

Is prop_in.charge_comp[j] a parameter which should never change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, should never change and is set as part of the property package configuration.

Copy link
Contributor

Choose a reason for hiding this comment

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

don't know if this would be overkill, but would it be safer to use smooth_abs? One potential scenario could be that the charge for a particular component is unknown, and we set it as a var to estimate. I think in that case, you'd want something like smooth_abs. I don't know how likely that scenario is, but just thinking out loud on whether we think it's fine to just use abs here for the long run.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe the compromise is to use pyo.environ.abs.

If this value is fixed but the user updates it for some reason you'll still get the correct value put to the solver.

Copy link
Collaborator

Choose a reason for hiding this comment

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

smooth_abs would have no effect in this case as written as you are using value inside the abs; i.e. it is effectively a constant and not a variable. However, that does lead me to ask if this is the intended behaviour, as with the value in there this is now just a constant and thus will not change if the value of the variable is changed later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

don't know if this would be overkill, but would it be safer to use smooth_abs? One potential scenario could be that the charge for a particular component is unknown, and we set it as a var to estimate. I think in that case, you'd want something like smooth_abs. I don't know how likely that scenario is, but just thinking out loud on whether we think it's fine to just use abs here for the long run.

A target_ion with an unknown charge should raise a ConfigurationError for this model. I also think if we were in a situation where we were trying to predict the charge of an unknown component, we would likely need a different model.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe the compromise is to use pyo.environ.abs.

If this value is fixed but the user updates it for some reason you'll still get the correct value put to the solver.

I tried this initially (i.e., from pyomo.environ import abs) and it resulted in an ImportError. Could it exist elsewhere in pyomo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

smooth_abs would have no effect in this case as written as you are using value inside the abs; i.e. it is effectively a constant and not a variable. However, that does lead me to ask if this is the intended behaviour, as with the value in there this is now just a constant and thus will not change if the value of the variable is changed later.

Yes this is the intended behavior - the value of charge_comp (a Param) must be either positive or negative to use the model, and would be set as part of the configuration of the property package (it is checked at the beginning of the build for this model) . It is assumed it is a fundamental property of the solute and wouldn't change later.

Copy link
Contributor

@adam-a-a adam-a-a Jun 1, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

True, it is currently a Param. I guess I was just thinking of a use case of my own, where I might need to set charge_comp as a Var for parameter estimation since, in that use case, the charge is unknown (just know it should be negative). But- right now charge_comp is just a Param so shouldn’t matter at this point.

kurbansitterley marked this conversation as resolved.
Show resolved Hide resolved
return b.mass_removed[j] * charge == pyunits.convert(
b.resin_eq_capacity * b.resin_bulk_dens * b.bed_vol_tot,
to_units=pyunits.mol,
Expand Down
Loading