From 6992aeef9847b03da81c80dabe27c84010e28abc Mon Sep 17 00:00:00 2001 From: "Haoyu (Daniel) YANG" Date: Sat, 26 Oct 2024 05:25:47 +0800 Subject: [PATCH] Prevent unit test of `boltztrap2` from modifying files in place (#4095) * rename setup method * use temp dir for TestBztTransportProperties * revert to setup method * revert previous changes, but split tests by sp/non-sp * use temp dir * Revert "use temp dir" This reverts commit f403974199b250908903c23ff05058ad16658eaf. * Revert "revert previous changes, but split tests by sp/non-sp" This reverts commit 950ac9708aa6044352d38c9064c6196a4c4fb208. * properly capture bt2 import exception * properly setup * try to revert to HEAD * also revert testcase * typo, that's the reason * add back TestCase inherit * extract tests from setup method * use temp dir * use a global skip mark * use temp dir for TestBztInterpolator as well * NEED CONFIRM: replace BoltztrapError with more semantic exception * pre-commit auto-fixes --------- Signed-off-by: Shyue Ping Ong Co-authored-by: Shyue Ping Ong Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> --- .../electronic_structure/boltztrap2.py | 27 ++-- tests/electronic_structure/test_boltztrap2.py | 140 ++++++++++-------- 2 files changed, 93 insertions(+), 74 deletions(-) diff --git a/src/pymatgen/electronic_structure/boltztrap2.py b/src/pymatgen/electronic_structure/boltztrap2.py index bc51103a62d..de576cec76d 100644 --- a/src/pymatgen/electronic_structure/boltztrap2.py +++ b/src/pymatgen/electronic_structure/boltztrap2.py @@ -35,7 +35,6 @@ from tqdm import tqdm from pymatgen.electronic_structure.bandstructure import BandStructure, BandStructureSymmLine, Spin -from pymatgen.electronic_structure.boltztrap import BoltztrapError from pymatgen.electronic_structure.dos import CompleteDos, Dos, Orbital from pymatgen.electronic_structure.plotter import BSPlotter, DosPlotter from pymatgen.io.ase import AseAtomsAdaptor @@ -52,7 +51,7 @@ from BoltzTraP2 import bandlib as BL from BoltzTraP2 import fite, sphere, units except ImportError as exc: - raise BoltztrapError("BoltzTraP2 has to be installed and working") from exc + raise ImportError("BoltzTraP2 has to be installed and working") from exc __author__ = "Francesco Ricci" @@ -85,7 +84,7 @@ def __init__(self, obj, structure=None, nelect=None) -> None: elif isinstance(obj, BandStructure): bs_obj = obj else: - raise BoltztrapError("The object provided is neither a Bandstructure nor a Vasprun.") + raise TypeError("The object provided is neither a Bandstructure nor a Vasprun.") self.kpoints = np.array([kp.frac_coords for kp in bs_obj.kpoints]) @@ -94,7 +93,7 @@ def __init__(self, obj, structure=None, nelect=None) -> None: elif structure: self.structure = structure else: - raise BoltztrapError("A structure must be given.") + raise ValueError("A structure must be given.") self.atoms = AseAtomsAdaptor.get_atoms(self.structure) self.proj_all = None @@ -131,7 +130,7 @@ def __init__(self, obj, structure=None, nelect=None) -> None: elif self.vbm_idx: self.nelect_all = self.vbm_idx + self.cbm_idx + 1 else: - raise BoltztrapError("nelect must be given.") + raise ValueError("nelect must be given.") @classmethod def from_file(cls, vasprun_file: str | Path) -> Self: @@ -322,7 +321,7 @@ def __init__(self, vrun_obj=None) -> None: self.proj = next(iter(vrun_obj.projected_eigenvalues.values())) elif len(vrun_obj.eigenvalues) == 2: - raise BoltztrapError("spin bs case not implemented") + raise NotImplementedError("spin bs case not implemented") self.lattvec = self.atoms.get_cell().T * units.Angstrom @@ -466,7 +465,7 @@ def load(self, fname="bztInterp.json.gz"): self.equivalences, coeffs = loadfn(fname) bands_loaded = False else: - raise BoltztrapError("Something wrong reading the data file!") + raise RuntimeError("Something wrong reading the data file!") self.coeffs = coeffs[0] + coeffs[1] * 1j return bands_loaded @@ -588,7 +587,7 @@ def get_partial_doses(self, tdos, eband_ud, spins, enr, npts_mu, T, progress): progress: Default False, If True a progress bar is shown. """ if not self.data.proj: - raise BoltztrapError("No projections loaded.") + raise ValueError("No projections loaded.") bkp_data_ebands = np.copy(self.data.ebands) @@ -1053,13 +1052,13 @@ def plot_props( props_short = tuple(p[: len(prop_y)] for p in props) if prop_y not in props_short: - raise BoltztrapError("prop_y not valid") + raise ValueError("prop_y not valid") if prop_x not in {"mu", "doping", "temp"}: - raise BoltztrapError("prop_x not valid") + raise ValueError("prop_x not valid") if prop_z not in {"doping", "temp"}: - raise BoltztrapError("prop_z not valid") + raise ValueError("prop_z not valid") idx_prop = props_short.index(prop_y) @@ -1095,7 +1094,7 @@ def plot_props( plt.xlabel(r"$\mu$ (eV)", fontsize=30) plt.xlim(xlim) else: - raise BoltztrapError( + raise ValueError( "only prop_x=mu and prop_z=temp are \ available for c.c. and Hall c.c.!" ) @@ -1172,7 +1171,7 @@ def plot_props( def plot_bands(self): """Plot a band structure on symmetry line using BSPlotter().""" if self.bzt_interp is None: - raise BoltztrapError("BztInterpolator not present") + raise ValueError("BztInterpolator not present") sbs = self.bzt_interp.get_band_structure() @@ -1181,7 +1180,7 @@ def plot_bands(self): def plot_dos(self, T=None, npoints=10000): """Plot the total Dos using DosPlotter().""" if self.bzt_interp is None: - raise BoltztrapError("BztInterpolator not present") + raise ValueError("BztInterpolator not present") tdos = self.bzt_interp.get_dos(T=T, npts_mu=npoints) dosPlotter = DosPlotter() diff --git a/tests/electronic_structure/test_boltztrap2.py b/tests/electronic_structure/test_boltztrap2.py index 7e93cfb3b9b..cdb75a24532 100644 --- a/tests/electronic_structure/test_boltztrap2.py +++ b/tests/electronic_structure/test_boltztrap2.py @@ -1,13 +1,14 @@ from __future__ import annotations +import shutil from unittest import TestCase import numpy as np import pytest from monty.serialization import loadfn +from monty.tempfile import ScratchDir from pytest import approx -from pymatgen.electronic_structure.boltztrap import BoltztrapError from pymatgen.electronic_structure.core import OrbitalType, Spin from pymatgen.io.vasp import Vasprun from pymatgen.util.testing import TEST_FILES_DIR @@ -22,9 +23,8 @@ VasprunLoader, ) -except BoltztrapError: - pytest.skip("No boltztrap2.", allow_module_level=True) - +except ImportError: + pytest.skip("No boltztrap2", allow_module_level=True) TEST_DIR = f"{TEST_FILES_DIR}/electronic_structure/boltztrap2" @@ -33,6 +33,7 @@ VASP_RUN_FILE_SPIN = f"{TEST_DIR}/vasprun_spin.xml" VASP_RUN_SPIN = Vasprun(VASP_RUN_FILE_SPIN, parse_projected_eigen=True) + BAND_STRUCT = loadfn(f"{TEST_DIR}/PbTe_bandstructure.json") BAND_STRUCT_SPIN = loadfn(f"{TEST_DIR}/N2_bandstructure.json") @@ -127,22 +128,24 @@ def test_from_file(self): class TestBztInterpolator(TestCase): def setUp(self): - self.loader = VasprunBSLoader(VASP_RUN) - self.bztInterp = BztInterpolator(self.loader, lpfac=2) - assert self.bztInterp is not None - # TODO: following creates file locally, use temp dir - self.bztInterp = BztInterpolator(self.loader, lpfac=2, save_bztInterp=True, fname=BZT_INTERP_FN) - assert self.bztInterp is not None - self.bztInterp = BztInterpolator(self.loader, load_bztInterp=True, fname=BZT_INTERP_FN) - assert self.bztInterp is not None - - self.loader_sp = VasprunBSLoader(VASP_RUN_SPIN) - self.bztInterp_sp = BztInterpolator(self.loader_sp, lpfac=2) - assert self.bztInterp_sp is not None - self.bztInterp_sp = BztInterpolator(self.loader_sp, lpfac=2, save_bztInterp=True, fname=BZT_INTERP_FN) - assert self.bztInterp_sp is not None - self.bztInterp_sp = BztInterpolator(self.loader_sp, lpfac=2, load_bztInterp=True, fname=BZT_INTERP_FN) - assert self.bztInterp_sp is not None + with ScratchDir("."): + shutil.copy(BZT_INTERP_FN, ".") + + loader = VasprunBSLoader(VASP_RUN) + self.bztInterp = BztInterpolator(loader, lpfac=2) + assert self.bztInterp is not None + self.bztInterp = BztInterpolator(loader, lpfac=2, save_bztInterp=True) + assert self.bztInterp is not None + self.bztInterp = BztInterpolator(loader, load_bztInterp=True) + assert self.bztInterp is not None + + loader_sp = VasprunBSLoader(VASP_RUN_SPIN) + self.bztInterp_sp = BztInterpolator(loader_sp, lpfac=2) + assert self.bztInterp_sp is not None + self.bztInterp_sp = BztInterpolator(loader_sp, lpfac=2, save_bztInterp=True) + assert self.bztInterp_sp is not None + self.bztInterp_sp = BztInterpolator(loader_sp, lpfac=2, load_bztInterp=True) + assert self.bztInterp_sp is not None def test_properties(self): assert self.bztInterp.cband.shape == (6, 3, 3, 3, 29791) @@ -204,105 +207,122 @@ def test_tot_proj_dos(self): class TestBztTransportProperties(TestCase): def setUp(self): + with ScratchDir("."): + shutil.copy(BZT_TRANSP_FN, ".") + + # non spin polarized + loader = VasprunBSLoader(VASP_RUN) + bztInterp = BztInterpolator(loader, lpfac=2) + + self.bztTransp = BztTransportProperties( + bztInterp, + temp_r=np.arange(300, 600, 100), + save_bztTranspProps=True, + ) + assert self.bztTransp is not None + + bztInterp = BztInterpolator(loader, lpfac=2) + self.bztTransp = BztTransportProperties( + bztInterp, + load_bztTranspProps=True, + ) + assert self.bztTransp is not None + + # spin polarized + loader_sp = VasprunBSLoader(VASP_RUN_SPIN) + bztInterp_sp = BztInterpolator(loader_sp, lpfac=2) + + self.bztTransp_sp = BztTransportProperties( + bztInterp_sp, + temp_r=np.arange(300, 600, 100), + save_bztTranspProps=True, + ) + assert self.bztTransp_sp is not None + + bztInterp_sp = BztInterpolator(loader_sp, lpfac=2) + self.bztTransp_sp = BztTransportProperties( + bztInterp_sp, + load_bztTranspProps=True, + ) + assert self.bztTransp_sp is not None + + def test_init(self): + # non spin polarized loader = VasprunBSLoader(VASP_RUN) bztInterp = BztInterpolator(loader, lpfac=2) self.bztTransp = BztTransportProperties(bztInterp, temp_r=np.arange(300, 600, 100)) assert self.bztTransp is not None - self.bztTransp = BztTransportProperties( - bztInterp, doping=10.0 ** np.arange(20, 22), temp_r=np.arange(300, 600, 100) - ) - assert self.bztTransp is not None - assert self.bztTransp.contain_props_doping - - bztInterp = BztInterpolator(loader, lpfac=2) self.bztTransp = BztTransportProperties( bztInterp, + doping=10.0 ** np.arange(20, 22), temp_r=np.arange(300, 600, 100), - save_bztTranspProps=True, - fname=BZT_TRANSP_FN, ) assert self.bztTransp is not None + assert self.bztTransp.contain_props_doping - bztInterp = BztInterpolator(loader, lpfac=2) - self.bztTransp = BztTransportProperties(bztInterp, load_bztTranspProps=True, fname=BZT_TRANSP_FN) - assert self.bztTransp is not None - + # spin polarized loader_sp = VasprunBSLoader(VASP_RUN_SPIN) bztInterp_sp = BztInterpolator(loader_sp, lpfac=2) self.bztTransp_sp = BztTransportProperties(bztInterp_sp, temp_r=np.arange(300, 600, 100)) assert self.bztTransp_sp is not None - bztInterp_sp = BztInterpolator(loader_sp, lpfac=2) - # TODO: following creates file locally, use temp dir - self.bztTransp_sp = BztTransportProperties( - bztInterp_sp, - temp_r=np.arange(300, 600, 100), - save_bztTranspProps=True, - fname=BZT_TRANSP_FN, - ) - assert self.bztTransp_sp is not None - - bztInterp_sp = BztInterpolator(loader_sp, lpfac=2) - self.bztTransp_sp = BztTransportProperties(bztInterp_sp, load_bztTranspProps=True, fname=BZT_TRANSP_FN) - assert self.bztTransp_sp is not None - def test_properties(self): - for p in [ + for p in ( self.bztTransp.Conductivity_mu, self.bztTransp.Seebeck_mu, self.bztTransp.Kappa_mu, self.bztTransp.Effective_mass_mu, self.bztTransp.Power_Factor_mu, - ]: + ): assert p.shape == (3, 3686, 3, 3) - for p in [ + for p in ( self.bztTransp.Carrier_conc_mu, self.bztTransp.Hall_carrier_conc_trace_mu, - ]: + ): assert p.shape == (3, 3686) - for p in [ + for p in ( self.bztTransp_sp.Conductivity_mu, self.bztTransp_sp.Seebeck_mu, self.bztTransp_sp.Kappa_mu, self.bztTransp_sp.Effective_mass_mu, self.bztTransp_sp.Power_Factor_mu, - ]: + ): assert p.shape == (3, 3252, 3, 3) - for p in [ + for p in ( self.bztTransp_sp.Carrier_conc_mu, self.bztTransp_sp.Hall_carrier_conc_trace_mu, - ]: + ): assert p.shape == (3, 3252) def test_compute_properties_doping(self): self.bztTransp.compute_properties_doping(doping=10.0 ** np.arange(20, 22)) - for p in [ + for p in ( self.bztTransp.Conductivity_doping, self.bztTransp.Seebeck_doping, self.bztTransp.Kappa_doping, self.bztTransp.Effective_mass_doping, self.bztTransp.Power_Factor_doping, - ]: + ): assert p["n"].shape == (3, 2, 3, 3) assert self.bztTransp.contain_props_doping self.bztTransp_sp.compute_properties_doping(doping=10.0 ** np.arange(20, 22)) - for p in [ + for p in ( self.bztTransp_sp.Conductivity_doping, self.bztTransp_sp.Seebeck_doping, self.bztTransp_sp.Kappa_doping, self.bztTransp_sp.Effective_mass_doping, self.bztTransp_sp.Power_Factor_doping, - ]: + ): assert p["n"].shape == (3, 2, 3, 3) assert self.bztTransp_sp.contain_props_doping -class TestBztPlotter(TestCase): +class TestBztPlotter: def test_plot(self): loader = VasprunBSLoader(VASP_RUN) bztInterp = BztInterpolator(loader, lpfac=2)