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

Bugfix in get_total_amount; CHANGELOG for next version #182

Merged
merged 4 commits into from
Sep 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions AUTHORS.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,12 @@ developed and maintained by the Kingsbury Lab at Princeton University.

Other contributors, listed alphabetically, are:

* Kirill Pushkarev (@kirill-push)
* Dhruv Duseja (@DhruvDuseja)
* Andrew Rosen (@arosen93)
* Hernan Grecco (@hgrecco)
- Arpit Bhardwaj (@abhardwaj73)
- Dhruv Duseja (@DhruvDuseja)
- Hernan Grecco (@hgrecco)
- Jaebeom Park (@Jaebeom-P)
- Kirill Pushkarev (@kirill-push)
- Andrew Rosen (@arosen93)
- Sui Xiong Tay (@SuixiongTay)

(If you think that your name belongs here, please let the maintainer know)
27 changes: 27 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,33 @@ All notable changes to this project will be documented in this file.
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).

## [1.1.6] - 2024-09-01

### Fixed

- `Solution.get_total_amount`: Bugfix that caused an error when called on certain elements
without specifying an oxidation state. For example, `get_total_amount('N')` could raise
an exception in a solution containing `Na` (but no `N`) due to a flaw in a logical
test.
- `Solution._adjust_charge_balance`: Removed a misleading and redundant log message (#162, @SuixiongTay)

### Added

- `gibbs_mix`: A new keyword argument `activity_correction` was added to `gibbs_mix`. It defaults
to `True` (no change from prior behavior), but can be set to `False` in order to calculate the
ideal mixing energy, which is equivalent to only considering entropic effects. (#178, @Jaebeom-P)
- `standardize_formula`: Improve formatting of ammonium sulfate salts. Aqueous ammonium sulfate previously
standardized to `H8S(NO2)2(aq)`, now it will display as `(NH4)2SO4(aq)`.

### Changed

- **BREAKING** `entropy_mix` now returns the ideal mixing _entropy_ in units of J/K rather than the mixing
_energy_ in J. This was done to improve clarity with respect to the function name. An `activity_correction`
kwarg was added to `gibbs_mix` so that you can still calculate the ideal mixing energy by setting it to `False`.
(#178, @Jaebeom-P)
- Revise documentation of `gibbs_mix`, `entropy_mix`, and `donnan_eql`. (#178, @Jaebeom-P)
- CI: Improve comprehensiveness of CI dependency testing. (#163, #164, @abhardwaj73)

## [1.1.5] - 2024-07-28

### Fixed
Expand Down
9 changes: 4 additions & 5 deletions src/pyEQL/solution.py
Original file line number Diff line number Diff line change
Expand Up @@ -1161,7 +1161,8 @@ def get_total_amount(self, element: str, units: str) -> Quantity:
:meth:`get_amount`
:func:`pyEQL.utils.interpret_units`
"""
TOT: Quantity = 0
_units = interpret_units(units)
TOT: Quantity = ureg.Quantity(0, _units)

# standardize the element formula and units
el = str(Element(element.split("(")[0]))
Expand All @@ -1178,7 +1179,7 @@ def get_total_amount(self, element: str, units: str) -> Quantity:
else:
species = []
for k, v in comp_by_element.items():
if el in k:
if k.split("(")[0] == el:
species.extend(v)

# loop through the species of interest, adding moles of element
Expand Down Expand Up @@ -2294,9 +2295,7 @@ def _adjust_charge_balance(self, atol=1e-8) -> None:
self.logger.info("balance_charge is None, so no charge balancing will be performed.")
return

self.logger.info(
f"Adjusting {self._cb_species} to compensate."
)
self.logger.info(f"Adjusting {self._cb_species} to compensate.")

if self.balance_charge == "pH":
# the charge imbalance associated with the H+ / OH- system can be expressed
Expand Down
6 changes: 6 additions & 0 deletions src/pyEQL/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,12 @@ def standardize_formula(formula: str):
elif sform == "C2I2ClO2[-1]":
sform = "CI2ClCOO[-1]"

# ammonium sulfate salts
elif sform == "H8S(NO2)2(aq)":
sform = "(NH4)2SO4(aq)"
elif sform == "H4SNO4[-1]":
sform = "NH4SO4[-1]"

# TODO - consider adding recognition of special formulas like MeOH for methanol or Cit for citrate
return sform

Expand Down
1 change: 1 addition & 0 deletions tests/test_solution.py
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,7 @@ def test_components_by_element(s1, s2):
def test_get_total_amount(s2):
assert np.isclose(s2.get_total_amount("Na(1)", "g").magnitude, 8 * 58, 44)
assert np.isclose(s2.get_total_amount("Na", "mol").magnitude, 8)
assert np.isclose(s2.get_total_amount("N", "mol").magnitude, 0)
assert np.isclose(s2.get_total_amount("Na", "ppm").magnitude, 4 * 23300, rtol=0.02)
sox = Solution({"Fe+2": "10 mM", "Fe+3": "40 mM", "Cl-": "50 mM"}, pH=3)
assert np.isclose(sox.get_total_amount("Fe(2)", "mol/L").magnitude, 0.01)
Expand Down
2 changes: 2 additions & 0 deletions tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ def test_standardize_formula():
# Cl+Br
assert standardize_formula("CBrCl2COO-") == "CBrCl2COO[-1]"
assert standardize_formula("CBr2ClCOO-") == "CBr2ClCOO[-1]"
assert standardize_formula("(NH4)2SO4") == "(NH4)2SO4(aq)"
assert standardize_formula("NH4SO4-") == "NH4SO4[-1]"


def test_formula_dict():
Expand Down