-
Notifications
You must be signed in to change notification settings - Fork 23
Add the to_phreeqc() method
#288
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #288 +/- ##
==========================================
- Coverage 84.89% 83.71% -1.18%
==========================================
Files 9 9
Lines 1476 1498 +22
Branches 257 261 +4
==========================================
+ Hits 1253 1254 +1
- Misses 193 214 +21
Partials 30 30 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
rkingsbury
left a comment
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.
Thanks for the speedy work @SuixiongTay ! Off to a good start
| for solute, amount in self.components.items(): | ||
| amount = self.get_amount(solute, units).magnitude | ||
| if solute in ["H2O(aq)", "H[+1]", "OH[-1]"]: | ||
| continue | ||
| k = standardize_formula(solute) | ||
| spl = k.split("[") | ||
| el = spl[0] | ||
| chg = spl[1].split("]")[0] | ||
| if chg[-1] == "1": | ||
| chg = chg[0] | ||
| k = el + chg |
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.
Here, I think you will need to specify the total element concentrations via get_total_amount(), because the phreeqc convention is not to specify individual species, right?
Also, any formula that is coming out of components is guaranteed to already be standardized, so it's not necessary to call standardize_formula on it
| target_mol = quantity.to("moles", "chem", mw=mw, volume=self.volume, solvent_mass=self.solvent_mass) | ||
| self.components[formula] = target_mol.to("moles").magnitude | ||
|
|
||
| def to_phreeqc(self, units: str = "mol/L", charge_balance: bool = "False") -> str: |
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.
The charge_balance should not be a kwarg here, but rather be read from the Solution.balance_charge attribute which is set on __init__. See here for the documentation.
This will require some careful handling depending on the different cases - the pH case is easy and will work very much like you've coded below.
If 'balance_charge' is another species, you'll have to determine the associated element (e.g., Na+ -> Na) and then append the charge to that line (I think).
| the solution. | ||
| """ | ||
| lines = ["SOLUTION 0"] | ||
| lines.append(f" temp {self.temperature.to('degC').magnitude:.2f}") |
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.
Solution also has pressure and volume attributes. How does PHREEQC handle those? If possible, pass them into the input file, too
|
One other question / idea I had - we already have In that case, the @SuixiongTay @YitongPan1 @vineetbansal what are your thoughts on this? |
Summary
Solution.to_phreeqc()method to theSolution()class.Todos:
Solution()schema to PHREEQCSOLUTIONkeywordS(6)to describeSO4[2-])atmospherechg) from formula (k = el + chg) and refactortest_to_phreeqc()Solution()kwargPHREEQC input:
Format output should match the PHREEQC example below:
Addressing PR #287