Skip to content

Commit

Permalink
Reset label for sites changed by Structure.replace_species() (#3672)
Browse files Browse the repository at this point in the history
* Reset label in `Structure.replace_species()`

* Add test for partial occupancy

* avoid needless trailing zeros in Site.species_string float formatting

* fix partial occu replacement test

* fix expected Si Site in test_str

---------

Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>
  • Loading branch information
stefsmeets and janosh committed Mar 5, 2024
1 parent 26cf207 commit a5a4887
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 16 deletions.
22 changes: 11 additions & 11 deletions pymatgen/analysis/phase_diagram.py
Original file line number Diff line number Diff line change
Expand Up @@ -1905,8 +1905,8 @@ def __init__(self, entry1, entry2, all_entries, tol: float = 1e-4, float_fmt="%.
number of decimal places in reaction string. Defaults to "%.4f".
"""
elem_set = set()
for ent in [entry1, entry2]:
elem_set.update([el.symbol for el in ent.elements])
for entry in [entry1, entry2]:
elem_set.update([el.symbol for el in entry.elements])

elements = tuple(elem_set) # Fix elements to ensure order.

Expand Down Expand Up @@ -1939,8 +1939,8 @@ def fmt(fl):

try:
mat = []
for ent in face_entries:
mat.append([ent.composition.get_atomic_fraction(el) for el in elements])
for entry in face_entries:
mat.append([entry.composition.get_atomic_fraction(el) for el in elements])
mat.append(comp_vec2 - comp_vec1)
matrix = np.array(mat).T
coeffs = np.linalg.solve(matrix, comp_vec2)
Expand All @@ -1967,12 +1967,12 @@ def fmt(fl):

energy = -(x * entry1.energy_per_atom + (1 - x) * entry2.energy_per_atom)

for c, ent in zip(coeffs[:-1], face_entries):
for c, entry in zip(coeffs[:-1], face_entries):
if c > tol:
r = ent.composition.reduced_composition
r = entry.composition.reduced_composition
products.append(f"{fmt(c / r.num_atoms * factor)} {r.reduced_formula}")
product_entries.append((c, ent))
energy += c * ent.energy_per_atom
product_entries.append((c, entry))
energy += c * entry.energy_per_atom

rxn_str += " + ".join(products)
comp = x * comp_vec1 + (1 - x) * comp_vec2
Expand All @@ -1995,9 +1995,9 @@ def fmt(fl):
self.entry2 = entry2
self.rxn_entries = rxn_entries
self.labels = {}
for i, ent in enumerate(rxn_entries):
self.labels[str(i + 1)] = ent.attribute
ent.name = str(i + 1)
for idx, entry in enumerate(rxn_entries):
self.labels[str(idx + 1)] = entry.attribute
entry.name = str(idx + 1)
self.all_entries = all_entries
self.pd = pd

Expand Down
5 changes: 2 additions & 3 deletions pymatgen/core/sites.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ def label(self) -> str:
return self._label if self._label is not None else self.species_string

@label.setter
def label(self, label: str) -> None:
def label(self, label: str | None) -> None:
self._label = label

@property
Expand Down Expand Up @@ -158,8 +158,7 @@ def species_string(self) -> str:
"""String representation of species on the site."""
if self.is_ordered:
return str(next(iter(self.species)))
sorted_species = sorted(self.species)
return ", ".join(f"{sp}:{self.species[sp]:.3f}" for sp in sorted_species)
return ", ".join(f"{sp}:{self.species[sp]:.3}" for sp in sorted(self.species))

@property
def specie(self) -> Element | Species | DummySpecies:
Expand Down
5 changes: 4 additions & 1 deletion pymatgen/core/structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -528,11 +528,13 @@ def replace_species(
) -> SiteCollection:
"""Swap species.
Note that this clears the label of any affected site.
Args:
species_mapping (dict): Species to swap. Species can be elements too. E.g.,
{Element("Li"): Element("Na")} performs a Li for Na substitution. The second species can
be a sp_and_occu dict. For example, a site with 0.5 Si that is passed the mapping
{Element('Si): {Element('Ge'): 0.75, Element('C'): 0.25} } will have .375 Ge and .125 C.
{Element('Si'): {Element('Ge'): 0.75, Element('C'): 0.25} } will have .375 Ge and .125 C.
in_place (bool): Whether to perform the substitution in place or modify a copy.
Defaults to True.
Expand All @@ -559,6 +561,7 @@ def replace_species(
except Exception:
comp += {new_sp: amt}
site.species = comp
site.label = None # type: ignore

return site_coll

Expand Down
2 changes: 1 addition & 1 deletion tests/core/test_sites.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ def test_repr(self):

def test_str(self):
assert str(self.site) == "[2.5 3.5 4.5] Fe"
assert str(self.site2) == "[0. 0. 0.] Si:0.500"
assert str(self.site2) == "[0. 0. 0.] Si:0.5"
assert str(self.propertied_site) == "[2.5 3.5 4.5] Fe2+"
assert str(self.labeled_site) == "[2.5 3.5 4.5] Fe"

Expand Down
11 changes: 11 additions & 0 deletions tests/core/test_structure.py
Original file line number Diff line number Diff line change
Expand Up @@ -977,6 +977,17 @@ def test_replace_species(self):
assert struct.formula == "K1.5 P0.5"
assert new_struct.formula == "Li1.5 P0.5"

def test_replace_species_labels(self):
"""https://github.com/materialsproject/pymatgen/issues/3658"""
struct = self.labeled_structure
new1 = struct.replace_species({"Si": "Ge"}, in_place=False)
assert new1.labels == ["Ge", "Ge"]

replacement = {"Si": 0.5, "Ge": 0.5}
label = ", ".join(f"{key}:{val:.3}" for key, val in replacement.items())
new2 = struct.replace_species({"Si": replacement}, in_place=False)
assert new2.labels == [label] * len(struct)

def test_append_insert_remove_replace_substitute(self):
struct = self.struct
struct.insert(1, "O", [0.5, 0.5, 0.5])
Expand Down

0 comments on commit a5a4887

Please sign in to comment.