Skip to content

Development of ASM3 Model #1593

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.

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

Open
wants to merge 145 commits into
base: main
Choose a base branch
from
Open

Conversation

luohezhiming
Copy link
Contributor

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:

  1. I agree my contributions are submitted under the license terms described in the LICENSE.txt file at the top level of this directory.
  2. 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.

@luohezhiming luohezhiming self-assigned this Jun 10, 2025
@lbianchi-lbl lbianchi-lbl added the Priority:Normal Normal Priority Issue or PR label Jun 12, 2025
Comment on lines 506 to 578
# def _TSS(self):
# tss = (
# self.conc_mass_comp["X_S"]
# + self.conc_mass_comp["X_I"]
# + self.conc_mass_comp["X_BH"]
# + self.conc_mass_comp["X_BA"]
# + self.conc_mass_comp["X_P"]
# )
# return self.params.COD_to_SS * tss
#
# self.TSS = pyo.Expression(
# rule=_TSS,
# doc="Total suspended solids (TSS)",
# )
#
# def _BOD5(self, i):
# bod5 = (
# self.conc_mass_comp["S_S"]
# + self.conc_mass_comp["X_S"]
# + (1 - self.params.f_p)
# * (self.conc_mass_comp["X_BH"] + self.conc_mass_comp["X_BA"])
# )
# # TODO: 0.25 should be a parameter instead as it changes by influent/effluent
# return self.params.BOD5_factor[i] * bod5
#
# self.BOD5 = pyo.Expression(
# ["raw", "effluent"],
# rule=_BOD5,
# doc="Five-day Biological Oxygen Demand (BOD5)",
# )
#
# def _COD(self):
# cod = (
# self.conc_mass_comp["S_S"]
# + self.conc_mass_comp["S_I"]
# + self.conc_mass_comp["X_S"]
# + self.conc_mass_comp["X_I"]
# + self.conc_mass_comp["X_BH"]
# + self.conc_mass_comp["X_BA"]
# + self.conc_mass_comp["X_P"]
# )
# return cod
#
# self.COD = pyo.Expression(
# rule=_COD,
# doc="Chemical Oxygen Demand",
# )
#
# def _TKN(self):
# tkn = (
# self.conc_mass_comp["S_NH4"]
# + self.conc_mass_comp["S_ND"]
# + self.conc_mass_comp["X_ND"]
# + self.params.i_xb
# * (self.conc_mass_comp["X_BH"] + self.conc_mass_comp["X_BA"])
# + self.params.i_xp
# * (self.conc_mass_comp["X_P"] + self.conc_mass_comp["X_I"])
# )
# return tkn
#
# self.TKN = pyo.Expression(
# rule=_TKN,
# doc="Total Kjeldahl Nitrogen",
# )
#
# def _Total_N(self):
# totaln = self.TKN + self.conc_mass_comp["S_NOX"]
# return totaln
#
# self.Total_N = pyo.Expression(
# rule=_Total_N,
# doc="Total Nitrogen",
# )
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the only missing piece according to Chenyu. Should just need to identify these equations in literature.

@luohezhiming luohezhiming marked this pull request as ready for review June 26, 2025 18:35
Copy link
Contributor

@MarcusHolly MarcusHolly left a comment

Choose a reason for hiding this comment

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

A few comments/questions. Overall great work on this tho!

self.H2O = Solvent()

# Soluble Components
self.S_O = Solute(doc="Dissolved Oxygen, S_O")
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be S_O or S_O2. Looking at the ASM3 files, it looks like they use SO2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The original paper for ASM3 use S_O, the review paper use S_O2. ASM1 use S_O, but ASM2d use S_O2, I think it is OK to just keep it as S_O cause ASM3 is more close to ASM1

{
"alkalinity": {"method": None},
"TSS": {"method": "_TSS"},
# "BOD5": {"method": "_BOD5"},
Copy link
Contributor

@MarcusHolly MarcusHolly Jun 26, 2025

Choose a reason for hiding this comment

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

I also have not been able to find a relationship for this effluent metric with our current resources unless the components in https://app.box.com/file/1130844180583 (X_B,H & X_B,A) can be directly mapped on to components in this model (X_H and X_A, respectively), which might be the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

X_H in ASM3 is Heterotrophic organisms, while X_B,H in ASM1 is Active heterotrophic biomass, looks similar.
However, X_A in ASM3 is Nitrifying organisms, while X_B,A in ASM1 is Active autotrophic biomass, looks different.
So I don't think we can make this assumption that they can be directly mapped.

t12 = self.f_XI * self.i_SSXI - self.i_SSBM

# Reaction Stoichiometry
# This is the stoichiometric part the Peterson matrix in dict form
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this matrix on Box somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

* b.conc_mass_comp_ref["X_H"],
to_units=pyo.units.kg / pyo.units.m**3 / pyo.units.s,
)
elif r == "R5":
Copy link
Contributor

Choose a reason for hiding this comment

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

Double check this equation - I think you may be missing some NOX terms

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you! Updated the NOX term

self.X_STO = Solute(
doc="A cell internal storage product of heterotrophic organisms, X_STO"
)
self.X_A = Solute(doc="Nitrifying organisms, X_A")
Copy link
Contributor

@MarcusHolly MarcusHolly Jun 27, 2025

Choose a reason for hiding this comment

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

Is this docstring accurate? This also loosely ties to the BOD5 comment

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, X_A is Nitrifying organisms

Copy link
Contributor

@MarcusHolly MarcusHolly left a comment

Choose a reason for hiding this comment

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

LGTM - documentation should eventually be added as well, but that doesn't necessarily need to happen in this PR

)

theta_T_k_H = pyo.log(self.params.k_H["10C"] / self.params.k_H["20C"]) / (
10 - 20
Copy link
Contributor

Choose a reason for hiding this comment

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

These 10 and 20 values are what I was referring to on our dev call. Technically, we can make these Params.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK. Got it, will do!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@adam-a-a finisehd, add parameters ref_temp_1 and ref_temp_2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
iedo Priority:Normal Normal Priority Issue or PR property_model
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants