Merge branch 'develop'

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

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)]

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",

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)
 

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,