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.

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

Add influent/effluent quality metrics for ASM1 #1243

Merged
merged 8 commits into from
Dec 12, 2023

Conversation

adam-a-a
Copy link
Contributor

@adam-a-a adam-a-a commented Dec 8, 2023

Fixes/Resolves:

Summary/Motivation:

This PR adds quality metrics to ASM1 which can be used to evaluate WWTP performance and should be useful for optimization cases. Said metrics/properties can now be calculated for any ASM1 stateblock.

Changes proposed in this PR:

  • add TSS
  • add BOD5- note- this is the only one indexed by "raw" or "effluent", each of which incorporate a different conversion factor
  • add COD
  • add Kjeldahl nitrogen
  • add total nitrogen
  • add associated params as needed and move a couple params from ASM1 rxn to prop model

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.

Comment on lines +412 to +414
self.BOD5 = pyo.Expression(
["raw", "effluent"],
rule=_BOD5,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe I'm just not understanding, but is there a reason these two are bundled together as opposed to having a separate expression for each?

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 only difference between the two is that for treated effluent, the whole expression should be multiplied by one value (0.25), while for another stateblock in the flowsheet (e.g., influent, stream bypassing activated sludge process, etc.), the expression is multiplied by another value (0.65). I could've made them separate but chose to do it this way. Note the BOD5_factor that I added to the parameter block is also indexed by "raw" or "effluent". (This is the factor that is 0.65 for "raw" and 0.25 for "effluent".)

Copy link

codecov bot commented Dec 8, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (06149c9) 94.84% compared to head (b615123) 94.84%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1243   +/-   ##
=======================================
  Coverage   94.84%   94.84%           
=======================================
  Files         356      356           
  Lines       35766    35793   +27     
=======================================
+ Hits        33922    33949   +27     
  Misses       1844     1844           

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

@adam-a-a adam-a-a added the iedo label Dec 11, 2023
@adam-a-a adam-a-a self-assigned this Dec 11, 2023
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 - I'm assuming this PR is done

Copy link
Contributor

@luohezhiming luohezhiming left a comment

Choose a reason for hiding this comment

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

LGTM. Once it get merged, we can move forward to do some anlysis on these key metrics.

@adam-a-a adam-a-a enabled auto-merge (squash) December 12, 2023 00:29
@adam-a-a adam-a-a merged commit dcb5c28 into watertap-org:main Dec 12, 2023
22 checks passed
@ksbeattie ksbeattie added the Priority:Normal Normal Priority Issue or PR label Dec 14, 2023
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.

5 participants