bump monty to use the monty.json import speedup patch, add import…

… time regression test, lazy load some rarely used but costly modules (#4128) * copy pyproject from 4073 * bump monty * recover networkx pin, and bump sympy * pin monty to lower ver to see what is causing the failure * revert all changes to pyproject but monty * bump sympy * sort and group optional deps * loose networkx pin for compatibility * lazy import sympy * lazy load scipy * Revert "lazy load scipy" This reverts commit 9688244. * try netcdf4 install with delvewheel, 1dfc9e4 * Revert "try netcdf4 install with delvewheel, 1dfc9e4" This reverts commit 75e23b1. * test netcdf4 1.7.1.post2, notice new release is out today * netcdf4 1.7.1.post2 doesn't work, try latest 1.7.2 * reset netcdf4 pin * why <= doesn't work? * add comment * exclude 1.7.2 as well (perhaps conditionally skip that test?) * add place holder * enhance comment * add TODO tag * tweak placeholder * replace assert_allclose for scalar compare * replace assert_allclose with isclose * fix is close * use np.allclose over np.all(np.isclose()) * lazy import AseAtomsAdaptor * guard type check import * lazy import matplotlib * toggle reference generation * update import time records * skip in not CI env * include current time in err msg * loose hard threshold to 100%, as it appear macos runner is prone to fluctuation * update type * skip test for macos * add PR tag for easy tracing * use perf_counter_ns for precision * bump torch timeout all the way to 5 min * Revert "bump torch timeout all the way to 5 min" This reverts commit 9c3d186. * migrate missing allclose relative tolerance --------- Co-authored-by: Shyue Ping Ong <[email protected]>
materialsproject · Nov 13, 2024 · 179cdeb · 179cdeb
1 parent 65f52ea
commit 179cdeb
Show file tree

Hide file tree

Showing 14 changed files with 249 additions and 73 deletions.
diff --git a/pyproject.toml b/pyproject.toml
@@ -56,8 +56,8 @@ classifiers = [
 dependencies = [
     "joblib>=1",
     "matplotlib>=3.8",
-    "monty>=2024.7.29",
-    "networkx>=3",  # PR4116
+    "monty>=2024.10.21",
+    "networkx>=2.7",  # PR4116
     "palettable>=3.3.3",
     "pandas>=2",
     "plotly>=4.5.0",
@@ -69,7 +69,7 @@ dependencies = [
     # https://github.com/scipy/scipy/issues/21052
     "scipy>=1.14.1; platform_system == 'Windows'",
     "spglib>=2.5.0",
-    "sympy>=1.2",
+    "sympy>=1.3",  # PR #4116
     "tabulate>=0.9",
     "tqdm>=4.60",
     "uncertainties>=3.1.4",
@@ -87,40 +87,34 @@ Issues = "https://github.com/materialsproject/pymatgen/issues"
 Pypi = "https://pypi.org/project/pymatgen"
 
 [project.optional-dependencies]
+# PR4128: netcdf4 1.7.[0/1] yanked, 1.7.1.post[1/2]/1.7.2 cause CI error
+abinit = ["netcdf4>=1.6.5,!=1.7.1.post1,!=1.7.1.post2,!=1.7.2"]
 ase = ["ase>=3.23.0"]
-# tblite only support Python 3.12+ through conda-forge
-# https://github.com/tblite/tblite/issues/175
-tblite = ["tblite[ase]>=0.3.0; python_version<'3.12'"]
-vis = ["vtk>=6.0.0"]
-abinit = ["netcdf4>=1.7.1"]
-mlp = ["chgnet>=0.3.8", "matgl>=1.1.3"]
-electronic_structure = ["fdint>=2.0.2"]
 ci = ["pytest-cov>=4", "pytest-split>=0.8", "pytest>=8"]
 docs = ["invoke", "sphinx", "sphinx_markdown_builder", "sphinx_rtd_theme"]
+electronic_structure = ["fdint>=2.0.2"]
+mlp = ["chgnet>=0.3.8", "matgl>=1.1.3"]
+numba = ["numba>=0.55"]
+numpy-v1 = ["numpy>=1.25.0,<2"] # Test NP1 on Windows (quite buggy ATM)
 optional = [
-    "ase>=3.23.0",
+    "pymatgen[abinit,ase,mlp,tblite]",
     "beautifulsoup4",
     # BoltzTraP2 build fails on Windows GitHub runners
     "BoltzTraP2>=24.9.4 ; platform_system != 'Windows'",
     "chemview>=0.6",
-    "chgnet>=0.3.8",
     "f90nml>=1.1.2",
     "galore>=0.6.1",
     "h5py>=3.11.0",
+    "hiphive>=1.3.1",
     "jarvis-tools>=2020.7.14",
-    "matgl>=1.1.3",
     "matplotlib>=3.8",
-    "netCDF4>=1.6.5",
     "phonopy>=2.23",
     "seekpath>=2.0.1",
-    # tblite only support Python 3.12+ through conda-forge
-    # https://github.com/tblite/tblite/issues/175
-    "hiphive>=1.3.1",
-    "openbabel-wheel>=3.1.1.20",
-    "tblite[ase]>=0.3.0; platform_system=='Linux' and python_version<'3.12'",
 ]
-numba = ["numba>=0.55"]
-numpy-v1 = ["numpy>=1.25.0,<2"] # Test NP1 on Windows (quite buggy ATM)
+# tblite only support Python 3.12+ through conda-forge
+# https://github.com/tblite/tblite/issues/175
+tblite = [ "tblite[ase]>=0.3.0; platform_system=='Linux' and python_version<'3.12'"]
+vis = ["vtk>=6.0.0"]
 
 [project.scripts]
 pmg = "pymatgen.cli.pmg:main"

diff --git a/src/pymatgen/analysis/adsorption.py b/src/pymatgen/analysis/adsorption.py
@@ -9,8 +9,6 @@
 from typing import TYPE_CHECKING
 
 import numpy as np
-from matplotlib import patches
-from matplotlib.path import Path
 from monty.serialization import loadfn
 from scipy.spatial import Delaunay
 
@@ -664,6 +662,10 @@ def plot_slab(
         decay (float): how the alpha-value decays along the z-axis
         inverse (bool): invert z axis to plot opposite surface
     """
+    # Expensive import (PR4128)
+    from matplotlib import patches
+    from matplotlib.path import Path
+
     orig_slab = slab.copy()
     slab = reorient_z(slab)
     orig_cell = slab.lattice.matrix.copy()

diff --git a/src/pymatgen/analysis/interfaces/coherent_interfaces.py b/src/pymatgen/analysis/interfaces/coherent_interfaces.py
@@ -6,7 +6,6 @@
 from typing import TYPE_CHECKING
 
 import numpy as np
-from numpy.testing import assert_allclose
 from scipy.linalg import polar
 
 from pymatgen.analysis.elasticity.strain import Deformation
@@ -101,21 +100,15 @@ def _find_matches(self) -> None:
         for match in self.zsl_matches:
             xform = get_2d_transform(film_vectors, match.film_vectors)
             strain, _rot = polar(xform)
-            assert_allclose(
-                strain,
-                np.round(strain),
-                atol=1e-12,
-                err_msg="Film lattice vectors changed during ZSL match, check your ZSL Generator parameters",
-            )
+            if not np.allclose(strain, np.round(strain), rtol=1e-7, atol=1e-12):
+                raise ValueError("Film lattice vectors changed during ZSL match, check your ZSL Generator parameters")
 
             xform = get_2d_transform(substrate_vectors, match.substrate_vectors)
             strain, _rot = polar(xform)
-            assert_allclose(
-                strain,
-                strain.astype(int),
-                atol=1e-12,
-                err_msg="Substrate lattice vectors changed during ZSL match, check your ZSL Generator parameters",
-            )
+            if not np.allclose(strain, strain.astype(int), rtol=1e-7, atol=1e-12):
+                raise ValueError(
+                    "Substrate lattice vectors changed during ZSL match, check your ZSL Generator parameters"
+                )
 
     def _find_terminations(self):
         """Find all terminations."""
@@ -226,37 +219,23 @@ def get_interfaces(
             ).astype(int)
             film_sl_slab = film_slab.copy()
             film_sl_slab.make_supercell(super_film_transform)
-            assert_allclose(
-                film_sl_slab.lattice.matrix[2],
-                film_slab.lattice.matrix[2],
-                atol=1e-08,
-                err_msg="2D transformation affected C-axis for Film transformation",
-            )
-            assert_allclose(
-                film_sl_slab.lattice.matrix[:2],
-                match.film_sl_vectors,
-                atol=1e-08,
-                err_msg="Transformation didn't make proper supercell for film",
-            )
+            if not np.allclose(film_sl_slab.lattice.matrix[2], film_slab.lattice.matrix[2], rtol=1e-7, atol=1e-08):
+                raise ValueError(
+                    "2D transformation affected C-axis for Film transformation",
+                )
+            if not np.allclose(film_sl_slab.lattice.matrix[:2], match.film_sl_vectors, rtol=1e-7, atol=1e-08):
+                raise ValueError("Transformation didn't make proper supercell for film")
 
             # Build substrate superlattice
             super_sub_transform = np.round(
                 from_2d_to_3d(get_2d_transform(sub_slab.lattice.matrix[:2], match.substrate_sl_vectors))
             ).astype(int)
             sub_sl_slab = sub_slab.copy()
             sub_sl_slab.make_supercell(super_sub_transform)
-            assert_allclose(
-                sub_sl_slab.lattice.matrix[2],
-                sub_slab.lattice.matrix[2],
-                atol=1e-08,
-                err_msg="2D transformation affected C-axis for Film transformation",
-            )
-            assert_allclose(
-                sub_sl_slab.lattice.matrix[:2],
-                match.substrate_sl_vectors,
-                atol=1e-08,
-                err_msg="Transformation didn't make proper supercell for substrate",
-            )
+            if not np.allclose(sub_sl_slab.lattice.matrix[2], sub_slab.lattice.matrix[2], rtol=1e-7, atol=1e-08):
+                raise ValueError("2D transformation affected C-axis for Film transformation")
+            if not np.allclose(sub_sl_slab.lattice.matrix[:2], match.substrate_sl_vectors, rtol=1e-7, atol=1e-08):
+                raise ValueError("Transformation didn't make proper supercell for substrate")
 
             # Add extra info
             match_dict = match.as_dict()

diff --git a/src/pymatgen/core/__init__.py b/src/pymatgen/core/__init__.py
@@ -5,7 +5,7 @@
 import os
 import warnings
 from importlib.metadata import PackageNotFoundError, version
-from typing import Any
+from typing import TYPE_CHECKING
 
 from ruamel.yaml import YAML
 
@@ -17,10 +17,14 @@
 from pymatgen.core.structure import IMolecule, IStructure, Molecule, PeriodicNeighbor, SiteCollection, Structure
 from pymatgen.core.units import ArrayWithUnit, FloatWithUnit, Unit
 
+if TYPE_CHECKING:
+    from typing import Any
+
 __author__ = "Pymatgen Development Team"
 __email__ = "[email protected]"
 __maintainer__ = "Shyue Ping Ong, Matthew Horton, Janosh Riebesell"
 __maintainer_email__ = "[email protected]"
+
 try:
     __version__ = version("pymatgen")
 except PackageNotFoundError:  # pragma: no cover

diff --git a/src/pymatgen/core/interface.py b/src/pymatgen/core/interface.py
@@ -14,7 +14,6 @@
 
 import numpy as np
 from monty.fractions import lcm
-from numpy.testing import assert_allclose
 from scipy.cluster.hierarchy import fcluster, linkage
 from scipy.spatial.distance import squareform
 
@@ -2788,10 +2787,16 @@ def from_slabs(
             substrate_slab = substrate_slab.get_orthogonal_c_slab()
         if isinstance(film_slab, Slab):
             film_slab = film_slab.get_orthogonal_c_slab()
-        assert_allclose(film_slab.lattice.alpha, 90, 0.1)
-        assert_allclose(film_slab.lattice.beta, 90, 0.1)
-        assert_allclose(substrate_slab.lattice.alpha, 90, 0.1)
-        assert_allclose(substrate_slab.lattice.beta, 90, 0.1)
+
+        if not math.isclose(film_slab.lattice.alpha, 90, rel_tol=0.1) or not math.isclose(
+            film_slab.lattice.beta, 90, rel_tol=0.1
+        ):
+            raise ValueError("The lattice angles alpha or beta of the film slab are not approximately 90 degrees.")
+
+        if not math.isclose(substrate_slab.lattice.alpha, 90, rel_tol=0.1) or not math.isclose(
+            substrate_slab.lattice.beta, 90, rel_tol=0.1
+        ):
+            raise ValueError("The lattice angles alpha or beta of the substrate slab are not approximately 90 degrees.")
 
         # Ensure sub is right-handed
         # IE sub has surface facing "up"

diff --git a/src/pymatgen/core/operations.py b/src/pymatgen/core/operations.py
@@ -449,7 +449,7 @@ def as_xyz_str(self) -> str:
         Only works for integer rotation matrices.
         """
         # Check for invalid rotation matrix
-        if not np.all(np.isclose(self.rotation_matrix, np.round(self.rotation_matrix))):
+        if not np.allclose(self.rotation_matrix, np.round(self.rotation_matrix)):
             warnings.warn("Rotation matrix should be integer")
 
         return transformation_to_string(

diff --git a/src/pymatgen/core/trajectory.py b/src/pymatgen/core/trajectory.py
@@ -15,7 +15,6 @@
 from monty.json import MSONable
 
 from pymatgen.core.structure import Composition, DummySpecies, Element, Lattice, Molecule, Species, Structure
-from pymatgen.io.ase import AseAtomsAdaptor
 
 if TYPE_CHECKING:
     from collections.abc import Iterator
@@ -580,9 +579,12 @@ def from_file(cls, filename: str | Path, constant_lattice: bool = True, **kwargs
             try:
                 from ase.io.trajectory import Trajectory as AseTrajectory
 
+                from pymatgen.io.ase import AseAtomsAdaptor
+
                 ase_traj = AseTrajectory(filename)
                 # Periodic boundary conditions should be the same for all frames so just check the first
                 pbc = ase_traj[0].pbc
+
                 if any(pbc):
                     structures = [AseAtomsAdaptor.get_structure(atoms) for atoms in ase_traj]
                 else:

diff --git a/src/pymatgen/io/vasp/outputs.py b/src/pymatgen/io/vasp/outputs.py
@@ -22,7 +22,6 @@
 from monty.json import MSONable, jsanitize
 from monty.os.path import zpath
 from monty.re import regrep
-from numpy.testing import assert_allclose
 from tqdm import tqdm
 
 from pymatgen.core import Composition, Element, Lattice, Structure
@@ -4973,8 +4972,8 @@ def __init__(
 
                     if i_spin == 0:
                         self.kpoints.append(kpoint)
-                    else:
-                        assert_allclose(self.kpoints[i_nk], kpoint)
+                    elif not np.allclose(self.kpoints[i_nk], kpoint, rtol=1e-7, atol=0):
+                        raise ValueError(f"kpoints of {i_nk=} mismatch")
 
                     if verbose:
                         print(f"kpoint {i_nk: 4} with {nplane: 5} plane waves at {kpoint}")

diff --git a/src/pymatgen/symmetry/settings.py b/src/pymatgen/symmetry/settings.py
@@ -7,8 +7,6 @@
 from typing import TYPE_CHECKING
 
 import numpy as np
-from sympy import Matrix
-from sympy.parsing.sympy_parser import parse_expr
 
 from pymatgen.core.lattice import Lattice
 from pymatgen.core.operations import MagSymmOp, SymmOp
@@ -100,6 +98,10 @@ def parse_transformation_string(
         Returns:
             tuple[list[list[float]] | np.ndarray, list[float]]: transformation matrix & vector
         """
+        # Import sympy is expensive (PR4128)
+        from sympy import Matrix
+        from sympy.parsing.sympy_parser import parse_expr
+
         try:
             a, b, c = np.eye(3)
             b_change, o_shift = transformation_string.split(";")

diff --git a/tests/files/performance/import_time_linux.json b/tests/files/performance/import_time_linux.json
@@ -0,0 +1,19 @@
+{
+    "from pymatgen.core.bonds import CovalentBond": 289.5851116666108,
+    "from pymatgen.core.composition import Composition": 292.8479909999548,
+    "from pymatgen.core.interface import Interface": 969.5693099999593,
+    "from pymatgen.core.ion import Ion": 291.07530133334575,
+    "from pymatgen.core.lattice import Lattice": 288.8340153333881,
+    "from pymatgen.core.libxcfunc import LibxcFunc": 293.4184753333587,
+    "from pymatgen.core.molecular_orbitals import MolecularOrbitals": 294.19796566658835,
+    "from pymatgen.core.operations import SymmOp": 296.4627546666634,
+    "from pymatgen.core.periodic_table import Element": 295.95872066662804,
+    "from pymatgen.core.sites import Site": 292.66485499999817,
+    "from pymatgen.core.spectrum import Spectrum": 486.72776566669046,
+    "from pymatgen.core.structure import Structure": 291.01618733333606,
+    "from pymatgen.core.surface import Slab": 301.90875833329756,
+    "from pymatgen.core.tensors import Tensor": 304.27744800003137,
+    "from pymatgen.core.trajectory import Trajectory": 300.45536066666045,
+    "from pymatgen.core.units import Unit": 305.4779056666348,
+    "from pymatgen.core.xcfunc import XcFunc": 309.1085626666275
+}
diff --git a/tests/files/performance/import_time_macos.json b/tests/files/performance/import_time_macos.json
@@ -0,0 +1,19 @@
+{
+    "from pymatgen.core.bonds import CovalentBond": 321.8502360000457,
+    "from pymatgen.core.composition import Composition": 292.35445800009074,
+    "from pymatgen.core.interface import Interface": 855.005861000033,
+    "from pymatgen.core.ion import Ion": 240.8930970000256,
+    "from pymatgen.core.lattice import Lattice": 329.09868066659936,
+    "from pymatgen.core.libxcfunc import LibxcFunc": 306.6966386666839,
+    "from pymatgen.core.molecular_orbitals import MolecularOrbitals": 281.78087466661356,
+    "from pymatgen.core.operations import SymmOp": 299.9741943333447,
+    "from pymatgen.core.periodic_table import Element": 293.6565829999533,
+    "from pymatgen.core.sites import Site": 280.3443330000543,
+    "from pymatgen.core.spectrum import Spectrum": 459.20266666666976,
+    "from pymatgen.core.structure import Structure": 265.4675833332476,
+    "from pymatgen.core.surface import Slab": 306.0919996667053,
+    "from pymatgen.core.tensors import Tensor": 310.54281933325,
+    "from pymatgen.core.trajectory import Trajectory": 335.25658333329983,
+    "from pymatgen.core.units import Unit": 294.03472200003006,
+    "from pymatgen.core.xcfunc import XcFunc": 309.3993196666058
+}
diff --git a/tests/files/performance/import_time_windows.json b/tests/files/performance/import_time_windows.json
@@ -0,0 +1,19 @@
+{
+    "from pymatgen.core.bonds import CovalentBond": 443.7560000000455,
+    "from pymatgen.core.composition import Composition": 441.06553333335796,
+    "from pymatgen.core.interface import Interface": 1828.751033333295,
+    "from pymatgen.core.ion import Ion": 443.58053333333675,
+    "from pymatgen.core.lattice import Lattice": 445.729999999988,
+    "from pymatgen.core.libxcfunc import LibxcFunc": 459.24773333338936,
+    "from pymatgen.core.molecular_orbitals import MolecularOrbitals": 440.4825999999957,
+    "from pymatgen.core.operations import SymmOp": 440.62226666665083,
+    "from pymatgen.core.periodic_table import Element": 441.64050000002436,
+    "from pymatgen.core.sites import Site": 442.1802333333744,
+    "from pymatgen.core.spectrum import Spectrum": 737.3025000000174,
+    "from pymatgen.core.structure import Structure": 445.0546333332568,
+    "from pymatgen.core.surface import Slab": 463.0683333333157,
+    "from pymatgen.core.tensors import Tensor": 463.5761666666743,
+    "from pymatgen.core.trajectory import Trajectory": 443.9995333333779,
+    "from pymatgen.core.units import Unit": 446.352766666602,
+    "from pymatgen.core.xcfunc import XcFunc": 469.42599999996065
+}
diff --git a/tests/io/abinit/test_netcdf.py b/tests/io/abinit/test_netcdf.py
@@ -97,6 +97,8 @@ def test_read_fe(self):
                 ref_magmom_collinear = [-0.5069359730980665]
                 path = os.path.join(tmp_dir, "Fe_magmoms_collinear_GSR.nc")
 
+                # TODO: PR4128, EtsfReader would fail in Ubuntu CI with netCDF4 > 1.6.5
+                # Need someone with knowledge in netCDF4 to fix it
                 with EtsfReader(path) as data:
                     structure = data.read_structure()
                     assert structure.site_properties["magmom"] == ref_magmom_collinear