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

Reformulate solubility_products constraints #1197

Merged
merged 23 commits into from
Jul 27, 2023
Merged

Reformulate solubility_products constraints #1197

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
513796e
Modified log_solubility_product function to accomodate the comparison…
May 26, 2023
517c0a1
Remove comments
Jun 7, 2023
6463f3b
run black
bknueven Jun 7, 2023
c940637
Update state_difinitions vars lower bouds
Jun 14, 2023
99db9c3
Add mutable parameters for reformated solubility_product
Jun 14, 2023
2b58efa
Update solubility_product tests
Jun 14, 2023
1edee16
Merge branch 'main' into chem_init
andrewlee94 Jun 15, 2023
d5b2aa3
test_plate_heat_exchanger.py
Jun 15, 2023
d598545
run black
Jun 15, 2023
28aed85
Reverse changes for lower bounds in state_definition
Jun 16, 2023
6eee6b5
Merge branch 'main' into chem_init
andrewlee94 Jun 16, 2023
4cd060d
Add assertion for default s_norm and a_scale checking
Jun 20, 2023
d6462cd
Typo corrected
Jun 20, 2023
89b6a2f
Add documentation for solubility_product
lxhowl Jun 20, 2023
a990507
Update doc string
Jun 20, 2023
d42a687
Merge branch 'IDAES:main' into chem_init
lxhowl Jun 20, 2023
5306d36
Equation corrected for sulubility product doc
Jun 22, 2023
4ca5c87
Merge branch 'IDAES:main' into chem_init
lxhowl Jul 5, 2023
e4a6376
Merge branch 'main' into chem_init
lbianchi-lbl Jul 7, 2023
bc88595
Switch to set_value for default s_norm
Jul 11, 2023
214f14f
Scaling applied in subsaturated cases (low concentrations) to use def…
Jul 18, 2023
7bec453
Run black
Jul 20, 2023
14cb740
Merge branch 'main' into chem_init
andrewlee94 Jul 27, 2023
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
5 changes: 3 additions & 2 deletions idaes/models/properties/modular_properties/reactions/equilibrium_forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ def return_expression(b, rblock, r_idx, T):
if Qunits is not None:
Q = Q / Qunits

return s - smooth_max(0, s - Q, rblock.eps) == 0
return Q - smooth_max(0, Q - s / (s + b.k_eq[r_idx]), rblock.eps) == 0
lxhowl marked this conversation as resolved.
Show resolved Hide resolved

@staticmethod
def calculate_scaling_factors(b, sf_keq):
Expand Down Expand Up @@ -287,7 +287,8 @@ def return_expression(b, rblock, r_idx, T):
Q = b.log_k_eq[r_idx] - e
# Q should be unitless due to log form

return s - smooth_max(0, s - Q, rblock.eps) == 0
s = s * 10 / (s + b.k_eq[r_idx])
lxhowl marked this conversation as resolved.
Show resolved Hide resolved
return Q - smooth_max(0, Q - s, rblock.eps) == 0
Copy link
Member

Choose a reason for hiding this comment

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

If one or both Q and s need to be zero, could this just bee smooth min(Q, s) == 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Current expression goes Q-0.5(Q-s+|Q-s|) == 0 while the smooth min gives 0.5(Q+s-|Q-s|)==0. The complexity remains the same. The current expression keeps Q as a leading term, which might be easy for users to scale the solubility product with other equilibrium reactions.

Copy link
Member

Choose a reason for hiding this comment

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

@eslickj I think this formulation is correct - it is a bit more complicated than just one of Q or s needing to be zero.

Copy link
Member

Choose a reason for hiding this comment

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

@andrewlee94, I'm sure you are probably right. Can someone explain the difference to me? Why is this one preferred? I think both should work, but I may be missing something. To me, the the one in the code currently just seems convoluted.

Copy link
Member

Choose a reason for hiding this comment

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

@eslickj I cannot remember the details - I asked Larry Biegler to help with this is and this is the form he suggested.


@staticmethod
def calculate_scaling_factors(b, sf_keq):
Expand Down
2 changes: 1 addition & 1 deletion idaes/models/properties/modular_properties/state_definitions/FpcTP.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def mole_frac_comp(b, j):

b.mole_frac_phase_comp = Var(
b.phase_component_set,
bounds=(1e-20, 1.001),
bounds=(1e-50, 1.001),
lxhowl marked this conversation as resolved.
Show resolved Hide resolved
initialize=1 / len(b.component_list),
doc="Phase mole fractions",
units=pyunits.dimensionless,
Expand Down