Add NaCl recovery value to crystallizer model #1120

Zhuoran29 · 2023-09-06T04:37:17Z

Fixes/Resolves:

Added the recovery value of NaCl from the crystallization product

Summary/Motivation:

For economic analysis purpose, specifically in SETO project, it is necessary to account for the recovery value.
In some cases the recovery value would result in a negative operating cost, which is not allowed in the current costing package. Associated change was made to it as well.

Changes proposed in this PR:

Changed the domain of total_operating_cost to pyo.Reals.
Update the solutions in three testing files that were impacted by the change above.
- watertap/examples/flowsheets/nf_dspmde/tests/test_nf.py
- watertap/examples/flowsheets/full_treatment_train/analysis/tests/test_flowsheets.py
- watertap/examples/flowsheets/full_treatment_train/flowsheet_components/chemistry/tests/test_posttreatment.py
Added a defined flow of NaCl in the crystallizer unit costing model
Added testing functions in the crystallizer tester file

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.

codecov · 2023-09-06T04:43:08Z

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (3b37c9c) 94.92% compared to head (a17266d) 94.92%.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1120   +/-   ##
=======================================
  Coverage   94.92%   94.92%           
=======================================
  Files         331      331           
  Lines       32501    32504    +3     
=======================================
+ Hits        30852    30855    +3     
  Misses       1649     1649

Files Changed	Coverage Δ
watertap/costing/watertap_costing_package.py	`100.00% <ø> (ø)`
watertap/costing/units/crystallizer.py	`100.00% <100.00%> (ø)`

☔ View full report in Codecov by Sentry.

📢 Have feedback on the report? Share it here.

Zhuoran29 · 2023-09-07T16:11:57Z

watertap/examples/flowsheets/nf_dspmde/tests/test_nf.py

        "recovery": [
            m.fs.NF.nfUnit.recovery_vol_phase[0.0, "Liq"] * 100,
-            73.47934090302432,
+            94.99999441324391,
        ],


This test has a larger difference in results. @bknueven @adam-a-a

@Zhuoran29 can you double check the value of the total_operating_cost here, both before and after the change?

@bknueven Sure, before the change total_operating_cost = 2369.18, and after the change total_operating_cost = 2429.43.

I was able to do this myself:
current main: total_operating_cost: 2369.1822124750583
With this change: total_operating_cost: 2429.430766595918

It's actually a bit higher, funnily enough ...

This change also brings the converged values without bypass much closer to those with by pass:

No Bypass, main:

total_operating_cost: 2369.182212475059 Optimal cost 0.16810666231247273 Optimal NF pressure (Bar) 6.560034522531212 Optimal area (m2) 285.7101510399154 Optimal nf recovery (%) 73.48423398501477 Feed hardness (mg/L as CaCO3) 1016.1545749479138 Product hardness (mg/L as CaCO3) 199.9999928713816 Disposal hardness (mg/L as CaCO3) 3277.997368528139

No Bypass, with this change:

total_operating_cost: 2429.430766595918 Optimal cost 0.15058960529129017 Optimal NF pressure (Bar) 8.12906860029318 Optimal area (m2) 423.89564182114776 Optimal nf recovery (%) 94.99999441324391 Feed hardness (mg/L as CaCO3) 1016.1545749479138 Product hardness (mg/L as CaCO3) 199.9995335609493 Disposal hardness (mg/L as CaCO3) 16523.08212268398

With Bypass with / without this change:

total_operating_cost: 2217.7019485486526 Optimal cost 0.1376886456374725 Optimal NF pressure (Bar) 6.702781481688757 Optimal area (m2) 419.7559334394971 Optimal nf recovery (%) 94.99999897882932 bypass (%) 10.592027708476932 Feed hardness (mg/L as CaCO3) 1016.1240639142685 Product hardness (mg/L as CaCO3) 199.9999851467023 Disposal hardness (mg/L as CaCO3) 18456.17762054043

Zhuoran29 · 2023-09-07T16:13:19Z

watertap/costing/watertap_costing_package.py

@@ -252,7 +252,7 @@ def build_process_costs(self):
        )
        self.total_operating_cost = pyo.Var(
            initialize=1e3,
-            domain=pyo.NonNegativeReals,
+            domain=pyo.Reals,
            doc="Total operating cost",


This is where I changed the domain in the costing package and it creates some different solutions in the testing files below, one of which has a difference larger than 10%. @bknueven @adam-a-a

bknueven

I guess my meta-question on this PR is if we've set other aggregates to have the domain NonNegativeReals and if they should all be changed to Reals.

Probably should be part of a follow-up, however.

bknueven

I am happy -- the change seems to resolve a local optima issue in the NF flowsheet. See: #1120 (comment)

I am unconcerned about the other test values changing -- the affected tests have always been a bit sensitive to otherwise inconsequential changes.

Zhuoran29 · 2023-09-07T18:05:36Z

I guess my meta-question on this PR is if we've set other aggregates to have the domain NonNegativeReals and if they should all be changed to Reals.

Probably should be part of a follow-up, however.

I see total_capital_cost and maintenance_labor_chemical_operating_cost are also constrained as NonNegativeReals, which should be the case in practice, but it's interesting to see if changing them to Reals has impact on the local optimal solutions as well. I tried that real quick in 'test_nf.py' but I don't see a change in total_operating_cost.

bknueven · 2023-09-07T18:31:27Z

I guess my meta-question on this PR is if we've set other aggregates to have the domain NonNegativeReals and if they should all be changed to Reals.
Probably should be part of a follow-up, however.

I see total_capital_cost and maintenance_labor_chemical_operating_cost are also constrained as NonNegativeReals, which should be the case in practice, but it's interesting to see if changing them to Reals has impact on the local optimal solutions as well. I tried that real quick in 'test_nf.py' but I don't see a change in total_operating_cost.

It looks like the zero order costing package doesn't bound these quantities at all. I would prefer not to have redundant bounds on variables. I will resolve this in #1122.

bknueven · 2023-09-07T18:33:12Z

I would like to merge this before #1122.

adam-a-a · 2023-09-07T19:00:53Z

...s/flowsheets/full_treatment_train/flowsheet_components/chemistry/tests/test_posttreatment.py

+        9.505698559578499e-07, rel=1e-3
    )
    assert model.fs.ideal_naocl_mixer_unit.outlet.flow_mol[0].value == pytest.approx(
        25.000025535888078, rel=1e-3
    )
    assert model.fs.ideal_naocl_mixer_unit.outlet.mole_frac_comp[
        0, "OCl_-"
-    ].value == pytest.approx(1.6123004572288052e-07, rel=1e-3)
+    ].value == pytest.approx(5.107133802822922e-07, rel=1e-3)

    assert model.fs.ideal_naocl_chlorination_unit.free_chlorine.value == pytest.approx(
        2, rel=1e-3
    )
    assert model.fs.ideal_naocl_chlorination_unit.outlet.mole_frac_comp[
        0, "OCl_-"
-    ].value == pytest.approx(5.0027242332010015e-08, rel=1e-3)
+    ].value == pytest.approx(4.5088044031726496e-07, rel=1e-3)
    assert model.fs.ideal_naocl_chlorination_unit.outlet.mole_frac_comp[
        0, "H_+"
-    ].value == pytest.approx(1.49011416785194e-10, rel=1e-3)
+    ].value == pytest.approx(5.4260427865375997e-11, rel=1e-3)


since these are all close to 0, wonder if they should have absolute tolerances instead of relative tolerances (what we typically want to use for ~0 values).

Yea good catch, I think that should be the case.

adam-a-a · 2023-09-07T19:01:40Z

watertap/examples/flowsheets/nf_dspmde/tests/test_nf.py

+        "lcow": [m.fs.costing.LCOW, 0.15058960529129017],
+        "pressure": [m.fs.NF.pump.outlet.pressure[0] / 1e5, 8.13],
+        "area": [m.fs.NF.nfUnit.area, 423.8956418211484],
        "recovery": [
            m.fs.NF.nfUnit.recovery_vol_phase[0.0, "Liq"] * 100,
-            73.47934090302432,
+            94.99999441324391,
        ],


FYI @avdudchenko

watertap/unit_models/tests/test_crystallizer.py

adam-a-a

LGTM - nice job!

Change to absolute tolerance for zeros. Co-authored-by: Adam Atia <aatia@keylogic.com>

watertap/unit_models/tests/test_crystallizer.py

Try GHA again; assert closer to 0

Zhuoran Zhang added 2 commits September 6, 2023 00:26

add NaCl value

0b59610

change opex domain

bd6e3c5

Zhuoran29 requested a review from adam-a-a September 6, 2023 04:37

Zhuoran29 mentioned this pull request Sep 6, 2023

Allow negative values of the total operating cost in WaterTAPCosting #1121

Closed

Zhuoran Zhang and others added 6 commits September 6, 2023 12:30

update solutions

8c0cfb0

Merge branch 'main' into crystallizer_mod

1613def

update solutions

7c88402

update solution

5afca59

update more solutions

d4d177b

merge main

f201ced

Zhuoran29 commented Sep 7, 2023

View reviewed changes

Zhuoran29 requested a review from bknueven September 7, 2023 16:13

macOS fail fix trial

3907d61

bknueven reviewed Sep 7, 2023

View reviewed changes

bknueven approved these changes Sep 7, 2023

View reviewed changes

bknueven marked this pull request as ready for review September 7, 2023 17:01

Merge branch 'main' into crystallizer_mod

74eb982

bknueven mentioned this pull request Sep 7, 2023

Unify costing APIs in WaterTAP #1122

Merged

8 tasks

adam-a-a reviewed Sep 7, 2023

View reviewed changes

watertap/unit_models/tests/test_crystallizer.py Outdated Show resolved Hide resolved

adam-a-a approved these changes Sep 7, 2023

View reviewed changes

ksbeattie added the Priority:Normal Normal Priority Issue or PR label Sep 7, 2023

ksbeattie assigned Zhuoran29 Sep 7, 2023

Update watertap/unit_models/tests/test_crystallizer.py

0b8e1b8

Change to absolute tolerance for zeros. Co-authored-by: Adam Atia <aatia@keylogic.com>

ksbeattie enabled auto-merge (squash) September 7, 2023 20:26

bknueven disabled auto-merge September 7, 2023 23:43

bknueven reviewed Sep 7, 2023

View reviewed changes

watertap/unit_models/tests/test_crystallizer.py Outdated Show resolved Hide resolved

bknueven approved these changes Sep 7, 2023

View reviewed changes

Update watertap/unit_models/tests/test_crystallizer.py

a17266d

Try GHA again; assert closer to 0

bknueven enabled auto-merge (squash) September 8, 2023 00:00

bknueven merged commit c49bf30 into watertap-org:main Sep 8, 2023
24 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add NaCl recovery value to crystallizer model #1120

Add NaCl recovery value to crystallizer model #1120

Zhuoran29 commented Sep 6, 2023 •

edited

Loading

codecov bot commented Sep 6, 2023 •

edited

Loading

Zhuoran29 Sep 7, 2023

bknueven Sep 7, 2023

Zhuoran29 Sep 7, 2023

bknueven Sep 7, 2023

bknueven Sep 7, 2023

Zhuoran29 Sep 7, 2023

bknueven left a comment

bknueven left a comment •

edited

Loading

Zhuoran29 commented Sep 7, 2023

bknueven commented Sep 7, 2023 •

edited

Loading

bknueven commented Sep 7, 2023

adam-a-a Sep 7, 2023

Zhuoran29 Sep 7, 2023

adam-a-a Sep 7, 2023

adam-a-a left a comment

Add NaCl recovery value to crystallizer model #1120

Add NaCl recovery value to crystallizer model #1120

Conversation

Zhuoran29 commented Sep 6, 2023 • edited Loading

Fixes/Resolves:

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

codecov bot commented Sep 6, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bknueven left a comment

Choose a reason for hiding this comment

bknueven left a comment • edited Loading

Choose a reason for hiding this comment

Zhuoran29 commented Sep 7, 2023

bknueven commented Sep 7, 2023 • edited Loading

bknueven commented Sep 7, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adam-a-a left a comment

Choose a reason for hiding this comment

Zhuoran29 commented Sep 6, 2023 •

edited

Loading

codecov bot commented Sep 6, 2023 •

edited

Loading

bknueven left a comment •

edited

Loading

bknueven commented Sep 7, 2023 •

edited

Loading