ElectroNP ZO model with UnitModelBlockData base #1020

luohezhiming · 2023-05-02T21:45:24Z

Fixes/Resolves:

Add a electroNP ZO model compatible with ASM2d

Summary/Motivation:

Changes proposed in this PR:

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-05-02T22:27:46Z

Codecov Report

Attention: Patch coverage is 97.40260% with 4 lines in your changes missing coverage. Please review.

Project coverage is 95.83%. Comparing base (cd2ec4d) to head (b6d734b).
Report is 329 commits behind head on main.

Files with missing lines	Patch %	Lines
watertap/unit_models/electroNP_ZO.py	96.92%	4 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##             main    #1020    +/-   ##
========================================
  Coverage   95.82%   95.83%            
========================================
  Files         299      301     +2     
  Lines       28647    28801   +154     
========================================
+ Hits        27451    27601   +150     
- Misses       1196     1200     +4

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

watertap/costing/units/electroNP.py

watertap/unit_models/electroNP_ZO.py

MarcusHolly · 2023-05-03T19:40:35Z

watertap/unit_models/tests/test_electroNP_ZO.py

+    @pytest.mark.solver
+    @pytest.mark.skipif(solver is None, reason="Solver not available")
+    @pytest.mark.component
+    def test_solution(self, ElectroNP_frame):


Is it worth listing out the solution for the byproduct stream too?

Since we don't have any byproduct, so I don't think we need to list them all.

I thought the byproduct stream would include the S_NH4 and S_PO4 that got removed?

I agree with @MarcusHolly. The treated stream would be fed back to the headworks of the plant or right before the activated sludge process, while the byproduct stream would comprise the recovered nutrients.

I added byproduct test for S_PO4 and S_NH4, but to be noticed, since we set the water recovery as 1, so the byproduct stream is assigned to a calculation error value extremely close to 0. So the byproduct concentration will be extremely high and does not make much sense.

@luohezhiming Maybe just add an assert value(m.fs.unit.byproduct.flow_vol[0]) so that it's more clear why the byproduct values are so high?

Co-authored-by: MarcusHolly <96305519+MarcusHolly@users.noreply.github.com>

adam-a-a · 2023-05-04T21:06:16Z

watertap/unit_models/electroNP_ZO.py

+        # Add performance variables
+        self.recovery_frac_mass_H2O = Var(
+            self.flowsheet().time,
+            initialize=0.8,
+            domain=NonNegativeReals,
+            units=pyunits.dimensionless,
+            bounds=(0.0, 1.0000001),
+            doc="Mass recovery fraction of water in the treated stream",
+        )


Now that we're making this its own unit, separate from the ZO model framework, I wonder if we even need this recovery variable here. If included, I'd think the value would be very close to 1.

I choose to fix the recovery to 1.

adam-a-a · 2023-05-04T21:08:20Z

watertap/unit_models/electroNP_ZO.py

+        # default water recovery
+        @self.Constraint(self.flowsheet().time, doc="Default water recovery equation")
+        def default_water_recovery_equation(b, t):
+            return b.recovery_frac_mass_H2O[t] == 1


I would just set the recovery_frac_mass_H2O variable to an initial value and then fix the variable within this build instead of having this constraint.

Yeah, modified.

adam-a-a · 2023-05-04T21:22:38Z

watertap/unit_models/electroNP_ZO.py

+        )
+        def default_solute_removal_equation(b, t, j):
+            if hasattr(self.config.property_package.solute_set, "S_PO4"):
+                return b.removal_frac_mass_comp[t, "S_PO4"] == 0.98


I think I would maybe have parameters defined for the values, then fix removal fractions directly to those parameter values. The user could change the parameter values as desired and have them fixed.

I add removal_P and removal_N as two mutable parameters.

watertap/unit_models/electroNP_ZO.py

adam-a-a · 2023-05-04T21:30:03Z

watertap/unit_models/tests/test_electroNP_ZO.py

+        m = ConcreteModel()
+        m.fs = FlowsheetBlock(dynamic=False)
+
+        m.fs.properties = ModifiedASM2dParameterBlock(


Hmmm.... I just thought about this, and there is a chance, due to how we've compartmentalized the property and reaction models, that some expressions/additional variables/additional constraints implemented in the reaction block could be desired in the unit model (but not sure yet since I didn't think about it deeply). Maybe this isn't an issue yet, and my concern is probably more relevant to our ADM1 formulation rather than ASM2D, but let's keep this in mind since we expect to make additions to the modified ASM2d property/reaction package.

Yeah, this is a good point, some additional variables can be added to the unit model instead of properties model. Just for modified ASM2D package, we might need to keep the additonal_solute config for now.

adam-a-a · 2023-05-04T21:34:07Z

watertap/unit_models/electroNP_ZO.py

+        )
+        def default_solute_removal_equation(b, t, j):
+            if hasattr(self.config.property_package.solute_set, "S_PO4"):
+                return b.removal_frac_mass_comp[t, "S_PO4"] == 0.98


We should probably make a note to go over this with electroN-P team and get their take on whether we should be summing soluble nitrogen (e..g., NH4 + NH2 + NO3 as nitrogen) or if this should suffice.

Added a note to remind us after we talk with electroN-P team.

adam-a-a · 2023-05-04T21:36:22Z

watertap/unit_models/electroNP_ZO.py

+        for (t, p, j), v in self.solute_removal_equation.items():
+            iscale.constraint_scaling_transform(
+                v,
+                iscale.get_scaling_factor(
+                    self.properties_in[t].get_material_flow_terms(p, j),
+                    default=1,
+                    warning=True,
+                    hint=" for solute removal",
+                ),


might not need these scaling transforms if we fix the variables and not build as constraints.

I still keep the variables for now.

adam-a-a · 2023-05-04T21:39:21Z

watertap/unit_models/tests/test_electroNP_ZO.py

+        m.fs.unit.inlet.alkalinity[0].fix(61 / 12 * units.mmol / units.liter)
+
+        # Unit option
+        m.fs.unit.energy_electric_flow_mass.fix(0.044 * units.kWh / units.kg)


Something just came to mind- we can probably later convert this model to not only focus on ZO formulation. Instead, we could consider having config options would would select between fixed performance or predictive performance from surrogates. In ZO config, removal fractions, etc., would just be fixed to variables, while in predictive mode, the constraint would be activated and the surrogate would be inserted there. Just food for thought.

It is a good idea, we could upgrade this model to be a surrogate modle with config option to choose, and I believe I can save a lot of time for building some basic vairables. I still name it as electroNP_ZO for now but would consider to rename it when we add the surrogate model.

Co-authored-by: Adam Atia <aatia@keylogic.com>

…into electroNP

MarcusHolly

Made one more small comment, but otherwise LGTM

luohezhiming added 14 commits March 16, 2023 16:00

add

7d35890

Merge branch 'main' of https://github.com/watertap-org/watertap

4766c52

Merge branch 'main' of https://github.com/watertap-org/watertap

ef81bae

Merge branch 'main' of https://github.com/watertap-org/watertap

168bbaf

Merge branch 'main' of https://github.com/watertap-org/watertap

a09b4e9

Merge branch 'main' of https://github.com/watertap-org/watertap

44a22e8

Merge branch 'main' of https://github.com/watertap-org/watertap

1c1a135

Merge branch 'main' of https://github.com/watertap-org/watertap

401f997

delete redundant files

258f6fe

Merge branch 'main' of https://github.com/watertap-org/watertap

2c994c3

Merge branch 'main' of https://github.com/watertap-org/watertap

5b6baed

Merge branch 'main' of https://github.com/watertap-org/watertap

0afa551

Merge branch 'main' of https://github.com/watertap-org/watertap

479fb6d

Add ElectroNP ZO model

f962022

luohezhiming requested review from adam-a-a, TimBartholomew, agarciadiego and MarcusHolly May 2, 2023 21:45

luohezhiming self-assigned this May 2, 2023

MarcusHolly reviewed May 3, 2023

View reviewed changes

watertap/costing/units/electroNP.py Outdated Show resolved Hide resolved

MarcusHolly reviewed May 3, 2023

View reviewed changes

watertap/unit_models/electroNP_ZO.py Outdated Show resolved Hide resolved

refine code

2dabf05

MarcusHolly reviewed May 3, 2023

View reviewed changes

watertap/unit_models/electroNP_ZO.py Outdated Show resolved Hide resolved

MarcusHolly reviewed May 3, 2023

View reviewed changes

Update watertap/unit_models/electroNP_ZO.py

31c8be6

Co-authored-by: MarcusHolly <96305519+MarcusHolly@users.noreply.github.com>

lbianchi-lbl added iedo Priority:Normal Normal Priority Issue or PR labels May 4, 2023

lbianchi-lbl requested a review from yalinli2 May 4, 2023 20:12

adam-a-a mentioned this pull request May 4, 2023

Implementing Benchmark Simulation Model No. 2 #934

Open

51 tasks

adam-a-a changed the title ~~ElectroNP ZO model~~ ElectroNP ZO model with UnitModelBlockData base May 4, 2023

adam-a-a reviewed May 4, 2023

View reviewed changes

watertap/unit_models/electroNP_ZO.py Outdated Show resolved Hide resolved

adam-a-a reviewed May 4, 2023

View reviewed changes

luohezhiming and others added 4 commits May 4, 2023 18:19

Update watertap/unit_models/electroNP_ZO.py

d31ce21

Co-authored-by: Adam Atia <aatia@keylogic.com>

refine model

7e67624

Merge branch 'electroNP' of https://github.com/luohezhiming/watertap …

5a20331

…into electroNP

Merge branch 'main' of https://github.com/watertap-org/watertap

b393ed0

luohezhiming requested review from adam-a-a and MarcusHolly May 9, 2023 15:09

Merge branch 'main' into electroNP

6444221

MarcusHolly approved these changes May 9, 2023

View reviewed changes

agarciadiego approved these changes May 9, 2023

View reviewed changes

Merge branch 'main' into electroNP

b6d734b

ksbeattie enabled auto-merge (squash) May 11, 2023 22:15

ksbeattie merged commit 1334584 into watertap-org:main May 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ElectroNP ZO model with UnitModelBlockData base #1020

ElectroNP ZO model with UnitModelBlockData base #1020

luohezhiming commented May 2, 2023

codecov bot commented May 2, 2023 •

edited

Loading

MarcusHolly May 3, 2023

luohezhiming May 3, 2023

MarcusHolly May 3, 2023

adam-a-a May 4, 2023

luohezhiming May 4, 2023 •

edited

Loading

MarcusHolly May 9, 2023

adam-a-a May 4, 2023 •

edited

Loading

luohezhiming May 4, 2023

adam-a-a May 4, 2023

luohezhiming May 4, 2023

adam-a-a May 4, 2023

luohezhiming May 4, 2023

adam-a-a May 4, 2023

luohezhiming May 4, 2023

adam-a-a May 4, 2023

luohezhiming May 4, 2023

adam-a-a May 4, 2023

luohezhiming May 4, 2023

adam-a-a May 4, 2023

luohezhiming May 4, 2023 •

edited

Loading

MarcusHolly left a comment

ElectroNP ZO model with UnitModelBlockData base #1020

ElectroNP ZO model with UnitModelBlockData base #1020

Conversation

luohezhiming commented May 2, 2023

Fixes/Resolves:

Summary/Motivation:

Changes proposed in this PR:

Legal Acknowledgement

codecov bot commented May 2, 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

luohezhiming May 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adam-a-a May 4, 2023 • edited Loading

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

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

Choose a reason for hiding this comment

luohezhiming May 4, 2023 • edited Loading

Choose a reason for hiding this comment

MarcusHolly left a comment

Choose a reason for hiding this comment

codecov bot commented May 2, 2023 •

edited

Loading

luohezhiming May 4, 2023 •

edited

Loading

adam-a-a May 4, 2023 •

edited

Loading

luohezhiming May 4, 2023 •

edited

Loading