Merge pull request #74 from SMTG-Bham/develop

Develop

CHANGELOG.rst

@@ -1,6 +1,13 @@
 Change Log
 ==========
 
+v3.3.4
+----------
+- Make oxidation state guessing more efficient.
+- Update Quantum Espresso and FHI-aims IO functions to work with new (and old) ASE release.
+- Minor updates to ensure compatibility with recent ``pymatgen`` release.
+- Allow unrecognised defect names when plotting.
+
 v3.3.3
 ----------
 - Add ``verbose`` option to more parsing/plotting functions for better control of output detail.

docs/Tips.rst

@@ -207,6 +207,21 @@ see `docs here <https://shakenbreak.readthedocs.io/en/latest/shakenbreak.energy_
 Troubleshooting
 -------------------
 
+For most error cases, ``ShakeNBreak`` has been designed to try and give informative error messages about
+why the functions are failing.
+In the majority of cases, if you encounter an error using ``ShakeNBreak`` which does not have a clear error
+message about the origin of the problem, it is likely to be an issue with your version of ``pymatgen``
+(and/or ``ShakeNBreak``/``doped``), and may be fixed by doing:
+
+.. code:: bash
+
+  pip install pymatgen pymatgen-analysis-defects monty --upgrade
+  pip install ShakeNBreak doped --upgrade
+
+If this does not solve your issue, please check the specific cases noted below. If your issue still isn't
+solved, then please contact the developers through the ``GitHub``
+`Issues <https://github.com/SMTG-Bham/ShakeNBreak/issues>`_ page, or by email.
+
 - A current known issue with ``numpy``/``pymatgen`` is that it might give an error similar to this:
 
   .. code:: python

docs/conf.py

@@ -25,7 +25,7 @@
 author = 'Irea Mosquera-Lois, Seán R. Kavanagh'
 
 # The full version, including alpha/beta/rc tags
-release = '3.3.3'
+release = '3.3.4'
 
 
 # -- General configuration ---------------------------------------------------

setup.py

@@ -131,7 +131,7 @@ def package_files(directory):
 
 setup(
     name="shakenbreak",
-    version="3.3.3",
+    version="3.3.4",
     description="Package to generate and analyse distorted defect structures, in order to "
     "identify ground-state and metastable defect configurations.",
     long_description=long_description,

shakenbreak/cli.py

@@ -9,7 +9,7 @@
 from subprocess import call
 
 import click
-from doped.core import _guess_and_set_oxi_states_with_timeout, _rough_oxi_state_cost_from_comp
+from doped.core import guess_and_set_oxi_states_with_timeout
 from doped.generation import get_defect_name_from_entry
 from doped.utils.parsing import get_outcar
 from doped.utils.plotting import format_defect_name
@@ -240,14 +240,14 @@ def generate(
             if key not in valid_args:
                 user_settings.pop(key)
 
-    defect_struc = Structure.from_file(defect)
-    bulk_struc = Structure.from_file(bulk)
+    defect_struct = Structure.from_file(defect)
+    bulk_struct = Structure.from_file(bulk)
 
     # Note that here the Defect.defect_structure is the defect `supercell`
     # structure, not the defect `primitive` structure.
     defect_object = input.identify_defect(
-        defect_structure=defect_struc,
-        bulk_structure=bulk_struc,
+        defect_structure=defect_struct,
+        bulk_structure=bulk_struct,
         defect_index=defect_index,
         defect_coords=defect_coords,
     )
@@ -464,22 +464,14 @@ def generate_all(
     Generate the trial distortions and input files for structure-searching
     for all defects in a given directory.
     """
-    bulk_struc = Structure.from_file(bulk)
+    bulk_struct = Structure.from_file(bulk)
     # try parsing the bulk oxidation states first, for later assigning defect "oxi_state"s (i.e.
     # fully ionised charge states):
-    # First check if the cost of guessing oxidation states is too high:
-    if _rough_oxi_state_cost_from_comp(bulk_struc.composition) > 1e6:
-        # If the cost is too high, avoid setting oxidation states as it will take too long
-        _bulk_oxi_states = False  # will take very long to guess oxi_state
-    else:
-        # Otherwise, proceed with setting oxidation states using a separate process to allow timeouts
-        from multiprocessing import Queue  # only import when necessary
-
-        queue = Queue()
-        _bulk_oxi_states = _guess_and_set_oxi_states_with_timeout(bulk_struc, queue=queue)
-        if _bulk_oxi_states:  # Retrieve the oxidation states if successfully guessed and set
-            bulk_struc = queue.get()  # oxi-state decorated structure
-            _bulk_oxi_states = {el.symbol: el.oxi_state for el in bulk_struc.composition.elements}
+    if bulk_struct_w_oxi := guess_and_set_oxi_states_with_timeout(
+        bulk_struct, break_early_if_expensive=True
+    ):
+        bulk_struct = bulk_struct_w_oxi
+        _bulk_oxi_states = {el.symbol: el.oxi_state for el in bulk_struct.composition.elements}
 
     defects_dirs = os.listdir(defects)
     if config is not None:
@@ -595,7 +587,7 @@ def parse_defect_position(defect_name, defect_settings):
     for defect in defects_dirs:  # file or directory
         if os.path.isfile(f"{defects}/{defect}"):
             try:  # try to parse structure from it
-                defect_struc = Structure.from_file(f"{defects}/{defect}")
+                defect_struct = Structure.from_file(f"{defects}/{defect}")
                 defect_name = parse_defect_name(defect, defect_settings)  # None if not recognised
 
             except Exception:
@@ -624,7 +616,7 @@ def parse_defect_position(defect_name, defect_settings):
                     )
                     continue
             if defect_file:
-                defect_struc = Structure.from_file(os.path.join(defects, defect, defect_file))
+                defect_struct = Structure.from_file(os.path.join(defects, defect, defect_file))
                 defect_name = parse_defect_name(defect, defect_settings)
         else:
             warnings.warn(f"Could not parse {defects}/{defect} as a defect, skipping.")
@@ -633,8 +625,8 @@ def parse_defect_position(defect_name, defect_settings):
         # Check if indices are provided in config file
         defect_index, defect_coords = parse_defect_position(defect_name, defect_settings)
         defect_object = input.identify_defect(
-            defect_structure=defect_struc,
-            bulk_structure=bulk_struc,
+            defect_structure=defect_struct,
+            bulk_structure=bulk_struct,
             defect_index=defect_index,
             defect_coords=defect_coords,
             oxi_state=(

shakenbreak/distortions.py

@@ -10,7 +10,10 @@
 from hiphive.structure_generation.rattle import _probability_mc_rattle, generate_mc_rattled_structures
 from pymatgen.analysis.local_env import CrystalNN, MinimumDistanceNN
 from pymatgen.core.structure import Structure
-from pymatgen.io.ase import AseAtomsAdaptor
+from pymatgen.io.ase import AseAtomsAdaptor  # could be removed to use the Structure.to/from_ase_atoms()
+
+# methods added in pymatgen 2024.5.31, but then not backwards compatible. Will refactor to this if/when
+# pymatgen>=2024.5.31 is a necessary requirement.
 
 
 def _warning_on_one_line(message, category, filename, lineno, file=None, line=None):

shakenbreak/energy_lowering_distortions.py

@@ -832,7 +832,7 @@ def write_retest_inputs(
         code (:obj:`str`):
             Code used for the geometry relaxations. The supported codes
             include "vasp", "cp2k", "espresso", "castep" and "fhi-aims"
-            (case insensitive).
+            (case-insensitive).
             (Default: "vasp")
         input_filename (:obj:`str`):
             Name of the code input file if different from `ShakeNBreak`
@@ -1000,6 +1000,9 @@ def _copy_espresso_files(
             filename=f"{distorted_dir}/{input_filename}",
             images=atoms,
             format="espresso-in",
+            pseudopotentials={  # doesn't matter, is rewritten below to use same as before
+                atom: "N/A" for atom in set(atoms.get_chemical_symbols())
+            },
         )
         with open(f"{distorted_dir}/{input_filename}") as f:
             new_struct = f.read()
@@ -1027,6 +1030,9 @@ def _copy_espresso_files(
                 filename=f"{distorted_dir}/{input_filename}",
                 images=atoms,
                 format="espresso-in",
+                pseudopotentials={  # doesn't matter, is rewritten below to use same as before
+                    atom: "N/A" for atom in set(atoms.get_chemical_symbols())
+                },
             )
             with open(f"{distorted_dir}/{input_filename}") as f:
                 new_struct = f.read()
@@ -1048,6 +1054,9 @@ def _copy_espresso_files(
                 filename=f"{distorted_dir}/{input_filename}",
                 images=atoms,
                 format="espresso-in",
+                pseudopotentials={
+                    atom: "Pseudopotentials not specified" for atom in set(atoms.get_chemical_symbols())
+                },
             )