Skip to content
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
4 changes: 2 additions & 2 deletions docs/scripts/generate_notebooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -441,7 +441,7 @@ def generate_argyrodite_site_analysis():
"coords = np.array(\n"
" [[0.5, 0.5, 0.5], # P (type 0) - PS4 tetrahedra\n"
" [0.9, 0.9, 0.6], # type 1 (Li in reference)\n"
" [0.23, 0.92, 0.09], # type 2 (Mg in reference)\n"
" [0.77, 0.585, 0.585], # type 2 (Mg in reference, 48h)\n"
" [0.25, 0.25, 0.25], # type 3 (Na in reference)\n"
" [0.15, 0.15, 0.15], # type 4 (Be in reference)\n"
" [0.0, 0.183, 0.183], # type 5 (K in reference)\n"
Expand Down Expand Up @@ -710,7 +710,7 @@ def generate_residence_times_and_transitions():
"coords = np.array(\n"
" [[0.5, 0.5, 0.5], # P (type 0) - PS4 tetrahedra\n"
" [0.9, 0.9, 0.6], # type 1 (Li in reference)\n"
" [0.23, 0.92, 0.09], # type 2 (Mg in reference)\n"
" [0.77, 0.585, 0.585], # type 2 (Mg in reference, 48h)\n"
" [0.25, 0.25, 0.25], # type 3 (Na in reference)\n"
" [0.15, 0.15, 0.15], # type 4 (Be in reference)\n"
" [0.0, 0.183, 0.183], # type 5 (K in reference)\n"
Expand Down
49 changes: 49 additions & 0 deletions docs/source/guides/builders.md
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,25 @@ Sets species to use for mapping sites between reference and target structures.
builder.with_site_mapping(mapping_species=["O", "Ti"])
```

#### `with_min_atom_distance(distance)`
Sets the minimum allowed distance between same-species atoms in the reference structure.

When `build()` is called with a reference structure, the builder checks that no two atoms of the same species are closer than this threshold. This catches a common mistake where an atom coordinate sits on a general Wyckoff position instead of the correct special position, which produces pairs of atoms very close together and results in duplicate coordination environments.

This check runs whenever a reference structure is set, regardless of site type. Set to 0 to disable the check.

**Parameters:**
- `distance`: Minimum distance in the same units as the lattice parameters. Must be non-negative. Default is 0.5.

**Examples:**
```python
# Tighten the threshold
builder.with_min_atom_distance(1.0)

# Disable the check entirely
builder.with_min_atom_distance(0)
```

#### `with_existing_sites(sites)`
Uses pre-existing site objects instead of creating new ones.

Expand Down Expand Up @@ -511,3 +530,33 @@ trajectory = builder.build() # Raises TypeError
builder.with_polyhedral_sites(...) # Without with_reference_structure()
trajectory = builder.build() # Raises ValueError
```

### Reference Structure Validation

When a reference structure is set, `build()` checks for two problems that can produce incorrect site definitions:

**Close same-species atoms**: If any pair of atoms of the same species are closer than `min_atom_distance` (default 0.5), the build fails with a `ValueError`. This typically means an atom coordinate is on a general Wyckoff position instead of the correct special position, producing near-duplicate atoms in the same coordination environment.

```python
# This will raise ValueError because the Mg coordinate is on a
# general position (96i), producing Mg pairs ~0.14 apart
builder.with_reference_structure(bad_reference)
.with_polyhedral_sites(...)
trajectory = builder.build()
# ValueError: Reference structure has Mg atoms at indices 42 and 43
# that are only 0.140 apart (threshold: 0.5). ...

# To disable this check (e.g. if close atoms are intentional):
builder.with_min_atom_distance(0)
```

**Duplicate sites**: After generating sites, the builder checks that no two polyhedral sites share the same vertex indices and no two dynamic Voronoi sites share the same reference indices. Duplicates indicate that the reference structure has multiple atoms inside the same coordination environment.

```python
# Even with the distance check disabled, duplicate sites are caught
builder.with_min_atom_distance(0)
.with_polyhedral_sites(...)
trajectory = builder.build()
# ValueError: Duplicate sites: site 0 and site 1 share the same
# vertex_indices [3, 7, 12, 15]. ...
```
49 changes: 4 additions & 45 deletions docs/source/tutorials/argyrodite_site_definitions.ipynb
Original file line number Diff line number Diff line change
Expand Up @@ -61,51 +61,10 @@
},
{
"cell_type": "code",
"execution_count": 2,
"execution_count": null,
"metadata": {},
"outputs": [
{
"name": "stdout",
"output_type": "stream",
"text": [
"Reference structure contains 1664 atoms\n",
"Composition: K384 Na32 Li128 Mg768 Be128 P32 S192\n"
]
}
],
"source": [
"# Create a reference structure with the argyrodite topology\n",
"# The key approach: use different atom types to differentiate each site type\n",
"# - P occupies the t0 tetrahedra (phosphorus in PS4 units)\n",
"# - Different dummy atoms (Li, Mg, Na, Be, K) occupy the t1-t5 tetrahedra\n",
"# to allow us to define each tetrahedral site type separately\n",
"# - S occupies all the anion sites\n",
"\n",
"lattice = Lattice.cubic(a=10.155) # Use the experimental lattice parameter\n",
"\n",
"coords = np.array(\n",
" [[0.5, 0.5, 0.5], # P (t0) - PS4 tetrahedra positions\n",
" [0.9, 0.9, 0.6], # t1 - first type of Li site (represented by Li atoms)\n",
" [0.23, 0.92, 0.09], # t2 - second type of Li site (represented by Mg atoms)\n",
" [0.25, 0.25, 0.25], # t3 - third type of Li site (represented by Na atoms)\n",
" [0.15, 0.15, 0.15], # t4 - fourth type of Li site (represented by Be atoms)\n",
" [0.0, 0.183, 0.183], # t5 - fifth type of Li site (represented by K atoms)\n",
" [0.0, 0.0, 0.0], # S - anion position (4a site)\n",
" [0.75, 0.25, 0.25], # S - anion position (4c site)\n",
" [0.11824, 0.11824, 0.38176]] # S - anion position (16e site)\n",
") \n",
"\n",
"# Create the reference structure with F-43m space group symmetry\n",
"# and replicate it as a 2x2x2 supercell to match the MD simulations\n",
"reference_structure = Structure.from_spacegroup(\n",
" sg=\"F-43m\",\n",
" lattice=lattice,\n",
" species=['P', 'Li', 'Mg', 'Na', 'Be', 'K', 'S', 'S', 'S'],\n",
" coords=coords) * [2, 2, 2]\n",
"\n",
"print(f\"Reference structure contains {len(reference_structure)} atoms\")\n",
"print(f\"Composition: {reference_structure.composition.formula}\")"
]
"outputs": [],
"source": "# Create a reference structure with the argyrodite topology\n# The key approach: use different atom types to differentiate each site type\n# - P occupies the t0 tetrahedra (phosphorus in PS4 units)\n# - Different dummy atoms (Li, Mg, Na, Be, K) occupy the t1-t5 tetrahedra\n# to allow us to define each tetrahedral site type separately\n# - S occupies all the anion sites\n\nlattice = Lattice.cubic(a=10.155) # Use the experimental lattice parameter\n\ncoords = np.array(\n [[0.5, 0.5, 0.5], # P (t0) - PS4 tetrahedra positions (4b)\n [0.9, 0.9, 0.6], # t1 - first type of Li site (16e)\n [0.77, 0.585, 0.585], # t2 - second type of Li site (48h)\n [0.25, 0.25, 0.25], # t3 - third type of Li site (4c)\n [0.15, 0.15, 0.15], # t4 - fourth type of Li site (16e)\n [0.0, 0.183, 0.183], # t5 - fifth type of Li site (48h)\n [0.0, 0.0, 0.0], # S - anion position (4a site)\n [0.75, 0.25, 0.25], # S - anion position (4c site)\n [0.11824, 0.11824, 0.38176]] # S - anion position (16e site)\n) \n\n# Create the reference structure with F-43m space group symmetry\n# and replicate it as a 2x2x2 supercell to match the MD simulations\nreference_structure = Structure.from_spacegroup(\n sg=\"F-43m\",\n lattice=lattice,\n species=['P', 'Li', 'Mg', 'Na', 'Be', 'K', 'S', 'S', 'S'],\n coords=coords) * [2, 2, 2]\n\nprint(f\"Reference structure contains {len(reference_structure)} atoms\")\nprint(f\"Composition: {reference_structure.composition.formula}\")"
},
{
"cell_type": "markdown",
Expand Down Expand Up @@ -517,4 +476,4 @@
},
"nbformat": 4,
"nbformat_minor": 4
}
}
2 changes: 1 addition & 1 deletion docs/source/tutorials/residence_times_and_transitions.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ lattice = Lattice.cubic(a=10.155)
coords = np.array(
[[0.5, 0.5, 0.5], # P (type 0) - PS4 tetrahedra
[0.9, 0.9, 0.6], # type 1 (Li in reference)
[0.23, 0.92, 0.09], # type 2 (Mg in reference)
[0.77, 0.585, 0.585], # type 2 (Mg in reference, 48h)
[0.25, 0.25, 0.25], # type 3 (Na in reference)
[0.15, 0.15, 0.15], # type 4 (Be in reference)
[0.0, 0.183, 0.183], # type 5 (K in reference)
Expand Down
138 changes: 130 additions & 8 deletions site_analysis/builders.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,10 @@ def reset(self) -> 'TrajectoryBuilder':

# Mapping options
self._mapping_species: list[str] | None = None


# Validation options
self._min_atom_distance: float = 0.5

# Functions to be called during build() to create sites
self._site_generators: list[Callable] = []

Expand Down Expand Up @@ -282,6 +285,41 @@ def with_site_mapping(self, mapping_species: str | list[str] | None) -> Trajecto
self._mapping_species = mapping_species
return self

def with_min_atom_distance(self, distance: float) -> 'TrajectoryBuilder':
"""Set the minimum allowed distance between same-species atoms
in the reference structure.

If any pair of atoms of the same species in the reference
structure is closer than this threshold, ``build()`` raises
``ValueError``. This catches reference structures where atoms
sit on a general Wyckoff position instead of the correct
special position, producing duplicate coordination environments.

This check runs whenever a reference structure is set,
regardless of site type. It is most relevant for polyhedral
and dynamic Voronoi site workflows, but also applies when a
reference structure is provided for mapping or alignment.

Set to 0 to disable the check.

Args:
distance: Minimum distance in the same units as the
lattice parameters. Must be non-negative.
The builder default is 0.5.

Returns:
self: For method chaining.

Raises:
ValueError: If distance is negative.
"""
if distance < 0:
raise ValueError(
f"min_atom_distance must be non-negative, got {distance}"
)
self._min_atom_distance = distance
return self

def with_spherical_sites(self,
centres: list[list[float]],
radii: float | list[float],
Expand Down Expand Up @@ -630,34 +668,115 @@ def with_existing_atoms(self, atoms: list) -> TrajectoryBuilder:
self._atoms = atoms
return self

def _validate_reference_atom_distances(self) -> None:
"""Check that no same-species atom pairs in the reference
structure are closer than ``_min_atom_distance``."""
from site_analysis.distances import all_mic_distances
from site_analysis.tools import indices_for_species

ref = self._reference_structure
lattice_matrix = np.array(ref.lattice.matrix) # type: ignore[union-attr]
species_list = [s.species_string for s in ref] # type: ignore[union-attr]
frac_coords = np.array(ref.frac_coords) # type: ignore[union-attr]

for sp in set(species_list):
indices = indices_for_species(species_list, sp)
if len(indices) < 2:
continue

coords = frac_coords[indices]
dists = all_mic_distances(coords, coords, lattice_matrix)
np.fill_diagonal(dists, np.inf)

min_dist = float(np.min(dists))
if min_dist < self._min_atom_distance:
i_local, j_local = np.unravel_index(
int(np.argmin(dists)), dists.shape)
raise ValueError(
f"Reference structure has {sp} atoms at indices "
f"{indices[i_local]} and {indices[j_local]} that are "
f"only {min_dist:.3f} apart (threshold: "
f"{self._min_atom_distance}). This typically means "
f"the atom coordinate is on a general Wyckoff position "
f"instead of the correct special position, which "
f"produces duplicate coordination environments. "
f"To disable this check, call "
f".with_min_atom_distance(0) on the builder."
)

def _validate_unique_sites(self, sites: list[Site]) -> None:
"""Check that no two sites share the same defining indices.

Applies to PolyhedralSite (vertex_indices) and
DynamicVoronoiSite (reference_indices). Skipped for
VoronoiSite and SphericalSite where duplicate detection
based on coordinates is unreliable.
"""
from site_analysis.polyhedral_site import PolyhedralSite
from site_analysis.dynamic_voronoi_site import DynamicVoronoiSite
from site_analysis.voronoi_site import VoronoiSite
from site_analysis.spherical_site import SphericalSite

if not sites:
return

if isinstance(sites[0], PolyhedralSite):
index_attr = "vertex_indices"
elif isinstance(sites[0], DynamicVoronoiSite):
index_attr = "reference_indices"
elif isinstance(sites[0], (VoronoiSite, SphericalSite)):
return
Comment thread
bjmorgan marked this conversation as resolved.
else:
return # Unknown site type — skip gracefully

seen: dict[frozenset[int], int] = {}
for site in sites:
key = frozenset(getattr(site, index_attr))
if key in seen:
raise ValueError(
f"Duplicate sites: site {seen[key]} and site "
f"{site.index} share the same {index_attr} "
f"{sorted(key)}. This typically means the reference "
f"structure has multiple atoms inside the same "
f"coordination environment."
)
seen[key] = site.index

def build(self) -> Trajectory:
"""Build and return the Trajectory object.

This method validates all required parameters and generates sites
using the previously configured site generator.

Returns:
Trajectory: The constructed Trajectory object
The constructed Trajectory object.

Raises:
ValueError: If required parameters are missing
ValueError: If required parameters are missing, if the
reference structure has same-species atom pairs closer
than ``min_atom_distance``, or if duplicate sites are
detected.
"""
# Validate basic requirements
if not self._structure:
raise ValueError("Structure must be set")
if not self._site_generators:
raise ValueError("Site type must be defined using one of the with_*_sites methods")

# Pre-build: validate reference structure
if self._reference_structure is not None and self._min_atom_distance > 0:
self._validate_reference_atom_distances()

# Reset the site index counter
Site.reset_index()

# Generate all sites
sites: list[Site] = []
site_type = None

for generator in self._site_generators:
generated_sites = generator()

# Verify site type consistency
if generated_sites:
current_type = type(generated_sites[0])
Expand All @@ -667,7 +786,10 @@ def build(self) -> Trajectory:
raise TypeError(f"Cannot mix site types: {site_type.__name__} and {current_type.__name__}")

sites.extend(generated_sites)


# Post-build: check for duplicate sites
self._validate_unique_sites(sites)

# Create atoms if not already set
if not self._atoms:
if not self._mobile_species:
Expand Down
Loading
Loading