Add sweep analysis for electroNP flowsheet #1077
Conversation
| # self.magnesium_chloride_cost = pyo.Param( | ||
| # mutable=True, | ||
| # initialize=0.0786, | ||
| # doc="Magnesium chloride cost", | ||
| # units=pyo.units.USD_2020 / pyo.units.kg, | ||
| # ) | ||
| # self.add_defined_flow("magnesium chloride", self.magnesium_chloride_cost) | ||
| # | ||
| # self.phosphorus_recovery_revenue = pyo.Param( | ||
| # mutable=True, | ||
| # initialize=-0.07, | ||
| # doc="Phosphorus recovery revenue", | ||
| # units=pyo.units.USD_2020 / pyo.units.kg, | ||
| # ) | ||
| # self.add_defined_flow( | ||
| # "phosphorus salt product", self.phosphorus_recovery_revenue | ||
| # ) |
|
|
||
| def set_up_sensitivity(m): | ||
| outputs = {} | ||
| # optimize_kwargs = {"fail_flag": False} |
watertap/examples/flowsheets/case_studies/electroNP/multi_sweep.py
Outdated
Show resolved
Hide resolved
watertap/unit_models/electroNP_ZO.py
Outdated
| sf = iscale.get_scaling_factor( | ||
| self.energy_electric_flow_mass, default=1e2, warning=True | ||
| ) | ||
| iscale.set_scaling_factor(self.energy_electric_flow_mass, sf) |
There was a problem hiding this comment.
Shouldn't this set_scaling_factor line be under the if conditional above?
Co-authored-by: Adam Atia <aatia@keylogic.com>
Co-authored-by: Adam Atia <aatia@keylogic.com>
Co-authored-by: Adam Atia <aatia@keylogic.com>
…p.py Co-authored-by: Adam Atia <aatia@keylogic.com>
watertap/unit_models/electroNP_ZO.py
Outdated
| sf = iscale.get_scaling_factor( | ||
| self.magnesium_chloride_dosage, default=1e1, warning=True | ||
| ) | ||
| iscale.set_scaling_factor(self.magnesium_chloride_dosage, sf) |
There was a problem hiding this comment.
Same here- this set_scaling_factor should be within the if conditional because we only want to set the scaling factor if the user has not provided one in their flowsheet. The way it is written now, if a user supplied a scaling factor for this variable and ran calculate_scaling_factors afterwards, the scaling factor they specified would be overwritten without them realizing.
There was a problem hiding this comment.
Also, just realized that you don't need both lines (get_scaling_factor and set_scaling_factor), since the first will set a default scaling factor and show a warning.
There was a problem hiding this comment.
@adam-a-a I fixed the indent problem, but I am a little bit unsure why I don't need to set_scaling_factor, I checked other files like nanofiltration_ZO.py, they also do get_scaling_factor then set_scaling_factor even with a warning=True.
Thank you for your careful review! I've addressed all revisions as you suggested. |
| for i in skip_constraint: | ||
| m.fs.electroNP.solute_removal_equation[0.0, "Liq", i].deactivate() | ||
| m.fs.electroNP.solute_treated_equation[0.0, "Liq", i].deactivate() | ||
|
|
There was a problem hiding this comment.
Have you checked why we can skip this constraints and the model still works?
There was a problem hiding this comment.
The only reason I could imagine is the removal fraction is setting to a really small value 1e-7 and these components should just pass through, but I still want to keep these removal equations just in case if we want to update these removal fractions later.
Fixes/Resolves:
(replace this with the issue # fixed or resolved, if no issue exists then a brief statement of what this PR does)
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: