diff --git a/docs/Dev_ToDo.md b/docs/Dev_ToDo.md index 9bddf961..2d1c49c4 100644 --- a/docs/Dev_ToDo.md +++ b/docs/Dev_ToDo.md @@ -42,7 +42,7 @@ with the original structure and get the difference (-> configurational degeneracy) from this. Not sure if we can do this in general? Taking the unrelaxed and relaxed defect structures, and getting the difference in symm-ops according to `spglib`? - - Also add consideration of odd/even number of electrons to account for spin degeneracy. + - Also add consideration of odd/even number of electrons to account for spin degeneracy. Note we can do this automatically even without `NELECT`, using magnetisation from OUTCAR/vasprun or atomic numbers (even/odd) plus charge (can also use this to double check odd is odd and even even as expected, warn user if not) - Complex defect / defect cluster automatic handling. Means we can natively handle complex defects, and also important for e.g. `ShakeNBreak` parsing, as in many cases we're ending up with what are effectively defect clusters rather than point defects (e.g. V_Sb^+1 actually Se_Sb^-1 + V_Se^+2 in @@ -168,6 +168,10 @@ automatically in `doped`, excited-state degeneracy (e.g. with bipolarons/dimers with single and triplet states) are not, so the user should manually account for this if present. Also note that temperature effects can be important in certain cases so see this review if that's the case. + - Should have recommendation somewhere about open science practices. The doped defect dict and dpd jsons should always be shared in e.g. Zenodo when publishing, as contains all info on the parsed defect data in a lean format. Also using the `formation_energy_table` etc. functions for SI tables is recommended. + - Add our general rule-of-thumbs/expectations regarding charge corrections: + - Potential alignment terms should rarely ever be massive + - In general, the correction terms should follow somewhat consistent trends (for a given charge state, across defects), so if you see a large outlier in the corrections, it's implying something odd is happening there. This is can be fairly easily scanned with `formation_energy_table`. - The Wyckoff analysis code is very useful and no other package can do this afaik. See https://github.com/spglib/spglib/issues/135. Should describe and exemplify this in the docs (i.e. the `get_wyckoff_label_and_equiv_coord_list()` from just a `pymatgen` site and spacegroup 🔥) and JOSS diff --git a/doped/corrections.py b/doped/corrections.py index 226e0617..93b16e5a 100644 --- a/doped/corrections.py +++ b/doped/corrections.py @@ -42,6 +42,7 @@ from monty.json import MontyDecoder from pymatgen.analysis.defects.corrections import freysoldt from pymatgen.analysis.defects.utils import CorrectionResult +from pymatgen.core.periodic_table import Element from pymatgen.io.vasp.outputs import Locpot, Outcar from shakenbreak.plotting import _install_custom_font @@ -472,6 +473,11 @@ def get_kumagai_correction( ) for label in labels ] + dummy_h = Element("H") # dummy element to check if valid symbol + labels = [ + label + r" ($V_{defect} - V_{bulk}$)" if dummy_h.is_valid_symbol(label) else label + for label in labels + ] # add entry for dashed red line: handles += [Line2D([0], [0], **spp._mpl_defaults.hline)] diff --git a/examples/dope_parsing_example.ipynb b/examples/dope_parsing_example.ipynb index cb799ef4..69011ab6 100644 --- a/examples/dope_parsing_example.ipynb +++ b/examples/dope_parsing_example.ipynb @@ -1172,9 +1172,9 @@ "cell_type": "markdown", "metadata": {}, "source": [ - "Here we can see we obtain a good plateau in the atomic potential differences ($\\Delta V$) between the \n", + "Here we can see we obtain a good plateau in the atomic potential differences (ΔV) between the \n", "defect and bulk supercells in the 'sampling region' (i.e. region of defect supercell furthest from the \n", - "defect site), the average of which is used to obtain our potential alignment ('Avg. $\\Delta V$') and \n", + "defect site), the average of which is used to obtain our potential alignment ('Avg. ΔV') and \n", "thus our final charge correction term.\n", "\n", "If there is still significant variance in the site potential differences in the sampling region (i.e. a \n", diff --git a/tests/data/remote_baseline_plots/F_O_+1_eFNV_plot.png b/tests/data/remote_baseline_plots/F_O_+1_eFNV_plot.png index 23ce4e1f..29eb5cff 100644 Binary files a/tests/data/remote_baseline_plots/F_O_+1_eFNV_plot.png and b/tests/data/remote_baseline_plots/F_O_+1_eFNV_plot.png differ diff --git a/tests/data/remote_baseline_plots/F_O_+1_eFNV_plot_custom.png b/tests/data/remote_baseline_plots/F_O_+1_eFNV_plot_custom.png index 156b77de..d2af8bfb 100644 Binary files a/tests/data/remote_baseline_plots/F_O_+1_eFNV_plot_custom.png and b/tests/data/remote_baseline_plots/F_O_+1_eFNV_plot_custom.png differ diff --git a/tests/data/remote_baseline_plots/Te_i_+2_eFNV_plot.png b/tests/data/remote_baseline_plots/Te_i_+2_eFNV_plot.png index c0b9b38e..b32539aa 100644 Binary files a/tests/data/remote_baseline_plots/Te_i_+2_eFNV_plot.png and b/tests/data/remote_baseline_plots/Te_i_+2_eFNV_plot.png differ diff --git a/tests/test_generation.py b/tests/test_generation.py index b1fbe3aa..61d653b3 100644 --- a/tests/test_generation.py +++ b/tests/test_generation.py @@ -59,6 +59,12 @@ def _potcars_available() -> bool: class DefectsGeneratorTest(unittest.TestCase): def setUp(self): + if not _potcars_available() and random.randint(1, 10) != 1: + # on GH Actions, only run the heavy tests 10% of times: + self.heavy_tests = False + else: + self.heavy_tests = True + self.data_dir = os.path.join(os.path.dirname(__file__), "data") self.cdte_data_dir = os.path.join(self.data_dir, "CdTe") self.example_dir = os.path.join(os.path.dirname(__file__), "..", "examples") @@ -1998,6 +2004,9 @@ def test_ytos_supercell_input(self): ) # for testing in test_vasp.py def test_ytos_no_generate_supercell(self): + if not self.heavy_tests: # skip one of the YTOS tests if on GH Actions + return + # tests the case of an input structure which is >10 Å in each direction, has # more atoms (198) than the pmg supercell (99), but generate_supercell = False, # so the _input_ supercell is used @@ -2119,6 +2128,9 @@ def lmno_defect_gen_check(self, lmno_defect_gen, generate_supercell=True): ) def test_lmno(self): + if not self.heavy_tests: # skip one of the LMNO tests if on GH Actions + return + # battery material with a variety of important Wyckoff sites (and the terminology mainly # used in this field). Tough to find suitable supercell, goes to 448-atom supercell. lmno_defect_gen, output = self._generate_and_test_no_warnings(self.lmno_primitive) @@ -2593,6 +2605,9 @@ def cd_i_cdte_supercell_defect_gen_check(self, cd_i_defect_gen): ) def test_supercell_w_defect_cd_i_cdte(self): + if not self.heavy_tests: + return + # test inputting a defective supercell cdte_defect_gen = DefectsGenerator(self.prim_cdte) diff --git a/tests/test_vasp.py b/tests/test_vasp.py index 13be5dd5..999d7723 100644 --- a/tests/test_vasp.py +++ b/tests/test_vasp.py @@ -88,6 +88,12 @@ def _check_nupdown_neutral_cell_warning(message): class DefectDictSetTest(unittest.TestCase): def setUp(self): + if not _potcars_available() and random.randint(1, 10) != 1: + # on GH Actions, only run the heavy tests 10% of times: + self.heavy_tests = False + else: + self.heavy_tests = True + self.data_dir = os.path.join(os.path.dirname(__file__), "data") self.cdte_data_dir = os.path.join(self.data_dir, "CdTe") self.example_dir = os.path.join(os.path.dirname(__file__), "..", "examples") @@ -640,6 +646,15 @@ def test_initialisation_and_writing(self): Test the initialisation of DefectRelaxSet for a range of `DefectEntry`s. """ + if not self.heavy_tests: + return + + def _check_drs_defect_entry_attribute_transfer(parent_drs, input_defect_entry): + assert parent_drs.defect_entry == input_defect_entry + assert parent_drs.defect_supercell == input_defect_entry.defect_supercell + assert parent_drs.charge_state == input_defect_entry.charge_state + assert parent_drs.bulk_supercell == input_defect_entry.bulk_supercell + # test initialising DefectRelaxSet with our generation-tests materials, and writing files to disk defect_gen_test_list = [ (self.cdte_defect_gen, "CdTe defect_gen"), @@ -661,32 +676,25 @@ def test_initialisation_and_writing(self): for defect_gen, defect_gen_name in defect_gen_test_list: print(f"Initialising and testing: {defect_gen_name}") - # randomly choose 3 defect entries from the defect_gen dict: - defect_entries = random.sample(list(defect_gen.values()), 3) + # randomly choose a defect entry from the defect_gen dict: + defect_entry = random.choice(list(defect_gen.values())) - for defect_entry in defect_entries: - print(f"Randomly testing {defect_entry.name}") - drs = DefectRelaxSet(defect_entry) - self._general_defect_relax_set_check(drs) - - def _check_drs_defect_entry_attribute_transfer(parent_drs, input_defect_entry): - assert parent_drs.defect_entry == input_defect_entry - assert parent_drs.defect_supercell == input_defect_entry.defect_supercell - assert parent_drs.charge_state == input_defect_entry.charge_state - assert parent_drs.bulk_supercell == input_defect_entry.bulk_supercell - - _check_drs_defect_entry_attribute_transfer(drs, defect_entry) - - custom_drs = DefectRelaxSet( - defect_entry, - user_incar_settings={"ENCUT": 350}, - user_potcar_functional="PBE_52", - user_potcar_settings={"Cu": "Cu_pv"}, - user_kpoints_settings={"reciprocal_density": 200}, - poscar_comment="Test pop", - ) - self._general_defect_relax_set_check(custom_drs) - _check_drs_defect_entry_attribute_transfer(custom_drs, defect_entry) + print(f"Randomly testing {defect_entry.name}") + drs = DefectRelaxSet(defect_entry) + self._general_defect_relax_set_check(drs) + + _check_drs_defect_entry_attribute_transfer(drs, defect_entry) + + custom_drs = DefectRelaxSet( + defect_entry, + user_incar_settings={"ENCUT": 350}, + user_potcar_functional="PBE_52", + user_potcar_settings={"Cu": "Cu_pv"}, + user_kpoints_settings={"reciprocal_density": 200}, + poscar_comment="Test pop", + ) + self._general_defect_relax_set_check(custom_drs) + _check_drs_defect_entry_attribute_transfer(custom_drs, defect_entry) # TODO: Test file writing, default folder naming # TODO: Explicitly check some poscar comments? For DS, DRS and DDS @@ -711,7 +719,7 @@ def test_default_kpoints_soc_handling(self): for defect_gen, defect_gen_name in defect_gen_test_list: print(f"Testing:{defect_gen_name}") # randomly choose 10 defect entries from the defect_gen dict: - defect_entries = random.sample(list(defect_gen.values()), 10) + defect_entries = random.sample(list(defect_gen.values()), min(10, len(defect_gen))) for defect_entry in defect_entries: print(f"Randomly testing {defect_entry.name}") @@ -767,7 +775,7 @@ def test_default_kpoints_soc_handling(self): # Test manually turning _on_ SOC and making vasp_gam _not_ converged: defect_gen = DefectsGenerator.from_json(f"{self.data_dir}/lmno_defect_gen.json") - defect_entries = random.sample(list(defect_gen.values()), 5) + defect_entries = random.sample(list(defect_gen.values()), min(5, len(defect_gen))) for defect_entry in defect_entries: print(f"Randomly testing {defect_entry.name}") @@ -936,6 +944,9 @@ def _general_defects_set_check(self, defects_set, **kwargs): ) def test_cdte_files(self): + if not self.heavy_tests: + return + cdte_se_defect_gen = DefectsGenerator(self.prim_cdte, extrinsic="Se") defects_set = DefectsSet( cdte_se_defect_gen,