From f202b7970a04817c189d0d16e58a1d45c0dad0ee Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Wed, 17 Jan 2024 19:54:00 +0100 Subject: [PATCH 1/2] deprecate Structure.ntypesp replaced by Structure.n_elems --- pymatgen/core/structure.py | 6 ++++++ pymatgen/io/abinit/abiobjects.py | 30 ++++++++++++++++-------------- pymatgen/io/cp2k/sets.py | 9 +++------ pymatgen/io/shengbte.py | 2 +- pymatgen/io/vasp/sets.py | 4 ++-- tests/core/test_structure.py | 10 +++++----- tests/io/abinit/test_inputs.py | 8 ++++---- tests/io/vasp/test_sets.py | 2 +- 8 files changed, 38 insertions(+), 33 deletions(-) diff --git a/pymatgen/core/structure.py b/pymatgen/core/structure.py index 2a65296929c..6a772913201 100644 --- a/pymatgen/core/structure.py +++ b/pymatgen/core/structure.py @@ -262,10 +262,16 @@ def species_and_occu(self) -> list[Composition]: return [site.species for site in self] @property + @deprecated(message="Use n_type_sp instead.") def ntypesp(self) -> int: """Number of types of atoms.""" return len(self.types_of_species) + @property + def n_elems(self) -> int: + """Number of types of atoms.""" + return len(self.types_of_species) + @property def types_of_species(self) -> tuple[Element | Species | DummySpecies]: """List of types of specie.""" diff --git a/pymatgen/io/abinit/abiobjects.py b/pymatgen/io/abinit/abiobjects.py index 49065600c87..4afbeb839f3 100644 --- a/pymatgen/io/abinit/abiobjects.py +++ b/pymatgen/io/abinit/abiobjects.py @@ -185,16 +185,17 @@ def species_by_znucl(structure: Structure) -> list[Species]: return types -def structure_to_abivars(structure, enforce_znucl=None, enforce_typat=None, **kwargs): +def structure_to_abivars( + structure: Structure, enforce_znucl: list | None = None, enforce_typat: list | None = None, **kwargs +): """ Receives a structure and returns a dictionary with ABINIT variables. Args: - enforce_znucl: List of ntypat entries with the value of Z for each type of atom. - Used to change the default ordering. - enforce_typat: List with natom entries with the type index. - Fortran conventions: start to count from 1. - Used to change the default ordering. + enforce_znucl (list): ntypat entries with the value of Z for each type of atom. + Used to change the default ordering. Defaults to None. + enforce_typat (list): natom entries with the type index. + Fortran conventions: start to count from 1. Used to change the default ordering. """ if not structure.is_ordered: raise ValueError( @@ -205,7 +206,6 @@ def structure_to_abivars(structure, enforce_znucl=None, enforce_typat=None, **kw ) n_atoms = len(structure) - n_types_atom = structure.ntypesp enforce_order = False if enforce_znucl is not None or enforce_typat is not None: @@ -219,19 +219,21 @@ def structure_to_abivars(structure, enforce_znucl=None, enforce_typat=None, **kw f"enforce_typat contains {len(enforce_typat)} entries while it should be {len(structure)=}" ) - if len(enforce_znucl) != n_types_atom: - raise ValueError(f"enforce_znucl contains {len(enforce_znucl)} entries while it should be {n_types_atom=}") + if len(enforce_znucl) != structure.n_elems: + raise ValueError( + f"enforce_znucl contains {len(enforce_znucl)} entries while it should be {structure.n_elems=}" + ) - if not enforce_order: + if enforce_order: + znucl_type = enforce_znucl + typat = enforce_typat or [] + else: types_of_specie = species_by_znucl(structure) znucl_type = [specie.number for specie in types_of_specie] typat = np.zeros(n_atoms, int) for atm_idx, site in enumerate(structure): typat[atm_idx] = types_of_specie.index(site.specie) + 1 - else: - znucl_type = enforce_znucl - typat = enforce_typat r_prim = ArrayWithUnit(structure.lattice.matrix, "ang").to("bohr") ang_deg = structure.lattice.angles @@ -245,7 +247,7 @@ def structure_to_abivars(structure, enforce_znucl=None, enforce_typat=None, **kw # Info on atoms. dct = { "natom": n_atoms, - "ntypat": n_types_atom, + "ntypat": structure.n_elems, "typat": typat, "znucl": znucl_type, "xred": x_red, diff --git a/pymatgen/io/cp2k/sets.py b/pymatgen/io/cp2k/sets.py index 8df8da3e98b..b0fe71bec34 100644 --- a/pymatgen/io/cp2k/sets.py +++ b/pymatgen/io/cp2k/sets.py @@ -1230,12 +1230,9 @@ def create_subsys(self, structure: Structure | Molecule) -> None: if isinstance(structure, Structure): subsys.insert(Cell(structure.lattice)) else: - x = max(structure.cart_coords[:, 0]) - y = max(structure.cart_coords[:, 1]) - z = max(structure.cart_coords[:, 2]) - x = x if x else 1 - y = y if y else 1 - z = z if z else 1 + x = max(*structure.cart_coords[:, 0], 1) + y = max(*structure.cart_coords[:, 1], 1) + z = max(*structure.cart_coords[:, 2], 1) cell = Cell(lattice=Lattice([[10 * x, 0, 0], [0, 10 * y, 0], [0, 0, 10 * z]])) cell.add(Keyword("PERIODIC", "NONE")) subsys.insert(cell) diff --git a/pymatgen/io/shengbte.py b/pymatgen/io/shengbte.py index 99fe29aaacc..c5fc381b732 100644 --- a/pymatgen/io/shengbte.py +++ b/pymatgen/io/shengbte.py @@ -216,7 +216,7 @@ def from_structure(cls, structure: Structure, reciprocal_density: int | None = 5 types = [types_dict[i] + 1 for i in structure.atomic_numbers] control_dict = { - "nelements": structure.ntypesp, + "nelements": structure.n_elems, "natoms": len(structure), "norientations": 0, "lfactor": 0.1, diff --git a/pymatgen/io/vasp/sets.py b/pymatgen/io/vasp/sets.py index 09934bcc8fc..9ba32e35920 100644 --- a/pymatgen/io/vasp/sets.py +++ b/pymatgen/io/vasp/sets.py @@ -2759,7 +2759,7 @@ def incar(self) -> Incar: defaults = { "ALGO": "Fast", "ISIF": 3, - "LANGEVIN_GAMMA": [10] * self.structure.ntypesp, + "LANGEVIN_GAMMA": [10] * self.structure.n_elems, "LANGEVIN_GAMMA_L": 1, "MDALGO": 3, "PMASS": 10, @@ -2771,7 +2771,7 @@ def incar(self) -> Incar: self.user_incar_settings = defaults # Set NPT-AIMD ENCUT = 1.5 * VASP_default - enmax = [self.potcar[i].keywords["ENMAX"] for i in range(self.structure.ntypesp)] + enmax = [self.potcar[idx].keywords["ENMAX"] for idx in range(self.structure.n_elems)] encut = max(enmax) * 1.5 self._config_dict["INCAR"]["ENCUT"] = encut diff --git a/tests/core/test_structure.py b/tests/core/test_structure.py index dbba7af0995..778a380b35f 100644 --- a/tests/core/test_structure.py +++ b/tests/core/test_structure.py @@ -71,7 +71,7 @@ def setUp(self): self.struct = IStructure(self.lattice, ["Si"] * 2, coords) assert len(self.struct) == 2, "Wrong number of sites in structure!" assert self.struct.is_ordered - assert self.struct.ntypesp == 1 + assert self.struct.n_elems == 1 coords = [[0, 0, 0], [0.0, 0, 0.0000001]] with pytest.raises( StructureError, match=f"sites are less than {self.struct.DISTANCE_TOLERANCE} Angstrom apart" @@ -967,7 +967,7 @@ def test_append_insert_remove_replace_substitute(self): struct = self.struct struct.insert(1, "O", [0.5, 0.5, 0.5]) assert struct.formula == "Si2 O1" - assert struct.ntypesp == 2 + assert struct.n_elems == 2 assert struct.symbol_set == ("O", "Si") assert struct.indices_from_symbol("Si") == (0, 2) assert struct.indices_from_symbol("O") == (1,) @@ -977,7 +977,7 @@ def test_append_insert_remove_replace_substitute(self): assert struct.indices_from_symbol("O") == (1,) struct.append("N", [0.25, 0.25, 0.25]) assert struct.formula == "Si1 N1 O1" - assert struct.ntypesp == 3 + assert struct.n_elems == 3 assert struct.symbol_set == ("N", "O", "Si") assert struct.indices_from_symbol("Si") == (0,) assert struct.indices_from_symbol("O") == (1,) @@ -987,7 +987,7 @@ def test_append_insert_remove_replace_substitute(self): assert struct.symbol_set == ("Ge", "N", "O") struct.replace_species({"Ge": "Si"}) assert struct.formula == "Si1 N1 O1" - assert struct.ntypesp == 3 + assert struct.n_elems == 3 struct.replace_species({"Si": {"Ge": 0.5, "Si": 0.5}}) assert struct.formula == "Si0.5 Ge0.5 N1 O1" @@ -995,7 +995,7 @@ def test_append_insert_remove_replace_substitute(self): struct.replace_species({"Ge": {"Ge": 0.5, "Si": 0.5}}) assert struct.formula == "Si0.75 Ge0.25 N1 O1" - assert struct.ntypesp == 4 + assert struct.n_elems == 4 struct.replace_species({"Ge": "Si"}) struct.substitute(1, "hydroxyl") diff --git a/tests/io/abinit/test_inputs.py b/tests/io/abinit/test_inputs.py index e6dc8fbc35f..9f1baa7c6d1 100644 --- a/tests/io/abinit/test_inputs.py +++ b/tests/io/abinit/test_inputs.py @@ -51,8 +51,8 @@ def test_api(self): inp = BasicAbinitInput(structure=unit_cell, pseudos=abiref_file("14si.pspnc")) - shiftk = [[0.5, 0.5, 0.5], [0.5, 0.0, 0.0], [0.0, 0.5, 0.0], [0.0, 0.0, 0.5]] - assert_array_equal(calc_shiftk(inp.structure), shiftk) + shift_k = [[0.5, 0.5, 0.5], [0.5, 0.0, 0.0], [0.0, 0.5, 0.0], [0.0, 0.0, 0.5]] + assert_array_equal(calc_shiftk(inp.structure), shift_k) assert num_valence_electrons(inp.structure, inp.pseudos) == 8 repr(inp), str(inp) @@ -81,8 +81,8 @@ def test_api(self): inp.set_vars_ifnotin(ecut=-10) assert inp["ecut"] == 5 - _, tmpname = tempfile.mkstemp(text=True) - inp.write(filepath=tmpname) + _, tmp_name = tempfile.mkstemp(text=True) + inp.write(filepath=tmp_name) # Cannot change structure variables directly. with pytest.raises(inp.Error): diff --git a/tests/io/vasp/test_sets.py b/tests/io/vasp/test_sets.py index cc17df352ce..5d921b4190b 100644 --- a/tests/io/vasp/test_sets.py +++ b/tests/io/vasp/test_sets.py @@ -1088,7 +1088,7 @@ def test_incar(self): assert incar["EDIFF"] == approx(1e-5) assert incar["LANGEVIN_GAMMA_L"] == 1 assert incar["LANGEVIN_GAMMA"] == [10, 10, 10] - enmax = max(npt_set.potcar[i].keywords["ENMAX"] for i in range(self.struct.ntypesp)) + enmax = max(npt_set.potcar[idx].keywords["ENMAX"] for idx in range(self.struct.n_elems)) assert incar["ENCUT"] == approx(1.5 * enmax) assert incar["ALGO"] == "Fast" assert incar["ISIF"] == 3 From 8b893efb0e2842187a244f785ea4ad863c690776 Mon Sep 17 00:00:00 2001 From: Janosh Riebesell Date: Wed, 17 Jan 2024 19:54:36 +0100 Subject: [PATCH 2/2] fix structure_to_abivars() for non-hexagonal structures not calling structure.lattice.is_hexagonal() --- pymatgen/core/lattice.py | 2 +- pymatgen/io/abinit/abiobjects.py | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pymatgen/core/lattice.py b/pymatgen/core/lattice.py index f272982ade5..22859ddd48e 100644 --- a/pymatgen/core/lattice.py +++ b/pymatgen/core/lattice.py @@ -1517,7 +1517,7 @@ def is_hexagonal(self, hex_angle_tol: float = 5, hex_length_tol: float = 0.01) - angles = self.angles right_angles = [i for i in range(3) if abs(angles[i] - 90) < hex_angle_tol] hex_angles = [ - i for i in range(3) if abs(angles[i] - 60) < hex_angle_tol or abs(angles[i] - 120) < hex_angle_tol + idx for idx in range(3) if abs(angles[idx] - 60) < hex_angle_tol or abs(angles[idx] - 120) < hex_angle_tol ] return ( diff --git a/pymatgen/io/abinit/abiobjects.py b/pymatgen/io/abinit/abiobjects.py index 4afbeb839f3..cc2920d6522 100644 --- a/pymatgen/io/abinit/abiobjects.py +++ b/pymatgen/io/abinit/abiobjects.py @@ -226,7 +226,7 @@ def structure_to_abivars( if enforce_order: znucl_type = enforce_znucl - typat = enforce_typat or [] + typat = enforce_typat or [] # or [] added for mypy else: types_of_specie = species_by_znucl(structure) @@ -255,11 +255,11 @@ def structure_to_abivars( } # Add info on the lattice. - # Should we use (rprim, acell) or (angdeg, acell) to specify the lattice? + # Should we use (rprim, acell) or (ang_deg, acell) to specify the lattice? geo_mode = kwargs.pop("geomode", "rprim") if geo_mode == "automatic": geo_mode = "rprim" - if structure.lattice.is_hexagonal: # or structure.lattice.is_rhombohedral + if structure.lattice.is_hexagonal(): # or structure.lattice.is_rhombohedral geo_mode = "angdeg" ang_deg = structure.lattice.angles # Here one could polish a bit the numerical values if they are not exact. @@ -283,15 +283,15 @@ def structure_to_abivars( return dct -def contract(s): +def contract(string): """ assert contract("1 1 1 2 2 3") == "3*1 2*2 1*3" assert contract("1 1 3 2 3") == "2*1 1*3 1*2 1*3" """ - if not s: - return s + if not string: + return string - tokens = s.split() + tokens = string.split() old = tokens[0] count = [[1, old]]