-
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 molar flows for precipitate and reagent to simplify reaktoro integration #1498
base: main
Are you sure you want to change the base?
Add molar flows for precipitate and reagent to simplify reaktoro integration #1498
Conversation
@OOAmusat @zacharybinger can you review? |
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.
This looks fine to me.
@@ -334,21 +329,28 @@ def build(self): | |||
self.reagent_dose = Var( | |||
self.reagent_list, | |||
initialize=1, | |||
domain=NonNegativeReals, | |||
domain=Reals, |
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.
@avdudchenko Why reals instead of non?
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.
We can just find out if this ends up causing issues (unexpected and undesired negative values) later on.
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.
LGTM
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.
Tests pass and changes make sense. LGTM
Fixes/Resolves:
N/A
(replace this with the issue # fixed or resolved, if no issue exists then a brief statement of what this PR does)
Summary/Motivation:
Adds molar flows of reagents and precipitates to simplify integration with reaktoro.
Changes proposed in this PR:
Legal Acknowledgement
By contributing to this software project, I agree to the following terms and conditions for my contribution: