Skip to content

Commit

Permalink
Fix coord_list_mapping_pbc() (#2782)
Browse files Browse the repository at this point in the history
* fix AttributeError: module 'numpy' has no attribute 'int'

* add missing arg descriptions in doc strings
  • Loading branch information
janosh authored Dec 26, 2022
1 parent d947703 commit def9dae
Show file tree
Hide file tree
Showing 3 changed files with 64 additions and 48 deletions.
59 changes: 32 additions & 27 deletions pymatgen/analysis/structure_matcher.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import abc
import itertools
from typing import Literal, Sequence
from typing import Literal, Mapping, Sequence

import numpy as np
from monty.json import MSONable
Expand Down Expand Up @@ -441,7 +441,7 @@ def _get_lattices(self, target_lattice, s, supercell_size=1):
cells in it
Args:
target_lattice (Lattice):
target_lattice (Lattice): target lattice.
s (Structure): input structure.
supercell_size (int): Number of primitive cells in returned lattice
"""
Expand Down Expand Up @@ -521,7 +521,7 @@ def _cart_dists(cls, s1, s2, avg_lattice, mask, normalization, lll_frac_tol=None
normalization (float): inverse normalization length
Returns:
Distances from s2 to s1, normalized by (V/Natom) ^ 1/3
Distances from s2 to s1, normalized by (V/atom) ^ 1/3
Fractional translation vector to apply to s2.
Mapping from s1 to s2, i.e. with numpy slicing, s1[mapping] => s2
"""
Expand Down Expand Up @@ -588,20 +588,19 @@ def fit(
Args:
struct1 (Structure): 1st structure
struct2 (Structure): 2nd structure
symmetric (Bool): Defaults to False
symmetric (bool): Defaults to False
If True, check the equality both ways.
This only impacts a small percentage of structures
skip_structure_reduction (Bool): Defaults to False
skip_structure_reduction (bool): Defaults to False
If True, skip to get a primitive structure and perform Niggli reduction for struct1 and struct2
Returns:
True or False.
bool: True if the structures are equivalent
"""
struct1, struct2 = self._process_species([struct1, struct2])

if not self._subset and self._comparator.get_hash(struct1.composition) != self._comparator.get_hash(
struct2.composition
):
hash_match = self._comparator.get_hash(struct1.composition) != self._comparator.get_hash(struct2.composition)
if not self._subset and not hash_match:
return False

if not symmetric:
Expand Down Expand Up @@ -727,25 +726,24 @@ def _match(

def _strict_match(
self,
struct1,
struct2,
fu,
s1_supercell=True,
use_rms=False,
break_on_match=False,
):
struct1: Structure,
struct2: Structure,
fu: int,
s1_supercell: bool = True,
use_rms: bool = False,
break_on_match: bool = False,
) -> tuple[float, float, np.ndarray, float, Mapping] | None:
"""
Matches struct2 onto struct1 (which should contain all sites in
struct2).
Args:
struct1, struct2 (Structure): structures to be matched
struct1 (Structure): structure to match onto
struct2 (Structure): structure to match
fu (int): size of supercell to create
s1_supercell (bool): whether to create the supercell of
struct1 (vs struct2)
s1_supercell (bool): whether to create the supercell of struct1 (vs struct2)
use_rms (bool): whether to minimize the rms of the matching
break_on_match (bool): whether to stop search at first
valid match
break_on_match (bool): whether to stop search at first match
"""
if fu < 1:
raise ValueError("fu cannot be less than 1")
Expand Down Expand Up @@ -895,9 +893,9 @@ def from_dict(cls, d):

def _anonymous_match(
self,
struct1,
struct2,
fu,
struct1: Structure,
struct2: Structure,
fu: int,
s1_supercell=True,
use_rms=False,
break_on_match=False,
Expand All @@ -906,7 +904,14 @@ def _anonymous_match(
"""
Tries all permutations of matching struct1 to struct2.
Args:
struct1, struct2 (Structure): Preprocessed input structures
struct1 (Structure): First structure
struct2 (Structure): Second structure
fu (int): Factor of unit cell of struct1 to match to struct2
s1_supercell (bool): whether to create the supercell of struct1 (vs struct2)
use_rms (bool): Whether to minimize the rms of the matching
break_on_match (bool): Whether to break search on first match
single_match (bool): Whether to return only the best match
Returns:
List of (mapping, match)
"""
Expand Down Expand Up @@ -1062,8 +1067,8 @@ def fit_anonymous(
Args:
struct1 (Structure): 1st structure
struct2 (Structure): 2nd structure
niggli (Bool): If true, perform Niggli reduction for struct1 and struct2
skip_structure_reduction (Bool): Defaults to False
niggli (bool): If true, perform Niggli reduction for struct1 and struct2
skip_structure_reduction (bool): Defaults to False
If True, skip to get a primitive structure and perform Niggli reduction for struct1 and struct2
Returns:
Expand Down
51 changes: 31 additions & 20 deletions pymatgen/io/lobster/lobsterenv.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,9 @@ def __init__(
id_blist_sg2="ICOBI",
):
"""
Args:
are_coops: (Bool) if True, the file is a ICOOPLIST.lobster and not a ICOHPLIST.lobster; only tested for
are_coops: (bool) if True, the file is a ICOOPLIST.lobster and not a ICOHPLIST.lobster; only tested for
ICOHPLIST.lobster so far
filename_ICOHP: (str) Path to ICOHPLIST.lobster
valences: (list of integers/floats) gives valence/charge for each element
Expand Down Expand Up @@ -215,7 +216,7 @@ def molecules_allowed(self):

def get_anion_types(self):
"""
will return the types of anions present in crystal structure
Return the types of anions present in crystal structure
Returns:
"""
if self.valences is None:
Expand Down Expand Up @@ -251,8 +252,9 @@ def get_nn_info(self, structure: Structure, n, use_weights=False):

def get_light_structure_environment(self, only_cation_environments=False, only_indices=None):
"""
will return a LobsterLightStructureEnvironments object
Return a LobsterLightStructureEnvironments object
if the structure only contains coordination environments smaller 13
Args:
only_cation_environments: only data for cations will be returned
only_indices: will only evaluate the list of isites in this list
Expand Down Expand Up @@ -359,7 +361,8 @@ def get_light_structure_environment(self, only_cation_environments=False, only_i

def get_info_icohps_to_neighbors(self, isites=None, onlycation_isites=True):
"""
this method will return information of cohps of neighbors
this method Return information of cohps of neighbors
Args:
isites: list of site ids, if isite==None, all isites will be used to add the icohps of the neighbors
onlycation_isites: will only use cations, if isite==[]
Expand Down Expand Up @@ -415,7 +418,8 @@ def plot_cohps_of_neighbors(
integrated=False,
):
"""
will plot summed cohps (please be careful in the spin polarized case (plots might overlap (exactly!))
Will plot summed cohps (please be careful in the spin polarized case (plots might overlap (exactly!))
Args:
isites: list of site ids, if isite==[], all isites will be used to add the icohps of the neighbors
onlycation_isites: bool, will only use cations, if isite==[]
Expand Down Expand Up @@ -464,7 +468,8 @@ def get_info_cohps_to_neighbors(
summed_spin_channels=False,
):
"""
will return info about the cohps from all sites mentioned in isites with neighbors
Return info about the cohps from all sites mentioned in isites with neighbors
Args:
path_to_COHPCAR: str, path to COHPCAR
isites: list of int that indicate the number of the site
Expand Down Expand Up @@ -588,7 +593,8 @@ def _get_plot_label(self, atoms, per_bond):

def get_info_icohps_between_neighbors(self, isites=None, onlycation_isites=True):
"""
will return infos about interactions between neighbors of a certain atom
Return infos about interactions between neighbors of a certain atom
Args:
isites: list of site ids, if isite==None, all isites will be used
onlycation_isites: will only use cations, if isite==None
Expand Down Expand Up @@ -699,6 +705,7 @@ def _evaluate_ce(
adapt_extremum_to_add_cond=False,
):
"""
Args:
lowerlimit: lower limit which determines the ICOHPs that are considered for the determination of the
neighbors
Expand Down Expand Up @@ -824,7 +831,8 @@ def _evaluate_ce(

def _find_environments(self, additional_condition, lowerlimit, upperlimit, only_bonds_to):
"""
will find all relevant neighbors based on certain restrictions
Will find all relevant neighbors based on certain restrictions
Args:
additional_condition (int): additional condition (see above)
lowerlimit (float): lower limit that tells you which ICOHPs are considered
Expand Down Expand Up @@ -936,7 +944,8 @@ def _find_environments(self, additional_condition, lowerlimit, upperlimit, only_

def _find_relevant_atoms_additional_condition(self, isite, icohps, additional_condition):
"""
will find all relevant atoms that fulfill the additional_conditions
Will find all relevant atoms that fulfill the additional_conditions
Args:
isite: number of site in structure (starts with 0)
icohps: icohps
Expand Down Expand Up @@ -1073,7 +1082,8 @@ def _find_relevant_atoms_additional_condition(self, isite, icohps, additional_co
@staticmethod
def _get_icohps(icohpcollection, isite, lowerlimit, upperlimit, only_bonds_to):
"""
will return icohp dict for certain site
Return icohp dict for certain site
Args:
icohpcollection: Icohpcollection object
isite (int): number of a site
Expand All @@ -1095,7 +1105,8 @@ def _get_icohps(icohpcollection, isite, lowerlimit, upperlimit, only_bonds_to):
@staticmethod
def _get_atomnumber(atomstring):
"""
will return the number of the atom within the initial POSCAR (e.g., will return 0 for "Na1")
Return the number of the atom within the initial POSCAR (e.g., Return 0 for "Na1")
Args:
atomstring: string such as "Na1"
Expand All @@ -1106,11 +1117,10 @@ def _get_atomnumber(atomstring):
@staticmethod
def _split_string(s):
"""
will split strings such as "Na1" in "Na" and "1" and return "1"
Will split strings such as "Na1" in "Na" and "1" and return "1"
Args:
s (str): string
Returns:
"""
head = s.rstrip("0123456789")
tail = s[len(head) :]
Expand All @@ -1119,11 +1129,10 @@ def _split_string(s):
@staticmethod
def _determine_unit_cell(site):
"""
based on the site it will determine the unit cell, in which this site is based
Based on the site it will determine the unit cell, in which this site is based
Args:
site: site object
Returns:
"""
unitcell = []
for coord in site.frac_coords:
Expand All @@ -1142,8 +1151,9 @@ def _get_limit_from_extremum(
# TODO: adapt this to give the extremum for the correct type of bond
# TODO add tests
"""
will return limits for the evaluation of the icohp values from an icohpcollection
will return -100000, min(max_icohp*0.15,-0.1)
Return limits for the evaluation of the icohp values from an icohpcollection
Return -100000, min(max_icohp*0.15,-0.1)
Args:
icohpcollection: icohpcollection object
percentage: will determine which ICOHPs will be considered (only 0.15 from the maximum value)
Expand Down Expand Up @@ -1248,7 +1258,8 @@ def from_Lobster(
valences=None,
):
"""
will set up a LightStructureEnvironments from Lobster
Will set up a LightStructureEnvironments from Lobster
Args:
structure: Structure object
list_ce_symbol: list of symbols for coordination environments
Expand Down
2 changes: 1 addition & 1 deletion pymatgen/util/coord_cython.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ def coord_list_mapping_pbc(subset, superset, atol=1e-8, pbc=(True, True, True)):
Returns:
list of indices such that superset[indices] = subset
"""
inds = np.zeros(len(subset), dtype=np.int) - 1
inds = np.zeros(len(subset)) - 1
subset = np.atleast_2d(subset)
superset = np.atleast_2d(superset)

Expand Down

0 comments on commit def9dae

Please sign in to comment.