-
Notifications
You must be signed in to change notification settings - Fork 61
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 Acid-Base Eq.Constraints to ADM1 Documentation #1013
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1013 +/- ##
=======================================
Coverage 95.82% 95.82%
=======================================
Files 301 301
Lines 28772 28772
=======================================
Hits 27572 27572
Misses 1200 1200 ☔ View full report in Codecov by Sentry. |
Just for clarification, this PR is pretty much done. I just want to double check that I got all the equations right and that any other changes I made are accurate. So this should be marked as "ready for review" soon |
@agarciadiego For our implementation of the Kw, Ka,co2, and Ka,IN equations in ADM1 and modified ADM1, why do we not multiply the gas constant by 100 as done in the literature? Maybe there's a reason and I just missed it... |
@MarcusHolly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job on this. I only think we need to recheck how certain items are categorized and ensure they are under the correct sections (e.g., S_H
is an additional variable rather than a parameter)
Fixes/Resolves:
Part of the master issue #934
Changes proposed in this PR:
Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: