Skip to content

Commit

Permalink
Merge pull request #46 from KingsburyLab/serialize_fix
Browse files Browse the repository at this point in the history
Fix Solution serialization
  • Loading branch information
rkingsbury authored Oct 17, 2023
2 parents be2a656 + e2df120 commit 3a0078a
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 14 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,14 @@ 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).

## [Unreleased]

### Fixed

- Bugfix in `as_dict` to make serialization via `dumpfn` possible. Previously, `Quantity`
were not converted to a serializable form. Now, `Quantity` are converted to `str` in
`as_dict()`.

## [0.8.1] - 2023-10-01

### Changed
Expand Down
2 changes: 1 addition & 1 deletion setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ install_requires =
numpy
scipy
pint
pymatgen>2023.8.10
pymatgen>=2023.10.11
iapws
monty
maggma
Expand Down
10 changes: 7 additions & 3 deletions src/pyEQL/solution.py
Original file line number Diff line number Diff line change
Expand Up @@ -1768,7 +1768,7 @@ def get_activity_coefficient(
"""
Return the activity coefficient of a solute in solution.
The model used to calculte the activity coefficient is determined by the Solution's equation of state
The model used to calculate the activity coefficient is determined by the Solution's equation of state
engine.
Args:
Expand Down Expand Up @@ -2359,6 +2359,10 @@ def as_dict(self) -> dict:
if self.volume_update_required:
self._update_volume()
d = super().as_dict()
for k, v in d.items():
# convert all Quantity to str
if isinstance(v, Quantity):
d[k] = str(v)
# replace solutes with the current composition
d["solutes"] = {k: f"{v} mol" for k, v in self.components.items()}
# replace the engine with the associated str
Expand Down Expand Up @@ -2480,15 +2484,15 @@ def __add__(self, other: Solution):
def __sub__(self, other: Solution):
raise NotImplementedError("Subtraction of solutions is not implemented.")

def __mul__(self, factor: float | int):
def __mul__(self, factor: float):
"""
Solution multiplication: scale all components by a factor. For example, Solution * 2 will double the moles of
every component (including solvent). No other properties will change.
"""
self.volume *= factor
return self

def __truediv__(self, factor: float | int):
def __truediv__(self, factor: float):
"""
Solution division: scale all components by a factor. For example, Solution / 2 will remove half of the moles
of every compoonents (including solvent). No other properties will change.
Expand Down
10 changes: 1 addition & 9 deletions src/pyEQL/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,7 @@ def standardize_formula(formula: str):
charge number will always be listed explicitly and 2) the charge number will be enclosed in square brackets to remove any ambiguity in the meaning of the formula. For example, 'Na+', 'Na+1', and 'Na[+]' will all
standardize to "Na[+1]"
"""
# TODO - hack to work around issues in pymatgen Ion.reduced_formula (until fixes can be merged upstream)
from pymatgen.util.string import charge_string

ion = Ion.from_formula(formula)
rform, factor = ion.get_reduced_formula_and_factor(hydrates=False)
charge = ion._charge / factor
chg_str = charge_string(charge)
return rform + chg_str
# return Ion.from_formula(formula).reduced_formula
return Ion.from_formula(formula).reduced_formula


class FormulaDict(UserDict):
Expand Down
48 changes: 47 additions & 1 deletion tests/test_solution.py
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,7 @@ def test_arithmetic_and_copy(s2, s6):
s2 + s_bad


def test_serialization(s1, s2, s5):
def test_as_from_dict(s1, s2):
assert isinstance(s1.as_dict(), dict)
s1_new = Solution.from_dict(s1.as_dict())
assert s1_new.volume.magnitude == 2
Expand Down Expand Up @@ -530,3 +530,49 @@ def test_serialization(s1, s2, s5):
# also should point to different Store instances
# TODO currently this test will fail due to a bug in maggma's __eq__
# assert s2_new.database != s2.database


def test_serialization(s1, s2, tmpdir):
from monty.serialization import dumpfn, loadfn

dumpfn(s1, str(tmpdir / "s1.json"))
s1_new = loadfn(str(tmpdir / "s1.json"))
assert s1_new.volume.magnitude == 2
assert s1_new._solutes["H[+1]"] == "2e-07 mol"
assert s1_new.get_total_moles_solute() == s1.get_total_moles_solute()
assert s1_new.components == s1.components
assert np.isclose(s1_new.pH, s1.pH)
assert np.isclose(s1_new._pH, s1._pH)
assert np.isclose(s1_new.pE, s1.pE)
assert np.isclose(s1_new._pE, s1._pE)
assert s1_new.temperature == s1.temperature
assert s1_new.pressure == s1.pressure
assert s1_new.solvent == s1.solvent
assert s1_new._engine == s1._engine
# the solutions should point to different EOS instances
assert s1_new.engine != s1.engine
# also should point to different Store instances
# TODO currently this test will fail due to a bug in maggma's __eq__
# assert s1_new.database != s1.database

dumpfn(s2, str(tmpdir / "s2.json"))
s2_new = loadfn(str(tmpdir / "s2.json"))
assert s2_new.volume == s2.volume
# components concentrations should be the same
assert s2_new.components == s2.components
# but not point to the same instances
assert s2_new.components is not s2.components
assert s2_new.get_total_moles_solute() == s2.get_total_moles_solute()
assert np.isclose(s2_new.pH, s2.pH)
assert np.isclose(s2_new._pH, s2._pH)
assert np.isclose(s2_new.pE, s2.pE)
assert np.isclose(s2_new._pE, s2._pE)
assert s2_new.temperature == s2.temperature
assert s2_new.pressure == s2.pressure
assert s2_new.solvent == s2.solvent
assert s2_new._engine == s2._engine
# the solutions should point to different EOS instances
assert s2_new.engine != s2.engine
# also should point to different Store instances
# TODO currently this test will fail due to a bug in maggma's __eq__
# assert s2_new.database != s2.database

0 comments on commit 3a0078a

Please sign in to comment.