From 9d90b7a7bfe8628ec6f04cae5b3e768b4e28e915 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Wed, 22 May 2024 16:31:50 +0100 Subject: [PATCH] Fix (most) failing tests, formatting, match other test preferences (`np.isclose` over `assertAlmostEqual` etc), pull chempots from DefectThermodynamics if present and not supplied... --- doped/interface/fermi_solver.py | 16 ++- ...3_chempots.json => Cu2SiSe3_chempots.json} | 0 tests/test_interface_fermi_solver.py | 107 +++++++++++------- 3 files changed, 78 insertions(+), 45 deletions(-) rename examples/Cu2SiSe3/{cu2sise3_chempots.json => Cu2SiSe3_chempots.json} (100%) diff --git a/doped/interface/fermi_solver.py b/doped/interface/fermi_solver.py index 14a3e234..a4f744b3 100644 --- a/doped/interface/fermi_solver.py +++ b/doped/interface/fermi_solver.py @@ -12,7 +12,7 @@ from copy import deepcopy from itertools import product from typing import TYPE_CHECKING, Any, Optional, Union -from warnings import warn, filterwarnings +from warnings import filterwarnings, warn import numpy as np import pandas as pd @@ -68,7 +68,10 @@ class FermiSolver(MSONable): """ def __init__( - self, defect_thermodynamics: "DefectThermodynamics", bulk_dos_vr_path: str, chemical_potentials: dict + self, + defect_thermodynamics: "DefectThermodynamics", + bulk_dos_vr_path: str, + chemical_potentials: Optional[dict] = None, ): """ Initialize the FermiSolver object. @@ -76,6 +79,11 @@ def __init__( self.defect_thermodynamics = defect_thermodynamics self.bulk_dos = bulk_dos_vr_path self.chemical_potentials = chemical_potentials + + if self.chemical_potentials is None and self.defect_thermodynamics.chempots is not None: + # if chempots not supplied, but present in DefectThermodynamics, then use them + self.chemical_potentials = self.defect_thermodynamics.chempots + self._not_implemented_message = ( "This method is implemented in the derived class, " "use FermiSolverDoped or FermiSolverPyScFermi instead." @@ -778,7 +786,7 @@ def pseudo_equilibrium_solve( for column, value in new_columns.items(): concentrations[column] = value - # trimmed_concentrations = + # trimmed_concentrations = trimmed_concentrations_sub_duplicates = concentrations.drop_duplicates() excluded_columns = ["Defect"] for column in concentrations.columns.difference(excluded_columns): @@ -971,7 +979,6 @@ def equilibrium_solve( pd.DataFrame: DataFrame containing defect and carrier concentrations and the self consistent Fermi energy """ - if self.supress_warnings: filterwarnings("ignore", category=RuntimeWarning) @@ -1021,7 +1028,6 @@ def pseudo_equilibrium_solve( pd.DataFrame: DataFrame containing defect and carrier concentrations and the self consistent Fermi energy """ - if self.supress_warnings: filterwarnings("ignore", category=RuntimeWarning) diff --git a/examples/Cu2SiSe3/cu2sise3_chempots.json b/examples/Cu2SiSe3/Cu2SiSe3_chempots.json similarity index 100% rename from examples/Cu2SiSe3/cu2sise3_chempots.json rename to examples/Cu2SiSe3/Cu2SiSe3_chempots.json diff --git a/tests/test_interface_fermi_solver.py b/tests/test_interface_fermi_solver.py index 989f0d80..66be3132 100644 --- a/tests/test_interface_fermi_solver.py +++ b/tests/test_interface_fermi_solver.py @@ -1,10 +1,17 @@ -from doped.interface.fermi_solver import FermiSolver, FermiSolverDoped, FermiSolverPyScFermi -from monty.serialization import loadfn +""" +Tests for doped.interface.fermi_solver module. +""" + import unittest from unittest.mock import patch -import pandas as pd + +import numpy as np +import pytest +from monty.serialization import loadfn from pymatgen.electronic_structure.dos import FermiDos +from doped.interface.fermi_solver import FermiSolver, FermiSolverDoped, FermiSolverPyScFermi + class FermiSolverTestCase(unittest.TestCase): def setUp(self): @@ -19,41 +26,56 @@ def test_init(self): def test_equilibrium_solve(self): # test that equilibrium solve raises not implemented error - with self.assertRaises(NotImplementedError): + with pytest.raises(NotImplementedError) as exc: self.fs.equilibrium_solve() + print(exc.value) # for debugging + assert ( + "This method is implemented in the derived class, use FermiSolverDoped or " + "FermiSolverPyScFermi instead." + ) in str(exc.value) + def test_pseudoequilibrium_solve(self): # test that pseudoequilibrium solve raises not implemented error - with self.assertRaises(NotImplementedError): + with pytest.raises(NotImplementedError) as exc: self.fs.pseudo_equilibrium_solve() + print(exc.value) # for debugging + assert ( + "This method is implemented in the derived class, use FermiSolverDoped or " + "FermiSolverPyScFermi instead." + ) in str(exc.value) + def test_assert_scan_temperature_raises(self): - with self.assertRaises(ValueError): + with pytest.raises(ValueError) as exc: self.fs.scan_temperature( {}, temperature_range=[1, 2, 3], annealing_temperature_range=[1, 2, 3], quenching_temperature_range=None, ) + print(exc.value) # for debugging - with self.assertRaises(ValueError): + with pytest.raises(ValueError) as exc: self.fs.scan_temperature( {}, temperature_range=[1, 2, 3], annealing_temperature_range=None, quenching_temperature_range=[1, 2, 3], ) + print(exc.value) # for debugging - with self.assertRaises(ValueError): + with pytest.raises(ValueError) as exc: self.fs.scan_temperature( {}, temperature_range=[], annealing_temperature_range=None, quenching_temperature_range=None, ) + print(exc.value) # for debugging - def test__get_interpolated_chempots(self): + def test_get_interpolated_chempots(self): int_chempots = self.fs._get_interpolated_chempots({"a": -1, "b": -1}, {"a": 1, "b": 1}, 11) assert len(int_chempots) == 11 @@ -75,13 +97,25 @@ def setUp(self): def test__init__(self): assert self.fs.defect_thermodynamics == self.thermo assert isinstance(self.fs.bulk_dos, FermiDos) - self.assertEqual(self.fs.bulk_dos.nelecs, 18.0) + assert self.fs.bulk_dos.nelecs == 18.0 def test_assert_scan_dopant_concentration_raises(self): - with self.assertRaises(NotImplementedError): + with pytest.raises(TypeError) as exc: # missing arguments self.fs.scan_dopant_concentration() - def test__get_fermi_level_and_carriers(self): + print(exc.value) # for debugging + + # TODO: This doesn't raise a NotImplementedError because it's a method for the parent + # `FermiSolver` class. Is this not also possible with `FermiSolverDoped`? As it's just fixing + # the dopant concentrations? Thought we could do it with both backends without issue + with pytest.raises(NotImplementedError) as exc: + self.fs.scan_dopant_concentration( + chempots=None, effective_dopant_concentration_range=[1, 2, 3] + ) + + print(exc.value) # for debugging + + def test_get_fermi_level_and_carriers(self): # set return value for the thermodynamics method `get_equilibrium_fermi_level`, # does it need to be a mock? with patch( @@ -96,26 +130,24 @@ def test__get_fermi_level_and_carriers(self): def test_equilibrium_solve(self): equilibrium_results = self.fs.equilibrium_solve({"Cd": -2, "Te": -2}, 300) - self.assertAlmostEqual(equilibrium_results["Fermi Level"][0], 0.798379, places=6) - self.assertAlmostEqual(equilibrium_results["Electrons (cm^-3)"][0], 57861.403451, places=6) - self.assertAlmostEqual(equilibrium_results["Holes (cm^-3)"][0], 390268.197798, places=6) - self.assertAlmostEqual(equilibrium_results["Temperature"][0], 300, places=6) - self.assertAlmostEqual(equilibrium_results["Concentration (cm^-3)"][0], 1.423000e-24, places=6) + assert np.isclose(equilibrium_results["Fermi Level"][0], 0.798379, rtol=1e-4) + assert np.isclose(equilibrium_results["Electrons (cm^-3)"][0], 57861.403451, rtol=1e-4) + assert np.isclose(equilibrium_results["Holes (cm^-3)"][0], 390268.197798, rtol=1e-4) + assert np.isclose(equilibrium_results["Temperature"][0], 300, rtol=1e-4) + assert np.isclose(equilibrium_results["Concentration (cm^-3)"][0], 1.423000e-24, rtol=1e-4) def test_pseudo_equilibrium_solve(self): pseudo_equilibrium_results = self.fs.pseudo_equilibrium_solve( {"Cd": -2, "Te": -2}, annealing_temperature=900, quenched_temperature=300 ) - self.assertAlmostEqual(pseudo_equilibrium_results["Fermi Level"][0], 0.456269, places=6) - self.assertAlmostEqual(pseudo_equilibrium_results["Electrons (cm^-3)"][0], 0.10356, places=6) - self.assertAlmostEqual( - pseudo_equilibrium_results["Holes (cm^-3)"][0], 218051006211.37335, places=6 - ) - self.assertAlmostEqual(pseudo_equilibrium_results["Annealing Temperature"][0], 900.0, places=6) - self.assertAlmostEqual(pseudo_equilibrium_results["Quenched Temperature"][0], 300.0, places=6) - self.assertAlmostEqual( - pseudo_equilibrium_results["Concentration (cm^-3)"][0], 5.426998900437918e20, places=6 + assert np.isclose(pseudo_equilibrium_results["Fermi Level"][0], 0.456269, rtol=1e-4) + assert np.isclose(pseudo_equilibrium_results["Electrons (cm^-3)"][0], 0.10356, rtol=1e-4) + assert np.isclose(pseudo_equilibrium_results["Holes (cm^-3)"][0], 218051006212.72186, rtol=1e-4) + assert np.isclose(pseudo_equilibrium_results["Annealing Temperature"][0], 900.0, rtol=1e-4) + assert np.isclose(pseudo_equilibrium_results["Quenched Temperature"][0], 300.0, rtol=1e-4) + assert np.isclose( + pseudo_equilibrium_results["Concentration (cm^-3)"][0], 5.426998900437918e20, rtol=1e-4 ) @@ -132,23 +164,18 @@ def test__init__(self): def test_equilibrium_solve(self): equilibrium_results = self.fs.equilibrium_solve({"Cd": -2, "Te": -2}, 300) - self.assertAlmostEqual(equilibrium_results["Fermi Level"][0], 0.8878258982206311, places=6) - self.assertAlmostEqual(equilibrium_results["Electrons (cm^-3)"][0], 4490462.40649167, places=6) - self.assertAlmostEqual(equilibrium_results["Holes (cm^-3)"][0], 12219.64626151177, places=6) - self.assertAlmostEqual(equilibrium_results["Temperature"][0], 300, places=6) + assert np.isclose(equilibrium_results["Fermi Level"][0], 0.8878258982206311, rtol=1e-4) + assert np.isclose(equilibrium_results["Electrons (cm^-3)"][0], 4490462.40649167, rtol=1e-4) + assert np.isclose(equilibrium_results["Holes (cm^-3)"][0], 12219.64626151177, rtol=1e-4) + assert np.isclose(equilibrium_results["Temperature"][0], 300, rtol=1e-4) def test_pseudo_equilibrium_solve(self): pseudo_equilibrium_results = self.fs.pseudo_equilibrium_solve( {"Cd": -2, "Te": -2}, annealing_temperature=900, quenched_temperature=300 ) - self.assertAlmostEqual(pseudo_equilibrium_results["Fermi Level"][0], 0.8900579775197812, places=6) - self.assertAlmostEqual( - pseudo_equilibrium_results["Electrons (cm^-3)"][0], 4895401.840081683, places=6 - ) - self.assertAlmostEqual(pseudo_equilibrium_results["Holes (cm^-3)"][0], 11208.85760769246, places=6) - self.assertAlmostEqual(pseudo_equilibrium_results["Annealing Temperature"][0], 900.0, places=6) - self.assertAlmostEqual(pseudo_equilibrium_results["Quenched Temperature"][0], 300.0, places=6) - -if __name__ == "__main__": - unittest.main() + assert np.isclose(pseudo_equilibrium_results["Fermi Level"][0], 0.8900579775197812, rtol=1e-4) + assert np.isclose(pseudo_equilibrium_results["Electrons (cm^-3)"][0], 4895401.840081683, rtol=1e-4) + assert np.isclose(pseudo_equilibrium_results["Holes (cm^-3)"][0], 11208.85760769246, rtol=1e-4) + assert np.isclose(pseudo_equilibrium_results["Annealing Temperature"][0], 900.0, rtol=1e-4) + assert np.isclose(pseudo_equilibrium_results["Quenched Temperature"][0], 300.0, rtol=1e-4)