From def9dae771622bf3d18d6c628ec96f33a8614566 Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Mon, 26 Dec 2022 15:01:40 -0800 Subject: [PATCH] Fix `coord_list_mapping_pbc()` (#2782) * fix AttributeError: module 'numpy' has no attribute 'int' * add missing arg descriptions in doc strings --- pymatgen/analysis/structure_matcher.py | 59 ++++++++++++++------------ pymatgen/io/lobster/lobsterenv.py | 51 +++++++++++++--------- pymatgen/util/coord_cython.pyx | 2 +- 3 files changed, 64 insertions(+), 48 deletions(-) diff --git a/pymatgen/analysis/structure_matcher.py b/pymatgen/analysis/structure_matcher.py index 340cdae5340..c4561a91f0c 100644 --- a/pymatgen/analysis/structure_matcher.py +++ b/pymatgen/analysis/structure_matcher.py @@ -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 @@ -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 """ @@ -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 """ @@ -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: @@ -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") @@ -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, @@ -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) """ @@ -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: diff --git a/pymatgen/io/lobster/lobsterenv.py b/pymatgen/io/lobster/lobsterenv.py index 7d303bf488a..8d8aa069927 100644 --- a/pymatgen/io/lobster/lobsterenv.py +++ b/pymatgen/io/lobster/lobsterenv.py @@ -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 @@ -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: @@ -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 @@ -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==[] @@ -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==[] @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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 @@ -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" @@ -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) :] @@ -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: @@ -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) @@ -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 diff --git a/pymatgen/util/coord_cython.pyx b/pymatgen/util/coord_cython.pyx index 917aec58fd7..66595fe3381 100644 --- a/pymatgen/util/coord_cython.pyx +++ b/pymatgen/util/coord_cython.pyx @@ -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)