Skip to content

Commit

Permalink
Generator fixes (#129)
Browse files Browse the repository at this point in the history
* Pass `VoronoiInterstitialGenerator` parameters through to `TopographyAnalyzer` (bugfix)

* Pass `min_dist` from `VoronoiInterstitialGenerator` initialisation through to `InterstitialGenerator` superclass (bugfix)

* Refactor `filter_colliding` to return coords _and_ index, for correct multiplicities (bugfix)

* Refactor `multiplicies` to `multiplicities` (small typo)

* Update type hint
  • Loading branch information
kavanase authored Jul 12, 2023
1 parent adc28ca commit 4709af1
Showing 1 changed file with 22 additions and 15 deletions.
37 changes: 22 additions & 15 deletions pymatgen/analysis/defects/generators.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,38 +207,36 @@ def generate(
self,
structure: Structure,
insertions: dict[str, list[list[float]]],
multiplicies: dict[str, list[int]] | None = None,
multiplicities: dict[str, list[int]] | None = None,
**kwargs,
) -> Generator[Interstitial, None, None]:
"""Generate interstitials.
Args:
structure: The bulk structure the interstitials are generated from.
insertions: The insertions to be made given as a dictionary {"Mg": [[0.0, 0.0, 0.0], [0.5, 0.5, 0.5]]}.
multiplicies: The multiplicities of the insertions to be made given as a dictionary {"Mg": [1, 2]}.
multiplicities: The multiplicities of the insertions to be made given as a dictionary {"Mg": [1, 2]}.
**kwargs: Additional keyword arguments for the ``Interstitial`` constructor.
Returns:
Generator[Interstitial, None, None]: Generator that yields a list of ``Interstitial`` objects
"""
if multiplicies is None:
multiplicies = {
if multiplicities is None:
multiplicities = {
el_str: [1] * len(coords) for el_str, coords in insertions.items()
}

for el_str, coords in insertions.items():
for i, coord in enumerate(
self._filter_colliding(coords, structure=structure)
):
mul = multiplicies[el_str][i]
for i, coord in self._filter_colliding(coords, structure=structure):
mul = multiplicities[el_str][i]
isite = PeriodicSite(
species=Species(el_str), coords=coord, lattice=structure.lattice
)
yield Interstitial(structure, isite, multiplicity=mul, **kwargs)

def _filter_colliding(
self, fcoords: list[list[float]], structure: Structure
) -> Generator[list[float], None, None]:
) -> Generator[tuple[int, list[float]], None, None]:
"""Check the sites for collisions.
Args:
Expand All @@ -250,10 +248,10 @@ def _filter_colliding(
fcoords=list(unique_fcoords), structure=structure, min_dist=self.min_dist
)
cleaned_fcoords = set(tuple(f) for f in cleaned_fcoords)
for fc in fcoords:
for i, fc in enumerate(fcoords):
if tuple(fc) not in cleaned_fcoords:
continue
yield fc
yield i, fc


class VoronoiInterstitialGenerator(InterstitialGenerator):
Expand Down Expand Up @@ -283,7 +281,7 @@ def __init__(
self.stol = stol
self.angle_tol = angle_tol
self.top_kwargs = kwargs
super().__init__()
super().__init__(min_dist=min_dist)

def generate(self, structure: Structure, insert_species: set[str] | list[str], **kwargs) -> Generator[Interstitial, None, None]: # type: ignore[override]
"""Generate interstitials.
Expand All @@ -302,7 +300,7 @@ def generate(self, structure: Structure, insert_species: set[str] | list[str], *
yield from super().generate(
structure,
insertions={species: cand_sites},
multiplicies={species: multiplicity},
multiplicities={species: multiplicity},
**kwargs,
)

Expand All @@ -316,7 +314,16 @@ def _get_candidate_sites(
"""
framework = list(structure.symbol_set)
top = TopographyAnalyzer(
structure, framework, [], check_volume=False, **self.top_kwargs
structure,
framework,
[],
check_volume=False,
clustering_tol=self.clustering_tol,
min_dist=self.min_dist,
ltol=self.ltol,
stol=self.stol,
angle_tol=self.angle_tol,
**self.top_kwargs,
)
insert_sites = dict()
multiplicity: dict[int, int] = dict()
Expand Down Expand Up @@ -383,7 +390,7 @@ def generate(self, chgcar: Chgcar, insert_species: set[str] | list[str], **kwarg
yield from super().generate(
chgcar.structure,
insertions={species: cand_sites},
multiplicies={species: multiplicity},
multiplicities={species: multiplicity},
**kwargs,
)

Expand Down

0 comments on commit 4709af1

Please sign in to comment.