Skip to content

Commit

Permalink
Fix (most) failing tests, formatting, match other test preferences (`…
Browse files Browse the repository at this point in the history
…np.isclose` over `assertAlmostEqual` etc), pull chempots from DefectThermodynamics if present and not supplied...
  • Loading branch information
kavanase committed May 22, 2024
1 parent 91850c5 commit 9d90b7a
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 45 deletions.
16 changes: 11 additions & 5 deletions doped/interface/fermi_solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -68,14 +68,22 @@ 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.
"""
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."
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down
File renamed without changes.
107 changes: 67 additions & 40 deletions tests/test_interface_fermi_solver.py
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -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
Expand All @@ -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(
Expand All @@ -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
)


Expand All @@ -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)

0 comments on commit 9d90b7a

Please sign in to comment.