From 566c08fd732d18b6dcd08ee38a07a9bf88610efa Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 11:56:43 +0100 Subject: [PATCH 01/28] Remove special character from yaml file (causes issues on certain OSs) --- SnB_input_files/incar.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/SnB_input_files/incar.yaml b/SnB_input_files/incar.yaml index 5c4be3a..5f5e282 100644 --- a/SnB_input_files/incar.yaml +++ b/SnB_input_files/incar.yaml @@ -5,8 +5,8 @@ ALGO: "Normal # Change to All if ZHEGV, FEXCP/F or ZBRENT errors encountered (d EDIFFG: -0.01 ENCUT: 300 HFSCREEN: 0.208 # correct HSE screening parameter; see https://aip.scitation.org/doi/10.1063/1.2404663 -# Note this 👆 differs from the Materials Project MPHSERelaxSet default of 0.2! (Will cause systematic -# energy shifts in HSE supercell calculations.) +# Note this HFSCREEN value differs from the Materials Project MPHSERelaxSet default of 0.2! This should +# be consistent between all your defect/bulk/competing-phase calculations. IBRION: '2 # Typically more stable / reliable than "1" (RMM-DIIS), but change if ionic convergence is poor (done automatically by snb-run)' ISIF: 2 ISMEAR: 0 From 90e005183c334a04875d307dafbf28661bffd822 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 13:40:18 +0100 Subject: [PATCH 02/28] Refactor to remove duplicated VASP input functions from `doped`, and now use directly from `doped` --- docs/ShakeNBreak_Example_Workflow.ipynb | 4 +- shakenbreak/cli.py | 24 +- shakenbreak/input.py | 171 +++++---- shakenbreak/vasp.py | 347 ------------------- tests/test_input.py | 121 +++---- tests/test_local.py | 6 +- tests/test_shakenbreak.py | 2 +- tutorials/ShakeNBreak_Example_Workflow.ipynb | 4 +- 8 files changed, 171 insertions(+), 508 deletions(-) delete mode 100644 shakenbreak/vasp.py diff --git a/docs/ShakeNBreak_Example_Workflow.ipynb b/docs/ShakeNBreak_Example_Workflow.ipynb index 769a2ed..589c22e 100644 --- a/docs/ShakeNBreak_Example_Workflow.ipynb +++ b/docs/ShakeNBreak_Example_Workflow.ipynb @@ -2523,7 +2523,7 @@ "source": [ "```{note} \n", "\n", - "Using the `incar_settings` optional argument for `Distortions.write_vasp_files()` above, we can also specify some custom `INCAR` tags to match our converged `ENCUT` for this system and optimal `NCORE` for the HPC we will run the calculations on. Note that any `INCAR` flags that aren't numbers (e.g. `{\"IBRION\": 1}`) or True/False (e.g. `{\"LREAL\": False}`) need to be input as strings with quotation marks (e.g. `{\"ALGO\": \"All\"}`).\n", + "Using the `user_incar_settings` optional argument for `Distortions.write_vasp_files()` above, we can also specify some custom `INCAR` tags to match our converged `ENCUT` for this system and optimal `NCORE` for the HPC we will run the calculations on. Note that any `INCAR` flags that aren't numbers (e.g. `{\"IBRION\": 1}`) or True/False (e.g. `{\"LREAL\": False}`) need to be input as strings with quotation marks (e.g. `{\"ALGO\": \"All\"}`).\n", "\n", "For the recommended default coarse structure-searching `INCAR` settings, either have a look at the `incar.yaml` file in the `SnB_input_files` folder or at the generated files: \n", "```" @@ -2589,7 +2589,7 @@ "Note that the `NELECT` `INCAR` tag (number of electrons) is automatically determined based on the choice\n", " of `POTCAR`s. The default in `ShakeNBreak` (and `doped`) is to use the\n", "[`MPRelaxSet` `POTCAR` choices](https://github.com/materialsproject/pymatgen/blob/master/pymatgen/io/vasp/MPRelaxSet.yaml), but if you're using\n", - "different ones, make sure to set `potcar_settings` in `write_vasp_files()`, so that `NELECT` is then set\n", + "different ones, make sure to set `user_potcar_settings` in `write_vasp_files()`, so that `NELECT` is then set\n", "accordingly.\n", "This requires the `pymatgen` config file `$HOME/.pmgrc.yaml` to be properly set up as detailed on the [Installation](https://shakenbreak.readthedocs.io/en/latest/Installation.html) docs page.\n", "```" diff --git a/shakenbreak/cli.py b/shakenbreak/cli.py index 2a66bb7..72e39c5 100755 --- a/shakenbreak/cli.py +++ b/shakenbreak/cli.py @@ -320,19 +320,19 @@ def generate( if code.lower() == "vasp": if input_file: incar = Incar.from_file(input_file) - incar_settings = incar.as_dict() - [incar_settings.pop(key, None) for key in ["@class", "@module"]] - if not incar_settings: + user_incar_settings = incar.as_dict() + [user_incar_settings.pop(key, None) for key in ["@class", "@module"]] + if not user_incar_settings: warnings.warn( f"Input file {input_file} specified but no valid INCAR tags found. " f"Should be in the format of VASP INCAR file." ) else: - incar_settings = None + user_incar_settings = None distorted_defects_dict, distortion_metadata = Dist.write_vasp_files( verbose=verbose, - potcar_settings=pseudopotentials, - incar_settings=incar_settings, + user_potcar_settings=pseudopotentials, + user_incar_settings=user_incar_settings, ) elif code.lower() == "cp2k": if input_file: @@ -684,19 +684,19 @@ def parse_defect_position(defect_name, defect_settings): if code.lower() == "vasp": if input_file: incar = Incar.from_file(input_file) - incar_settings = incar.as_dict() - [incar_settings.pop(key, None) for key in ["@class", "@module"]] - if incar_settings == {}: + user_incar_settings = incar.as_dict() + [user_incar_settings.pop(key, None) for key in ["@class", "@module"]] + if user_incar_settings == {}: warnings.warn( f"Input file {input_file} specified but no valid INCAR tags found. " f"Should be in the format of VASP INCAR file." ) else: - incar_settings = None + user_incar_settings = None distorted_defects_dict, distortion_metadata = Dist.write_vasp_files( verbose=verbose, - potcar_settings=pseudopotentials, - incar_settings=incar_settings, + user_potcar_settings=pseudopotentials, + user_incar_settings=user_incar_settings, ) elif code.lower() == "cp2k": if input_file: diff --git a/shakenbreak/input.py b/shakenbreak/input.py index 8dbbb93..951e6f6 100755 --- a/shakenbreak/input.py +++ b/shakenbreak/input.py @@ -24,6 +24,7 @@ get_defect_name_from_entry, name_defect_entries, ) +from doped.vasp import DefectDictSet from monty.json import MontyDecoder from monty.serialization import dumpfn, loadfn from pymatgen.analysis.defects.core import Defect @@ -34,16 +35,21 @@ from pymatgen.entries.computed_entries import ComputedStructureEntry from pymatgen.io.ase import AseAtomsAdaptor from pymatgen.io.cp2k.inputs import Cp2kInput -from pymatgen.io.vasp.inputs import UnknownPotcarWarning -from pymatgen.io.vasp.sets import BadInputSetWarning +from pymatgen.io.vasp.inputs import Kpoints, UnknownPotcarWarning +from pymatgen.io.vasp.sets import BadInputSetWarning, UserPotcarFunctional from pymatgen.util.coord import pbc_diff from scipy.cluster.hierarchy import fcluster, linkage from scipy.spatial import Voronoi from scipy.spatial.distance import squareform -from shakenbreak import analysis, distortions, io, vasp +from shakenbreak import analysis, distortions, io MODULE_DIR = os.path.dirname(os.path.abspath(__file__)) +default_potcar_dict = loadfn(f"{MODULE_DIR}/../SnB_input_files/default_POTCARs.yaml") +# Load default INCAR settings for the ShakeNBreak geometry relaxations +default_incar_settings = loadfn( + os.path.join(MODULE_DIR, "../SnB_input_files/incar.yaml") +) warnings.filterwarnings( @@ -178,8 +184,9 @@ def _write_distortion_metadata( def _create_vasp_input( defect_name: str, distorted_defect_dict: dict, - incar_settings: dict, - potcar_settings: Optional[dict] = None, + user_incar_settings: Optional[dict] = None, + user_potcar_functional: Optional[UserPotcarFunctional] = "PBE_54", + user_potcar_settings: Optional[dict] = None, output_path: str = ".", ) -> None: """ @@ -190,13 +197,18 @@ def _create_vasp_input( Folder name distorted_defect_dict (:obj:`dict`): Dictionary with the distorted structures of charged defect - incar_settings (:obj:`dict`): + user_incar_settings (:obj:`dict`): Dictionary of user VASP INCAR settings, to overwrite/update the `doped` defaults. - potcar_settings (:obj:`dict`): - Dictionary of user VASP POTCAR settings, to overwrite/update the - `doped` defaults. Using `pymatgen` syntax (e.g. {'POTCAR': - {'Fe': 'Fe_pv', 'O': 'O'}}). + user_potcar_functional (str): + POTCAR functional to use. Default is "PBE" and if this fails, + tries "PBE_52", then "PBE_54". + user_potcar_settings (:obj:`dict`): + Dictionary of user VASP POTCAR settings, to overwrite/update + the `doped` defaults (e.g. {'Fe': 'Fe_pv', 'O': 'O'}}). Highly + recommended to look at output `POTCAR`s, or `shakenbreak` + `SnB_input_files/default_POTCARs.yaml`, to see what the default + `POTCAR` settings are. (Default: None) output_path (:obj:`str`): Path to directory in which to write distorted defect structures and calculation inputs. @@ -206,10 +218,10 @@ def _create_vasp_input( None """ # create folder for defect - defect_name_wout_charge, charge = defect_name.rsplit( + defect_name_wout_charge, charge_state = defect_name.rsplit( "_", 1 ) # `defect_name` includes charge - charge = int(charge) + charge_state = int(charge_state) test_letters = [ "h", "g", @@ -228,9 +240,10 @@ def _create_vasp_input( for letter in test_letters for dir in os.listdir(output_path) if dir - == f"{defect_name_wout_charge}{letter}_{'+' if charge > 0 else ''}{charge}" + == f"{defect_name_wout_charge}{letter}_{'+' if charge_state > 0 else ''}{charge_state}" and os.path.isdir( - f"{output_path}/{defect_name_wout_charge}{letter}_{'+' if charge > 0 else ''}{charge}" + f"{output_path}/{defect_name_wout_charge}{letter}_{'+' if charge_state > 0 else ''}" + f"{charge_state}" ) ] except Exception: @@ -270,15 +283,20 @@ def _create_vasp_input( for letter in test_letters for dir in matching_dirs if dir - == f"{defect_name_wout_charge}{letter}_{'+' if charge > 0 else ''}{charge}" + == f"{defect_name_wout_charge}{letter}_{'+' if charge_state > 0 else ''}{charge_state}" ][0] - prev_dir_name = f"{defect_name_wout_charge}{last_letter}_{'+' if charge > 0 else ''}{charge}" + prev_dir_name = ( + f"{defect_name_wout_charge}{last_letter}_{'+' if charge_state > 0 else ''}" + f"{charge_state}" + ) if last_letter == "": # rename prev defect folder new_prev_dir_name = ( - f"{defect_name_wout_charge}a_{'+' if charge > 0 else ''}{charge}" + f"{defect_name_wout_charge}a_{'+' if charge_state > 0 else ''}" + f"{charge_state}" ) new_current_dir_name = ( - f"{defect_name_wout_charge}b_{'+' if charge > 0 else ''}{charge}" + f"{defect_name_wout_charge}b_{'+' if charge_state > 0 else ''}" + f"{charge_state}" ) warnings.warn( f"A previously-generated defect folder {prev_dir_name} exists in " @@ -298,7 +316,7 @@ def _create_vasp_input( next_letter = test_letters[test_letters.index(last_letter) - 1] new_current_dir_name = ( f"{defect_name_wout_charge}{next_letter}" - f"_{'+' if charge > 0 else ''}{charge}" + f"_{'+' if charge_state > 0 else ''}{charge_state}" ) warnings.warn( f"Previously-generated defect folders ({prev_dir_name}...) exist in " @@ -311,22 +329,47 @@ def _create_vasp_input( defect_name = new_current_dir_name _create_folder(os.path.join(output_path, defect_name)) + + potcar_settings = copy.deepcopy(default_potcar_dict)["POTCAR"] + potcar_settings.update(user_potcar_settings or {}) + incar_settings = copy.deepcopy(default_incar_settings) + incar_settings.update(user_incar_settings or {}) + single_defect_dict = list(distorted_defect_dict.values())[0] + + num_elements = len(single_defect_dict["Defect Structure"].composition.elements) + incar_settings.update({"ROPT": ("1e-3 " * num_elements).rstrip()}) + + dds = DefectDictSet( # create one DefectDictSet first, then just edit structure & comment for each + single_defect_dict["Defect Structure"], + charge_state=single_defect_dict["Charge State"], + user_incar_settings=incar_settings, + user_kpoints_settings=Kpoints().from_dict( + { + "comment": "Γ-only KPOINTS from ShakeNBreak", + "generation_style": "Gamma", + } + ), + user_potcar_functional=user_potcar_functional, + user_potcar_settings=potcar_settings, + poscar_comment=None, + ) + for ( distortion, single_defect_dict, ) in ( distorted_defect_dict.items() ): # for each distortion, create sub-subfolder folder - potcar_settings_copy = copy.deepcopy( - potcar_settings - ) # files empties `potcar_settings dict` (via pop()), so make a - # deepcopy each time - vasp.write_vasp_gam_files( - single_defect_dict=single_defect_dict, - input_dir=f"{output_path}/{defect_name}/{distortion}", - incar_settings=incar_settings, - potcar_settings=potcar_settings_copy, - ) + dds._structure = single_defect_dict["Defect Structure"] + dds.poscar_comment = single_defect_dict.get("POSCAR Comment", None) + dds.write_input(f"{output_path}/{defect_name}/{distortion}") + # TODO: Should be able to add tests with potcar_spec? By adding kwargs here? + # TODO: Edit output warning when user POTCARs not set up (and check this) + # warnings.warn( + # "POTCAR directory not set up with pymatgen, so only POSCAR files " + # "will be generated (POTCARs also needed to determine appropriate " + # "NELECT setting in INCAR files)" + # ) def _get_bulk_comp(defect_object) -> Composition: @@ -2326,7 +2369,7 @@ def apply_distortions( for each charge state of each defect, in the format: {'defect_name': { 'charges': { - 'charge_state': { + {charge_state}: { 'structures': {...}, }, }, @@ -2404,8 +2447,8 @@ def apply_distortions( ): sorted_distances = np.sort(struct.distance_matrix.flatten()) shortest_interatomic_distance = sorted_distances[len(struct)] - if shortest_interatomic_distance < 1.0 and not any( - [el.symbol == "H" for el in struct.composition.elements] + if shortest_interatomic_distance < 1.0 and all( + el.symbol != "H" for el in struct.composition.elements ): if verbose: warnings.warn( @@ -2421,12 +2464,9 @@ def apply_distortions( "Unperturbed": defect_distorted_structures[ "Unperturbed" ].sc_entry.structure, - "distortions": { - dist: struct - for dist, struct in defect_distorted_structures[ - "distortions" - ].items() - }, + "distortions": dict( + defect_distorted_structures["distortions"].items() + ), } # Store distortion parameters/info in self.distortion_metadata @@ -2448,8 +2488,9 @@ def apply_distortions( def write_vasp_files( self, - incar_settings: Optional[dict] = None, - potcar_settings: Optional[dict] = None, + user_incar_settings: Optional[dict] = None, + user_potcar_functional: Optional[UserPotcarFunctional] = "PBE_54", + user_potcar_settings: Optional[dict] = None, output_path: str = ".", verbose: bool = False, ) -> Tuple[dict, dict]: @@ -2458,23 +2499,23 @@ def write_vasp_files( structures. Args: - incar_settings (:obj:`dict`): + user_incar_settings (:obj:`dict`): Dictionary of user VASP INCAR settings (e.g. {"ENCUT": 300, ...}), to overwrite the `ShakenBreak` defaults for those tags. Highly recommended to look at output `INCAR`s, or `SnB_input_files/incar.yaml` to see what the default `INCAR` settings are. Note that any flags that aren't numbers or True/False need to be input as strings with quotation marks - (e.g. `{"ALGO": "All"}`). - (Default: None) - potcar_settings (:obj:`dict`): + (e.g. `{"ALGO": "All"}`). (Default: None) + user_potcar_functional (str): + POTCAR functional to use. Default is "PBE" and if this fails, + tries "PBE_52", then "PBE_54". + user_potcar_settings (:obj:`dict`): Dictionary of user VASP POTCAR settings, to overwrite/update - the `doped` defaults. Using `pymatgen` syntax - (e.g. {'POTCAR': {'Fe': 'Fe_pv', 'O': 'O'}}). Highly + the `doped` defaults (e.g. {'Fe': 'Fe_pv', 'O': 'O'}}). Highly recommended to look at output `POTCAR`s, or `shakenbreak` `SnB_input_files/default_POTCARs.yaml`, to see what the default - `POTCAR` settings are. - (Default: None) + `POTCAR` settings are. (Default: None) write_files (:obj:`bool`): Whether to write output files (Default: True) output_path (:obj:`str`): @@ -2501,50 +2542,44 @@ def write_vasp_files( # loop for each defect in dict for defect_name, defect_dict in distorted_defects_dict.items(): - dict_transf = { - k: v for k, v in defect_dict.items() if k != "charges" - } # Single defect dict - # loop for each charge state of defect - for charge in defect_dict["charges"]: - charged_defect = {} + for charge_state in defect_dict["charges"]: + charged_defect_dict = {} for key_distortion, struct in zip( [ "Unperturbed", ] + list( - defect_dict["charges"][charge]["structures"][ + defect_dict["charges"][charge_state]["structures"][ "distortions" ].keys() ), - [defect_dict["charges"][charge]["structures"]["Unperturbed"]] + [defect_dict["charges"][charge_state]["structures"]["Unperturbed"]] + list( - defect_dict["charges"][charge]["structures"][ + defect_dict["charges"][charge_state]["structures"][ "distortions" ].values() ), ): poscar_comment = self._generate_structure_comment( defect_name=defect_name, - charge=charge, + charge=charge_state, key_distortion=key_distortion, ) - charged_defect[key_distortion] = { + charged_defect_dict[key_distortion] = { "Defect Structure": struct, "POSCAR Comment": poscar_comment, - "Transformation Dict": copy.deepcopy(dict_transf), + "Charge State": charge_state, } - charged_defect[key_distortion]["Transformation Dict"].update( - {"charge": charge} - ) # Add charge state to transformation dict _create_vasp_input( - defect_name=f"{defect_name}_{'+' if charge > 0 else ''}{charge}", - distorted_defect_dict=charged_defect, - incar_settings=incar_settings, - potcar_settings=potcar_settings, + defect_name=f"{defect_name}_{'+' if charge_state > 0 else ''}{charge_state}", + distorted_defect_dict=charged_defect_dict, + user_incar_settings=user_incar_settings, + user_potcar_functional=user_potcar_functional, + user_potcar_settings=user_potcar_settings, output_path=output_path, ) @@ -2657,7 +2692,7 @@ def write_espresso_files( images=atoms, format="espresso-in", ) - elif pseudopotentials and not write_structures_only: + else: # write complete input file default_input_parameters["SYSTEM"][ "tot_charge" diff --git a/shakenbreak/vasp.py b/shakenbreak/vasp.py deleted file mode 100644 index 35d2df3..0000000 --- a/shakenbreak/vasp.py +++ /dev/null @@ -1,347 +0,0 @@ -"""Module to generate VASP input files for defect calculations""" -import os -import warnings -from copy import deepcopy # See https://stackoverflow.com/a/22341377/14020960 why -from typing import TYPE_CHECKING - -import numpy as np -from monty.io import zopen -from monty.os.path import zpath -from monty.serialization import loadfn -from pymatgen.io.vasp import Incar, Kpoints -from pymatgen.io.vasp.inputs import BadIncarWarning, Potcar, PotcarSingle, incar_params -from pymatgen.io.vasp.sets import BadInputSetWarning, MPRelaxSet - -if TYPE_CHECKING: - import pymatgen.core.periodic_table - import pymatgen.core.structure - - -MODULE_DIR = os.path.dirname(os.path.abspath(__file__)) -default_potcar_dict = loadfn(f"{MODULE_DIR}/../SnB_input_files/default_POTCARs.yaml") -# Load default INCAR settings for the ShakenBreak geometry relaxations -default_incar_settings = loadfn( - os.path.join(MODULE_DIR, "../SnB_input_files/incar.yaml") -) - - -def _check_psp_dir(): # Provided by Katarina Brlec, from github.com/SMTG-UCL/surfaxe - """ - Helper function to check if potcars are set up correctly for use with - pymatgen, to be compatible across pymatgen versions (breaking changes in v2022) - """ - potcar = False - try: - import pymatgen.settings - - pmg_settings = pymatgen.settings.SETTINGS - if "PMG_VASP_PSP_DIR" in pmg_settings: - potcar = True - except ModuleNotFoundError: - try: - import pymatgen - - pmg_settings = pymatgen.SETTINGS - if "PMG_VASP_PSP_DIR" in pmg_settings: - potcar = True - except AttributeError: - from pymatgen.core import SETTINGS - - pmg_settings = SETTINGS - if "PMG_VASP_PSP_DIR" in pmg_settings: - potcar = True - return potcar - - -def _import_psp(): - """Import pmg settings for _PotcarSingleMod. - Duplicated code from doped (from github.com/SMTG-UCL/doped). - """ - pmg_settings = None - try: - import pymatgen.settings - - pmg_settings = pymatgen.settings.SETTINGS - except ModuleNotFoundError: - try: - import pymatgen - - pmg_settings = pymatgen.SETTINGS - except AttributeError: - from pymatgen.core import SETTINGS - - pmg_settings = SETTINGS - - if pmg_settings is None: - raise ValueError("pymatgen settings not found?") - else: - return pmg_settings - - -class _PotcarSingleMod(PotcarSingle): - """Modified PotcarSingle class.""" - - def __init__(self, *args, **kwargs): - super(self.__class__, self).__init__(*args, **kwargs) - - @staticmethod - def from_symbol_and_functional(symbol, functional=None): - settings = _import_psp() - if functional is None: - functional = settings.get("PMG_DEFAULT_FUNCTIONAL", "PBE") - funcdir = PotcarSingle.functional_dir[functional] - - if not os.path.isdir(os.path.join(settings.get("PMG_VASP_PSP_DIR"), funcdir)): - functional_dir = { - "LDA_US": "pot", - "PW91_US": "pot_GGA", - "LDA": "potpaw", - "PW91": "potpaw_GGA", - "LDA_52": "potpaw_LDA.52", - "LDA_54": "potpaw_LDA.54", - "PBE": "potpaw_PBE", - "PBE_52": "potpaw_PBE.52", - "PBE_54": "potpaw_PBE.54", - } - funcdir = functional_dir[functional] - - d = settings.get("PMG_VASP_PSP_DIR") - if d is None: - raise ValueError( - "No POTCAR directory found. Please set " - "the VASP_PSP_DIR environment variable" - ) - - paths_to_try = [ - os.path.join(d, funcdir, "POTCAR.{}".format(symbol)), - os.path.join(d, funcdir, symbol, "POTCAR.Z"), - os.path.join(d, funcdir, symbol, "POTCAR"), - ] - for p in paths_to_try: - p = os.path.expanduser(p) - p = zpath(p) - if os.path.exists(p): - return _PotcarSingleMod.from_file(p) - raise IOError( - "You do not have the right POTCAR with functional " - + f"{functional} and label {symbol} in your VASP_PSP_DIR" - ) - - -class _PotcarMod(Potcar): - """Modified Potcar class.""" - - def __init__(self, **kwargs): - super(self.__class__, self).__init__(**kwargs) - - def set_symbols(self, symbols, functional=None, sym_potcar_map=None): - """ - Initialize the POTCAR from a set of symbols. Currently, the POTCARs can - be fetched from a location specified in .pmgrc.yaml. Use pmg config - to add this setting. - - Args: - symbols ([str]): A list of element symbols - functional (str): The functional to use. If None, the setting - PMG_DEFAULT_FUNCTIONAL in .pmgrc.yaml is used, or if this is - not set, it will default to PBE. - sym_potcar_map (dict): A map of symbol:raw POTCAR string. If - sym_potcar_map is specified, POTCARs will be generated from - the given map data rather than the config file location. - """ - del self[:] - if sym_potcar_map: - for el in symbols: - self.append(_PotcarSingleMod(sym_potcar_map[el])) - else: - for el in symbols: - p = _PotcarSingleMod.from_symbol_and_functional(el, functional) - self.append(p) - - -class DefectRelaxSet(MPRelaxSet): - """ - Extension to MPRelaxSet which modifies some parameters appropriate - for defect calculations. - - Args: - charge (:obj:`int`): - Charge of the defect structure - """ - - def __init__(self, structure, **kwargs): - charge = kwargs.pop("charge", 0) - super(self.__class__, self).__init__(structure, **kwargs) - self.charge = charge - - @property - def incar(self): - """Get Incar object""" - inc = super(self.__class__, self).incar - try: - inc["NELECT"] = self.nelect - self.charge - except Exception: - print("NELECT flag is not set due to non-availability of POTCARs") - - return inc - - @property - def potcar(self): - """Potcar object.""" - return _PotcarMod( - symbols=self.potcar_symbols, functional=self.potcar_functional - ) - - @property - def all_input(self): - """ - Returns all input files as a dict of {filename: vasp object} - - Returns: - dict of {filename: object}, e.g., {'INCAR': Incar object, ...} - """ - try: - return super(DefectRelaxSet, self).all_input - except Exception: # Expecting the error to be POTCAR related, its ignored - kpoints = self.kpoints - incar = self.incar - if np.product(kpoints.kpts) < 4 and incar.get("ISMEAR", 0) == -5: - incar["ISMEAR"] = 0 - - return {"INCAR": incar, "KPOINTS": kpoints, "POSCAR": self.poscar} - - -def _scaled_ediff(natoms): # 1e-5 for 50 atoms, up to max 1e-4 - ediff = float(f"{((natoms/50)*1e-5):.1g}") - return ediff if ediff <= 1e-4 else 1e-4 - - -def write_vasp_gam_files( - single_defect_dict: dict, - input_dir: str = None, - incar_settings: dict = None, - potcar_settings: dict = None, -) -> None: - """ - Generates input files for vasp Gamma-point-only relaxation. - - Args: - single_defect_dict (:obj:`dict`): - Single defect-dictionary from prepare_vasp_defect_inputs() - output dictionary of defect calculations (see example notebook) - input_dir (:obj:`str`): - Folder in which to create vasp_gam calculation inputs folder - (Recommended to set as the key of the prepare_vasp_defect_inputs() - output directory) - (default: None) - incar_settings (:obj:`dict`): - Dictionary of user INCAR settings (AEXX, NCORE etc.) to override - default settings. Highly recommended to look at - `/SnB_input_files/incar.yaml`, or output INCARs or doped.vasp_input - source code, to see what the default INCAR settings are. - Note that any flags that aren't numbers or True/False need to be - input as strings with quotation marks (e.g. `{"ALGO": "All"}`). - (default: None) - potcar_settings (:obj:`dict`): - Dictionary of user POTCAR settings to override default settings. - Highly recommended to look at `default_potcar_dict` from - doped.vasp_input to see what the (Pymatgen) syntax and doped - default settings are. - (default: None) - - Returns: - None - """ - supercell = single_defect_dict["Defect Structure"] - num_elements = len(supercell.composition.elements) # for ROPT setting in INCAR - poscar_comment = ( - single_defect_dict["POSCAR Comment"] - if "POSCAR Comment" in single_defect_dict - else None - ) - - # Directory - vaspgaminputdir = input_dir + "/" if input_dir else "VASP_Files/" - if not os.path.exists(vaspgaminputdir): - os.makedirs(vaspgaminputdir) - - warnings.filterwarnings( - "ignore", category=BadInputSetWarning - ) # Ignore POTCAR warnings because Pymatgen incorrectly detecting POTCAR types - potcar_dict = deepcopy(default_potcar_dict) - if potcar_settings: - if "POTCAR_FUNCTIONAL" in potcar_settings.keys(): - potcar_dict["POTCAR_FUNCTIONAL"] = potcar_settings["POTCAR_FUNCTIONAL"] - if "POTCAR" in potcar_settings.keys(): - potcar_dict["POTCAR"].update(potcar_settings.pop("POTCAR")) - - defect_relax_set = DefectRelaxSet( - supercell, - charge=single_defect_dict["Transformation Dict"]["charge"], - user_potcar_settings=potcar_dict["POTCAR"], - user_potcar_functional=potcar_dict["POTCAR_FUNCTIONAL"], - ) - potcars = _check_psp_dir() - if potcars: - defect_relax_set.potcar.write_file(vaspgaminputdir + "POTCAR") - else: # make the folders without POTCARs - warnings.warn( - "POTCAR directory not set up with pymatgen, so only POSCAR files " - "will be generated (POTCARs also needed to determine appropriate " - "NELECT setting in INCAR files)" - ) - vaspgamposcar = defect_relax_set.poscar - if poscar_comment: - vaspgamposcar.comment = poscar_comment - vaspgamposcar.write_file(vaspgaminputdir + "POSCAR") - return # exit here - - relax_set_incar = defect_relax_set.incar - try: - # Only set if change in NELECT - nelect = relax_set_incar.as_dict()["NELECT"] - except KeyError: - # Get NELECT if no change (-dNELECT = 0) - nelect = defect_relax_set.nelect - - # Update system dependent parameters - default_incar_settings_copy = default_incar_settings.copy() - default_incar_settings_copy.update( - { - "NELECT": nelect, - "NUPDOWN": f"{nelect % 2:.0f} # But could be {nelect % 2 + 2:.0f} " - + "if strong spin polarisation or magnetic behaviour present", - "EDIFF": f"{_scaled_ediff(supercell.num_sites)} # May need to reduce for tricky relaxations", - "ROPT": ("1e-3 " * num_elements).rstrip(), - } - ) - if incar_settings: - for ( - k - ) in ( - incar_settings.keys() - ): # check INCAR flags and warn if they don't exist (typos) - if k not in incar_params.keys() and not k.startswith( - "#" - ): # comment tag # this code is taken from pymatgen.io.vasp.inputs - warnings.warn( # but only checking keys, not values so we can add comments etc - f"Cannot find {k} from your incar_settings in the list of " - "INCAR flags", - BadIncarWarning, - ) - default_incar_settings_copy.update(incar_settings) - - vaspgamincar = Incar.from_dict(default_incar_settings_copy) - - # kpoints - vaspgamkpts = Kpoints().from_dict( - {"comment": "Gamma-only KPOINTS from ShakeNBreak", "generation_style": "Gamma"} - ) - - vaspgamposcar = defect_relax_set.poscar - if poscar_comment: - vaspgamposcar.comment = poscar_comment - vaspgamposcar.write_file(vaspgaminputdir + "POSCAR") - with zopen(vaspgaminputdir + "INCAR", "wt") as incar_file: - incar_file.write(vaspgamincar.get_str()) - vaspgamkpts.write_file(vaspgaminputdir + "KPOINTS") diff --git a/tests/test_input.py b/tests/test_input.py index fbdbd15..71fb483 100755 --- a/tests/test_input.py +++ b/tests/test_input.py @@ -874,30 +874,20 @@ def test_apply_snb_distortions_kwargs(self, mock_print): # test create_folder and create_vasp_input simultaneously: def test_create_vasp_input(self): """Test create_vasp_input function""" + # Create doped/PyCDT-style defect dict: supercell = self.V_Cd_dict["supercell"] - V_Cd_defect_relax_set = vasp.DefectRelaxSet(supercell["structure"], charge=0) - poscar = V_Cd_defect_relax_set.poscar - struct = V_Cd_defect_relax_set.structure - dict_transf = { - "defect_type": self.V_Cd_dict["name"], - "defect_site": self.V_Cd_dict["unique_site"], - "defect_supercell_site": self.V_Cd_dict["bulk_supercell_site"], - "defect_multiplicity": self.V_Cd_dict["site_multiplicity"], - "charge": 0, - "supercell": supercell["size"], - } - poscar.comment = ( + poscar_comment = ( self.V_Cd_dict["name"] - + str(dict_transf["defect_supercell_site"].frac_coords) + + str(self.V_Cd_dict["bulk_supercell_site"].frac_coords) + "_-dNELECT=" # change in NELECT from bulk supercell + str(0) ) vasp_defect_inputs = { "vac_1_Cd_0": { - "Defect Structure": struct, - "POSCAR Comment": poscar.comment, - "Transformation Dict": dict_transf, + "Defect Structure": supercell, + "POSCAR Comment": poscar_comment, + "Charge State": 0, } } V_Cd_updated_charged_defect_dict = _update_struct_defect_dict( @@ -912,13 +902,11 @@ def test_create_vasp_input(self): input._create_vasp_input( "vac_1_Cd_0", distorted_defect_dict=V_Cd_charged_defect_dict, - incar_settings=vasp.default_incar_settings, + user_incar_settings=vasp.default_incar_settings, + ) + V_Cd_POSCAR = self._check_V_Cd_rattled_poscar( + "vac_1_Cd_0/Bond_Distortion_-50.0%" ) - V_Cd_Bond_Distortion_folder = "vac_1_Cd_0/Bond_Distortion_-50.0%" - self.assertTrue(os.path.exists(V_Cd_Bond_Distortion_folder)) - V_Cd_POSCAR = Poscar.from_file(V_Cd_Bond_Distortion_folder + "/POSCAR") - self.assertEqual(V_Cd_POSCAR.comment, "V_Cd Rattled") - self.assertEqual(V_Cd_POSCAR.structure, self.V_Cd_minus0pt5_struc_rattled) # only test POSCAR as INCAR, KPOINTS and POTCAR not written on GitHub actions, # but tested locally @@ -937,27 +925,15 @@ def test_create_vasp_input(self): input._create_vasp_input( "vac_1_Cd_0", distorted_defect_dict=V_Cd_charged_defect_dict, - incar_settings=kwarged_incar_settings, - ) - self.assertTrue( - any( # here we get this warning because no Unperturbed structures were - # written so couldn't be compared - f"A previously-generated defect folder vac_1_Cd_0 exists in " - f"{os.path.basename(os.path.abspath('.'))}, and the Unperturbed defect structure " - f"could not be matched to the current defect species: vac_1_Cd_0. These are assumed " - f"to be inequivalent defects, so the previous vac_1_Cd_0 will be renamed to " - f"vac_1_Cda_0 and ShakeNBreak files for the current defect will be saved to " - f"vac_1_Cdb_0, to prevent overwriting." in str(warning.message) - for warning in w + user_incar_settings=kwarged_incar_settings, ) + self._check_V_Cd_folder_renaming( + w, + 'A previously-generated defect folder vac_1_Cd_0 exists in ', + ', and the Unperturbed defect structure could not be matched to the current defect species: vac_1_Cd_0. These are assumed to be inequivalent defects, so the previous vac_1_Cd_0 will be renamed to vac_1_Cda_0 and ShakeNBreak files for the current defect will be saved to vac_1_Cdb_0, to prevent overwriting.', ) - self.assertFalse(os.path.exists("vac_1_Cd_0")) - self.assertTrue(os.path.exists("vac_1_Cda_0")) - self.assertTrue(os.path.exists("vac_1_Cdb_0")) V_Cd_kwarg_folder = "vac_1_Cdb_0/Bond_Distortion_-50.0%" - V_Cd_POSCAR = Poscar.from_file(V_Cd_kwarg_folder + "/POSCAR") - self.assertEqual(V_Cd_POSCAR.comment, "V_Cd Rattled") - self.assertEqual(V_Cd_POSCAR.structure, self.V_Cd_minus0pt5_struc_rattled) + V_Cd_POSCAR = self._check_V_Cd_rattled_poscar(V_Cd_kwarg_folder) # only test POSCAR as INCAR, KPOINTS and POTCAR not written on GitHub actions, # but tested locally @@ -965,15 +941,12 @@ def test_create_vasp_input(self): input._create_vasp_input( "vac_1_Cd_0", distorted_defect_dict=V_Cd_charged_defect_dict, - incar_settings=kwarged_incar_settings, + user_incar_settings=kwarged_incar_settings, output_path="test_path", ) - V_Cd_kwarg_folder = "test_path/vac_1_Cd_0/Bond_Distortion_-50.0%" - self.assertTrue(os.path.exists(V_Cd_kwarg_folder)) - V_Cd_POSCAR = Poscar.from_file(V_Cd_kwarg_folder + "/POSCAR") - self.assertEqual(V_Cd_POSCAR.comment, "V_Cd Rattled") - self.assertEqual(V_Cd_POSCAR.structure, self.V_Cd_minus0pt5_struc_rattled) - + V_Cd_POSCAR = self._check_V_Cd_rattled_poscar( + "test_path/vac_1_Cd_0/Bond_Distortion_-50.0%" + ) # Test correct handling of cases where defect folders with the same name have previously # been written: # 1. If the Unperturbed defect structure cannot be matched to the current defect species, @@ -997,20 +970,13 @@ def test_create_vasp_input(self): input._create_vasp_input( "vac_1_Cd_0", distorted_defect_dict=V_Cd_charged_defect_dict, - incar_settings={}, - ) - self.assertTrue( - any( - f"The previously-generated defect folder vac_1_Cdb_0 in " - f"{os.path.basename(os.path.abspath('.'))} has the same Unperturbed defect " - f"structure as the current defect species: vac_1_Cd_0. ShakeNBreak files in " - f"vac_1_Cdb_0 will be overwritten." in str(warning.message) - for warning in w + user_incar_settings={}, ) + self._check_V_Cd_folder_renaming( + w, + 'The previously-generated defect folder vac_1_Cdb_0 in ', + ' has the same Unperturbed defect structure as the current defect species: vac_1_Cd_0. ShakeNBreak files in vac_1_Cdb_0 will be overwritten.', ) - self.assertFalse(os.path.exists("vac_1_Cd_0")) - self.assertTrue(os.path.exists("vac_1_Cda_0")) - self.assertTrue(os.path.exists("vac_1_Cdb_0")) self.assertFalse(os.path.exists("vac_1_Cdc_0")) V_Cd_POSCAR = Poscar.from_file("vac_1_Cdb_0/Unperturbed/POSCAR") self.assertEqual(V_Cd_POSCAR.comment, "V_Cd Unperturbed, Overwritten") @@ -1027,15 +993,31 @@ def test_create_vasp_input(self): input._create_vasp_input( "vac_1_Cd_0", distorted_defect_dict=V_Cd_charged_defect_dict, - incar_settings={}, + user_incar_settings={}, ) + self._check_V_Cd_folder_renaming( + w, + 'Previously-generated defect folders (vac_1_Cdb_0...) exist in ', + ', and the Unperturbed defect structures could not be matched to the current defect species: vac_1_Cd_0. These are assumed to be inequivalent defects, so ShakeNBreak files for the current defect will be saved to vac_1_Cdc_0 to prevent overwriting.', + ) + self.assertTrue(os.path.exists("vac_1_Cdc_0")) + self.assertFalse(os.path.exists("vac_1_Cdd_0")) + V_Cd_prev_POSCAR = Poscar.from_file("vac_1_Cdb_0/Unperturbed/POSCAR") + self.assertEqual(V_Cd_prev_POSCAR.comment, "V_Cd Unperturbed, Overwritten") + V_Cd_new_POSCAR = Poscar.from_file("vac_1_Cdc_0/Unperturbed/POSCAR") + self.assertEqual(V_Cd_new_POSCAR.comment, "V_Cd Rattled, New Folder") + self.assertEqual(V_Cd_new_POSCAR.structure, self.V_Cd_minus0pt5_struc_rattled) + + def _check_V_Cd_rattled_poscar(self, defect_dir): + result = Poscar.from_file(f"{defect_dir}/POSCAR") + self.assertEqual(result.comment, "V_Cd Rattled") + self.assertEqual(result.structure, self.V_Cd_minus0pt5_struc_rattled) + return result + + def _check_V_Cd_folder_renaming(self, w, top_dir, defect_dir): self.assertTrue( any( - f"Previously-generated defect folders (vac_1_Cdb_0...) exist in " - f"{os.path.basename(os.path.abspath('.'))}, and the Unperturbed defect structures " - f"could not be matched to the current defect species: vac_1_Cd_0. These are " - f"assumed to be inequivalent defects, so ShakeNBreak files for the current defect " - f"will be saved to vac_1_Cdc_0 to prevent overwriting." + f"{top_dir}{os.path.basename(os.path.abspath('.'))}{defect_dir}" in str(warning.message) for warning in w ) @@ -1043,13 +1025,6 @@ def test_create_vasp_input(self): self.assertFalse(os.path.exists("vac_1_Cd_0")) self.assertTrue(os.path.exists("vac_1_Cda_0")) self.assertTrue(os.path.exists("vac_1_Cdb_0")) - self.assertTrue(os.path.exists("vac_1_Cdc_0")) - self.assertFalse(os.path.exists("vac_1_Cdd_0")) - V_Cd_prev_POSCAR = Poscar.from_file("vac_1_Cdb_0/Unperturbed/POSCAR") - self.assertEqual(V_Cd_prev_POSCAR.comment, "V_Cd Unperturbed, Overwritten") - V_Cd_new_POSCAR = Poscar.from_file("vac_1_Cdc_0/Unperturbed/POSCAR") - self.assertEqual(V_Cd_new_POSCAR.comment, "V_Cd Rattled, New Folder") - self.assertEqual(V_Cd_new_POSCAR.structure, self.V_Cd_minus0pt5_struc_rattled) def test_generate_defect_object(self): """Test generate_defect_object""" @@ -1326,7 +1301,7 @@ def test_write_vasp_files(self): ) with patch("builtins.print") as mock_print: _, distortion_metadata = dist.write_vasp_files( - incar_settings={"ENCUT": 212, "IBRION": 0, "EDIFF": 1e-4}, + user_incar_settings={"ENCUT": 212, "IBRION": 0, "EDIFF": 1e-4}, verbose=False, ) diff --git a/tests/test_local.py b/tests/test_local.py index d84060e..7f368d0 100644 --- a/tests/test_local.py +++ b/tests/test_local.py @@ -292,7 +292,7 @@ def test_create_vasp_input(self): input._create_vasp_input( "vac_1_Cd_0", distorted_defect_dict=V_Cd_charged_defect_dict, - incar_settings=vasp.default_incar_settings, + user_incar_settings=vasp.default_incar_settings, ) V_Cd_minus50_folder = "vac_1_Cd_0/Bond_Distortion_-50.0%" self.assertTrue(os.path.exists(V_Cd_minus50_folder)) @@ -327,7 +327,7 @@ def test_create_vasp_input(self): input._create_vasp_input( "vac_1_Cd_0", distorted_defect_dict=V_Cd_charged_defect_dict, - incar_settings=kwarged_incar_settings, + user_incar_settings=kwarged_incar_settings, ) V_Cd_kwarg_minus50_folder = "vac_1_Cd_0/Bond_Distortion_-50.0%" self.assertTrue(os.path.exists(V_Cd_kwarg_minus50_folder)) @@ -361,7 +361,7 @@ def test_write_vasp_files(self, mock_print): local_rattle=False, ) distorted_defect_dict, _ = dist.write_vasp_files( - incar_settings={"ENCUT": 212, "IBRION": 0, "EDIFF": 1e-4}, + user_incar_settings={"ENCUT": 212, "IBRION": 0, "EDIFF": 1e-4}, verbose=False, ) diff --git a/tests/test_shakenbreak.py b/tests/test_shakenbreak.py index 90427fe..6d039c3 100644 --- a/tests/test_shakenbreak.py +++ b/tests/test_shakenbreak.py @@ -105,7 +105,7 @@ def test_SnB_integration(self): oxidation_states=oxidation_states, ) distortion_defect_dict, structures_defect_dict = dist.write_vasp_files( - incar_settings={"ENCUT": 212, "IBRION": 0, "EDIFF": 1e-4}, + user_incar_settings={"ENCUT": 212, "IBRION": 0, "EDIFF": 1e-4}, verbose=False, ) shutil.rmtree("vac_1_Cd_0") diff --git a/tutorials/ShakeNBreak_Example_Workflow.ipynb b/tutorials/ShakeNBreak_Example_Workflow.ipynb index a22e905..c9d0023 100644 --- a/tutorials/ShakeNBreak_Example_Workflow.ipynb +++ b/tutorials/ShakeNBreak_Example_Workflow.ipynb @@ -2466,7 +2466,7 @@ "id": "771afcb5-1fcd-4a77-99ea-63899513992d", "metadata": {}, "source": [ - "Using the `incar_settings` optional argument for `Distortions.write_vasp_files()` above, we can also specify some custom `INCAR` tags to match our converged `ENCUT` for this system and optimal `NCORE` for the HPC we will run the calculations on. More information on the distortions generated can be obtained by setting `verbose = True`. Note that any `INCAR` flags that aren't numbers (e.g. `{\"IBRION\": 1}`) or True/False (e.g. `{\"LREAL\": False}`) need to be input as strings with quotation marks (e.g. `{\"ALGO\": \"All\"}`)." + "Using the `user_incar_settings` optional argument for `Distortions.write_vasp_files()` above, we can also specify some custom `INCAR` tags to match our converged `ENCUT` for this system and optimal `NCORE` for the HPC we will run the calculations on. More information on the distortions generated can be obtained by setting `verbose = True`. Note that any `INCAR` flags that aren't numbers (e.g. `{\"IBRION\": 1}`) or True/False (e.g. `{\"LREAL\": False}`) need to be input as strings with quotation marks (e.g. `{\"ALGO\": \"All\"}`)." ] }, { @@ -2536,7 +2536,7 @@ "Note that the `NELECT` `INCAR` tag (number of electrons) is automatically determined based on the choice\n", " of `POTCAR`s. The default in `ShakeNBreak` (and `doped`) is to use the\n", "[`MPRelaxSet` `POTCAR` choices](https://github.com/materialsproject/pymatgen/blob/master/pymatgen/io/vasp/MPRelaxSet.yaml), but if you're using\n", - "different ones, make sure to set `potcar_settings` in `write_vasp_files()`, so that `NELECT` is then set\n", + "different ones, make sure to set `user_potcar_settings` in `write_vasp_files()`, so that `NELECT` is then set\n", "accordingly.\n", "This requires the `pymatgen` config file `$HOME/.pmgrc.yaml` to be properly set up as detailed on the [Installation](https://shakenbreak.readthedocs.io/en/latest/Installation.html) docs page." ] From 265a4a3247449c48e980d4dba2bc02a7fe1396b8 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 15:30:05 +0100 Subject: [PATCH 03/28] Update `test_input` to automatically test POTCARs and INCAR writing if POTCARs available (i.e. if testing locally) --- shakenbreak/input.py | 27 ++++++---- tests/test_input.py | 119 ++++++++++++++++++++++++++++++++++++------- 2 files changed, 119 insertions(+), 27 deletions(-) diff --git a/shakenbreak/input.py b/shakenbreak/input.py index 951e6f6..48706a8 100755 --- a/shakenbreak/input.py +++ b/shakenbreak/input.py @@ -185,9 +185,10 @@ def _create_vasp_input( defect_name: str, distorted_defect_dict: dict, user_incar_settings: Optional[dict] = None, - user_potcar_functional: Optional[UserPotcarFunctional] = "PBE_54", + user_potcar_functional: Optional[UserPotcarFunctional] = "PBE", user_potcar_settings: Optional[dict] = None, output_path: str = ".", + **kwargs, ) -> None: """ Creates folders for storing VASP ShakeNBreak files. @@ -213,6 +214,10 @@ def _create_vasp_input( Path to directory in which to write distorted defect structures and calculation inputs. (Default is current directory = "./") + **kwargs: + Keyword arguments to pass to `DefectDictSet.write_input()` (e.g. + `potcar_spec`). If `potcars` in `kwargs`, then this is passed to + `DefectDictSet()`. Mainly for POTCAR testing on GH Actions. Returns: None @@ -352,6 +357,7 @@ def _create_vasp_input( user_potcar_functional=user_potcar_functional, user_potcar_settings=potcar_settings, poscar_comment=None, + potcars=kwargs.pop("potcars", None), ) for ( @@ -362,7 +368,7 @@ def _create_vasp_input( ): # for each distortion, create sub-subfolder folder dds._structure = single_defect_dict["Defect Structure"] dds.poscar_comment = single_defect_dict.get("POSCAR Comment", None) - dds.write_input(f"{output_path}/{defect_name}/{distortion}") + dds.write_input(f"{output_path}/{defect_name}/{distortion}", **kwargs) # TODO: Should be able to add tests with potcar_spec? By adding kwargs here? # TODO: Edit output warning when user POTCARs not set up (and check this) # warnings.warn( @@ -1683,9 +1689,7 @@ def apply_snb_distortions( "defect_site_index" ] = bond_distorted_defect["defect_site_index"] - elif ( - num_nearest_neighbours == 0 - ): # when no extra/missing electrons, just rattle the structure. + else: # when no extra/missing electrons, just rattle the structure # Likely to be a shallow defect. if defect_type == "vacancy": defect_site_index = None @@ -2250,7 +2254,7 @@ def _generate_structure_comment( approx_coords = ( f"~[{frac_coords[0]:.1f},{frac_coords[1]:.1f},{frac_coords[2]:.1f}]" ) - poscar_comment = ( + return ( str( key_distortion.split("_")[-1] ) # Get distortion factor (-60.%) or 'Rattled' @@ -2262,7 +2266,6 @@ def _generate_structure_comment( ) + f" {approx_coords}" ) - return poscar_comment def _setup_distorted_defect_dict( self, @@ -2489,10 +2492,11 @@ def apply_distortions( def write_vasp_files( self, user_incar_settings: Optional[dict] = None, - user_potcar_functional: Optional[UserPotcarFunctional] = "PBE_54", + user_potcar_functional: Optional[UserPotcarFunctional] = "PBE", user_potcar_settings: Optional[dict] = None, output_path: str = ".", verbose: bool = False, + **kwargs, ) -> Tuple[dict, dict]: """ Generates the input files for `vasp_gam` relaxations of all output @@ -2524,8 +2528,10 @@ def write_vasp_files( (Default is current directory = ".") verbose (:obj:`bool`): Whether to print distortion information (bond atoms and - distances). - (Default: False) + distances). (Default: False) + kwargs: + Additional keyword arguments to pass to `_create_vasp_input()` + (Mainly for testing purposes). Returns: :obj:`tuple`: @@ -2581,6 +2587,7 @@ def write_vasp_files( user_potcar_functional=user_potcar_functional, user_potcar_settings=user_potcar_settings, output_path=output_path, + **kwargs, ) self.write_distortion_metadata(output_path=output_path) diff --git a/tests/test_input.py b/tests/test_input.py index 71fb483..d5cd8a7 100755 --- a/tests/test_input.py +++ b/tests/test_input.py @@ -1,5 +1,6 @@ import copy import datetime +import filecmp import os import shutil import unittest @@ -9,6 +10,7 @@ import numpy as np from ase.build import bulk, make_supercell from ase.calculators.aims import Aims +from doped.vasp import _test_potcar_functional_choice from monty.serialization import dumpfn, loadfn from pymatgen.analysis.defects.generators import VacancyGenerator from pymatgen.analysis.defects.thermo import DefectEntry @@ -16,10 +18,10 @@ from pymatgen.core.structure import Composition, PeriodicSite, Structure from pymatgen.entries.computed_entries import ComputedStructureEntry from pymatgen.io.ase import AseAtomsAdaptor -from pymatgen.io.vasp.inputs import Poscar, UnknownPotcarWarning +from pymatgen.io.vasp.inputs import Poscar, UnknownPotcarWarning, Incar, Kpoints, Potcar -from shakenbreak import distortions, input, vasp -from shakenbreak.distortions import rattle +from shakenbreak import input +from shakenbreak.distortions import rattle, distort def if_present_rm(path): @@ -27,6 +29,19 @@ def if_present_rm(path): shutil.rmtree(path) +def _potcars_available() -> bool: + """ + Check if the POTCARs are available for the tests (i.e. testing locally). + + If not (testing on GitHub Actions), POTCAR testing will be skipped. + """ + try: + _test_potcar_functional_choice("PBE") + return True + except ValueError: + return False + + def _update_struct_defect_dict( defect_dict: dict, structure: Structure, poscar_comment: str ) -> dict: @@ -200,6 +215,12 @@ def setUp(self): ) ) + # get example INCAR: + self.V_Cd_INCAR_file = os.path.join( + self.VASP_CDTE_DATA_DIR, "vac_1_Cd_0/default_INCAR" + ) + self.V_Cd_INCAR = Incar.from_file(self.V_Cd_INCAR_file) + # Setup distortion parameters self.V_Cd_distortion_parameters = { "unique_site": np.array([0.0, 0.0, 0.0]), @@ -902,16 +923,27 @@ def test_create_vasp_input(self): input._create_vasp_input( "vac_1_Cd_0", distorted_defect_dict=V_Cd_charged_defect_dict, - user_incar_settings=vasp.default_incar_settings, + potcars=_potcars_available(), # to allow testing on GH Actions ) V_Cd_POSCAR = self._check_V_Cd_rattled_poscar( "vac_1_Cd_0/Bond_Distortion_-50.0%" ) - # only test POSCAR as INCAR, KPOINTS and POTCAR not written on GitHub actions, - # but tested locally + kpoints = Kpoints.from_file("vac_1_Cd_0/Bond_Distortion_-50.0%/KPOINTS") + self.assertEqual(kpoints.kpts, [[1, 1, 1]]) + + if _potcars_available(): + assert filecmp.cmp( + "vac_1_Cd_0/Bond_Distortion_-50.0%/INCAR", self.V_Cd_INCAR_file + ) + + # check if POTCARs have been written: + potcar = Potcar.from_file("vac_1_Cd_0/Bond_Distortion_-50.0%/POTCAR") + assert set(potcar.as_dict()["symbols"]) == { + input.default_potcar_dict["POTCAR"][el_symbol] + for el_symbol in V_Cd_POSCAR.structure.symbol_set + } - # test with kwargs: (except POTCAR settings because we can't have this on the GitHub test - # server) + # test with kwargs: kwarg_incar_settings = { "NELECT": 3, "IBRION": 42, @@ -929,24 +961,74 @@ def test_create_vasp_input(self): ) self._check_V_Cd_folder_renaming( w, - 'A previously-generated defect folder vac_1_Cd_0 exists in ', - ', and the Unperturbed defect structure could not be matched to the current defect species: vac_1_Cd_0. These are assumed to be inequivalent defects, so the previous vac_1_Cd_0 will be renamed to vac_1_Cda_0 and ShakeNBreak files for the current defect will be saved to vac_1_Cdb_0, to prevent overwriting.', + "A previously-generated defect folder vac_1_Cd_0 exists in ", + ", and the Unperturbed defect structure could not be matched to the current defect species: " + "vac_1_Cd_0. These are assumed to be inequivalent defects, so the previous vac_1_Cd_0 will " + "be renamed to vac_1_Cda_0 and ShakeNBreak files for the current defect will be saved to " + "vac_1_Cdb_0, to prevent overwriting.", ) V_Cd_kwarg_folder = "vac_1_Cdb_0/Bond_Distortion_-50.0%" V_Cd_POSCAR = self._check_V_Cd_rattled_poscar(V_Cd_kwarg_folder) - # only test POSCAR as INCAR, KPOINTS and POTCAR not written on GitHub actions, - # but tested locally + kpoints = Kpoints.from_file(f"{V_Cd_kwarg_folder}/KPOINTS") + self.assertEqual(kpoints.kpts, [[1, 1, 1]]) + + if _potcars_available(): + assert not filecmp.cmp( # INCAR settings changed now + f"{V_Cd_kwarg_folder}/INCAR", self.V_Cd_INCAR_file + ) + assert self.V_Cd_INCAR != Incar.from_file(f"{V_Cd_kwarg_folder}/INCAR") + kwarged_INCAR = self.V_Cd_INCAR.copy() + kwarged_INCAR.update(kwarg_incar_settings) + kwarged_INCAR["NELECT"] = 812.0 # changed POTCARs + assert kwarged_INCAR == Incar.from_file(f"{V_Cd_kwarg_folder}/INCAR") + + # check if POTCARs have been written: + potcar = Potcar.from_file(f"{V_Cd_kwarg_folder}/POTCAR") + assert set(potcar.as_dict()["symbols"]) == { + "Cd_sv", + "Te_GW", + } # Cd_sv_GW POTCAR has Cd_sv + # symbol, checked # test output_path option input._create_vasp_input( "vac_1_Cd_0", distorted_defect_dict=V_Cd_charged_defect_dict, - user_incar_settings=kwarged_incar_settings, + user_incar_settings=kwarg_incar_settings, output_path="test_path", + potcars=_potcars_available(), # to allow testing on GH Actions ) V_Cd_POSCAR = self._check_V_Cd_rattled_poscar( "test_path/vac_1_Cd_0/Bond_Distortion_-50.0%" ) + kpoints = Kpoints.from_file( + "test_path/vac_1_Cd_0/Bond_Distortion_-50.0%/KPOINTS" + ) + self.assertEqual(kpoints.kpts, [[1, 1, 1]]) + + if _potcars_available(): + assert not filecmp.cmp( # INCAR settings changed now + "test_path/vac_1_Cd_0/Bond_Distortion_-50.0%/INCAR", + self.V_Cd_INCAR_file, + ) + assert self.V_Cd_INCAR != Incar.from_file( + "test_path/vac_1_Cd_0/Bond_Distortion_-50.0%/INCAR" + ) + kwarged_INCAR = self.V_Cd_INCAR.copy() + kwarged_INCAR.update(kwarg_incar_settings) + assert kwarged_INCAR == Incar.from_file( + "test_path/vac_1_Cd_0/Bond_Distortion_-50.0%/INCAR" + ) + + # check if POTCARs have been written: + potcar = Potcar.from_file( + "test_path/vac_1_Cd_0/Bond_Distortion_-50.0%/POTCAR" + ) + assert set(potcar.as_dict()["symbols"]) == { + input.default_potcar_dict["POTCAR"][el_symbol] + for el_symbol in V_Cd_POSCAR.structure.symbol_set + } + # Test correct handling of cases where defect folders with the same name have previously # been written: # 1. If the Unperturbed defect structure cannot be matched to the current defect species, @@ -971,11 +1053,14 @@ def test_create_vasp_input(self): "vac_1_Cd_0", distorted_defect_dict=V_Cd_charged_defect_dict, user_incar_settings={}, + potcars=_potcars_available(), # to allow testing on GH Actions + user_potcar_functional="PBE_54", # check setting POTCAR functional to one that isn't + # present locally ) self._check_V_Cd_folder_renaming( w, - 'The previously-generated defect folder vac_1_Cdb_0 in ', - ' has the same Unperturbed defect structure as the current defect species: vac_1_Cd_0. ShakeNBreak files in vac_1_Cdb_0 will be overwritten.', + "The previously-generated defect folder vac_1_Cdb_0 in ", + " has the same Unperturbed defect structure as the current defect species: vac_1_Cd_0. ShakeNBreak files in vac_1_Cdb_0 will be overwritten.", ) self.assertFalse(os.path.exists("vac_1_Cdc_0")) V_Cd_POSCAR = Poscar.from_file("vac_1_Cdb_0/Unperturbed/POSCAR") @@ -997,8 +1082,8 @@ def test_create_vasp_input(self): ) self._check_V_Cd_folder_renaming( w, - 'Previously-generated defect folders (vac_1_Cdb_0...) exist in ', - ', and the Unperturbed defect structures could not be matched to the current defect species: vac_1_Cd_0. These are assumed to be inequivalent defects, so ShakeNBreak files for the current defect will be saved to vac_1_Cdc_0 to prevent overwriting.', + "Previously-generated defect folders (vac_1_Cdb_0...) exist in ", + ", and the Unperturbed defect structures could not be matched to the current defect species: vac_1_Cd_0. These are assumed to be inequivalent defects, so ShakeNBreak files for the current defect will be saved to vac_1_Cdc_0 to prevent overwriting.", ) self.assertTrue(os.path.exists("vac_1_Cdc_0")) self.assertFalse(os.path.exists("vac_1_Cdd_0")) From bab17b3cd5e9a31e4ac624bd68ec375e194c0dbc Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 15:54:28 +0100 Subject: [PATCH 04/28] Add KPOINTS/POTCAR/INCAR tests to `test_write_vasp_files()` --- tests/data/vasp/CdTe/vac_1_Cd_0/default_INCAR | 35 +++++ tests/test_input.py | 136 +++++++++++++++--- 2 files changed, 154 insertions(+), 17 deletions(-) create mode 100644 tests/data/vasp/CdTe/vac_1_Cd_0/default_INCAR diff --git a/tests/data/vasp/CdTe/vac_1_Cd_0/default_INCAR b/tests/data/vasp/CdTe/vac_1_Cd_0/default_INCAR new file mode 100644 index 0000000..9a186a5 --- /dev/null +++ b/tests/data/vasp/CdTe/vac_1_Cd_0/default_INCAR @@ -0,0 +1,35 @@ +# KPAR = # no kpar, only one kpoint +# May want to change NCORE, KPAR, AEXX, ENCUT, NUPDOWN, ISPIN, POTIM = +# ShakeNBreak INCAR with coarse settings to maximise speed with sufficient accuracy for qualitative structure searching = +AEXX = 0.25 +ALGO = Normal # change to all if zhegv, fexcp/f or zbrent errors encountered (done automatically by snb-run) +EDIFF = 1e-05 +EDIFFG = -0.01 +ENCUT = 300 +GGA = Pe +HFSCREEN = 0.208 +IBRION = 2 +ICHARG = 1 +ICORELEVEL = 0 # needed if using the kumagai-oba (efnv) anisotropic charge correction scheme +ISIF = 2 +ISMEAR = 0 +ISPIN = 2 +ISYM = 0 # symmetry breaking extremely likely for defects +LASPH = True +LCHARG = False +LHFCALC = True +LMAXMIX = 4 +LORBIT = 11 +LREAL = Auto +LVHAR = True +LWAVE = False +NCORE = 16 +NEDOS = 2000 +NELECT = 564.0 +NELM = 40 +NSW = 300 +NUPDOWN = 0 +PREC = Accurate +PRECFOCK = Fast +ROPT = 1e-3 1e-3 +SIGMA = 0.05 diff --git a/tests/test_input.py b/tests/test_input.py index d5cd8a7..7cd4762 100755 --- a/tests/test_input.py +++ b/tests/test_input.py @@ -529,7 +529,7 @@ def test_apply_rattle_bond_distortions_V_Cd(self): verbose=True, ) vac_coords = np.array([0, 0, 0]) # Cd vacancy fractional coordinates - output = distortions.distort(self.V_Cd_struc, 2, 0.5, frac_coords=vac_coords) + output = distort(self.V_Cd_struc, 2, 0.5, frac_coords=vac_coords) np.testing.assert_raises( AssertionError, np.testing.assert_array_equal, V_Cd_distorted_dict, output ) # Shouldn't match because rattling not done yet @@ -539,9 +539,7 @@ def test_apply_rattle_bond_distortions_V_Cd(self): rattling_atom_indices = rattling_atom_indices[ ~idx ] # removed distorted Te indices - output[ - "distorted_structure" - ] = distortions.rattle( # overwrite with distorted and rattle + output["distorted_structure"] = rattle( # overwrite with distorted and rattle # structure output["distorted_structure"], d_min=d_min, @@ -581,7 +579,7 @@ def test_apply_rattle_bond_distortions_Int_Cd_2(self): stdev=0.28333683853583164, # 10% of CdTe bond length, default seed=40, # distortion_factor * 100, default ) - output = distortions.distort(self.Int_Cd_2_struc, 2, 0.4, site_index=65) + output = distort(self.Int_Cd_2_struc, 2, 0.4, site_index=65) np.testing.assert_raises( AssertionError, np.testing.assert_array_equal, @@ -596,9 +594,7 @@ def test_apply_rattle_bond_distortions_Int_Cd_2(self): rattling_atom_indices = rattling_atom_indices[ ~idx ] # removed distorted Cd indices - output[ - "distorted_structure" - ] = distortions.rattle( # overwrite with distorted and rattle + output["distorted_structure"] = rattle( # overwrite with distorted and rattle output["distorted_structure"], d_min=d_min, active_atoms=rattling_atom_indices, @@ -945,19 +941,19 @@ def test_create_vasp_input(self): # test with kwargs: kwarg_incar_settings = { - "NELECT": 3, "IBRION": 42, "LVHAR": True, "LWAVE": True, "LCHARG": True, + "AEXX": 0.35, } - kwarged_incar_settings = vasp.default_incar_settings.copy() - kwarged_incar_settings.update(kwarg_incar_settings) with warnings.catch_warnings(record=True) as w: input._create_vasp_input( "vac_1_Cd_0", distorted_defect_dict=V_Cd_charged_defect_dict, - user_incar_settings=kwarged_incar_settings, + user_incar_settings=kwarg_incar_settings, + user_potcar_settings={"Cd": "Cd_sv_GW", "Te": "Te_GW"}, + potcars=_potcars_available(), # to allow testing on GH Actions ) self._check_V_Cd_folder_renaming( w, @@ -1374,8 +1370,55 @@ def test_write_vasp_files(self): oxidation_states = {"Cd": +2, "Te": -2} bond_distortions = list(np.arange(-0.6, 0.601, 0.05)) - # Use customised names for defects + # test input file kwargs: # TODO: Move to bottom after this works! + reduced_Int_Cd_2 = copy.deepcopy(self.Int_Cd_2) + reduced_Int_Cd_2.user_charges = [ + 1, + ] + reduced_Int_Cd_2_entries = [ + input._get_defect_entry_from_defect(reduced_Int_Cd_2, c) + for c in reduced_Int_Cd_2.user_charges + ] + dist = input.Distortions( + {"Int_Cd_2": reduced_Int_Cd_2_entries}, + oxidation_states=oxidation_states, + distortion_increment=0.25, + distorted_elements={"Int_Cd_2": ["Cd"]}, + dict_number_electrons_user={"Int_Cd_2": 3}, + local_rattle=False, + stdev=0.25, # old default + seed=42, # old default + ) + _, distortion_metadata = dist.write_vasp_files( + verbose=True, + user_potcar_settings={"Cd": "Cd_sv_GW", "Te": "Te_GW"}, + user_potcar_functional="PBE_52", + potcars=_potcars_available(), # to allow testing on GH Actions + ) + self.assertTrue(os.path.exists("Int_Cd_2_+1/Unperturbed")) + _int_Cd_2_POSCAR = Poscar.from_file( + "Int_Cd_2_+1/Unperturbed/POSCAR" + ) # test POSCAR loaded fine + kpoints = Kpoints.from_file("Int_Cd_2_+1/Unperturbed/KPOINTS") + self.assertEqual(kpoints.kpts, [[1, 1, 1]]) + + if _potcars_available(): + assert not filecmp.cmp( # INCAR settings changed now + "Int_Cd_2_+1/Unperturbed/INCAR", self.V_Cd_INCAR_file + ) + int_Cd_2_INCAR = Incar.from_file("Int_Cd_2_+1/Unperturbed/INCAR") + v_Cd_INCAR = self.V_Cd_INCAR.copy() + v_Cd_INCAR.pop("NELECT") # NELECT and NUPDOWN differs for the two defects + v_Cd_INCAR.pop("NUPDOWN") + int_Cd_2_INCAR.pop("NELECT") + int_Cd_2_INCAR.pop("NUPDOWN") + assert v_Cd_INCAR == int_Cd_2_INCAR + + # check if POTCARs have been written: + potcar = Potcar.from_file("Int_Cd_2_+1/Unperturbed/POTCAR") + assert set(potcar.as_dict()["symbols"]) == {"Cd_sv", "Te_GW"} + # Use customised names for defects dist = input.Distortions( self.cdte_defects, oxidation_states=oxidation_states, @@ -1388,6 +1431,7 @@ def test_write_vasp_files(self): _, distortion_metadata = dist.write_vasp_files( user_incar_settings={"ENCUT": 212, "IBRION": 0, "EDIFF": 1e-4}, verbose=False, + potcars=_potcars_available(), # to allow testing on GH Actions ) # check if expected folders were created: @@ -1451,8 +1495,28 @@ def test_write_vasp_files(self): "-50.0% N(Distort)=2 ~[0.0,0.0,0.0]", ) # default self.assertEqual(V_Cd_POSCAR.structure, self.V_Cd_minus0pt5_struc_rattled) - # only test POSCAR as INCAR, KPOINTS and POTCAR not written on GitHub actions, - # but tested locally + kpoints = Kpoints.from_file(f"{V_Cd_Bond_Distortion_folder}/KPOINTS") + self.assertEqual(kpoints.kpts, [[1, 1, 1]]) + + if _potcars_available(): + assert not filecmp.cmp( # INCAR settings changed now + f"{V_Cd_Bond_Distortion_folder}/INCAR", self.V_Cd_INCAR_file + ) + assert self.V_Cd_INCAR != Incar.from_file( + f"{V_Cd_Bond_Distortion_folder}/INCAR" + ) + kwarged_INCAR = self.V_Cd_INCAR.copy() + kwarged_INCAR.update({"ENCUT": 212, "IBRION": 0, "EDIFF": 1e-4}) + assert kwarged_INCAR == Incar.from_file( + f"{V_Cd_Bond_Distortion_folder}/INCAR" + ) + + # check if POTCARs have been written: + potcar = Potcar.from_file(f"{V_Cd_Bond_Distortion_folder}/POTCAR") + assert set(potcar.as_dict()["symbols"]) == { + input.default_potcar_dict["POTCAR"][el_symbol] + for el_symbol in V_Cd_POSCAR.structure.symbol_set + } Int_Cd_2_Bond_Distortion_folder = "Int_Cd_2_0/Bond_Distortion_-60.0%" self.assertTrue(os.path.exists(Int_Cd_2_Bond_Distortion_folder)) @@ -1464,8 +1528,46 @@ def test_write_vasp_files(self): self.assertNotEqual( # Int_Cd_2_minus0pt6_struc_rattled is with new default `stdev` & `seed` Int_Cd_2_POSCAR.structure, self.Int_Cd_2_minus0pt6_struc_rattled ) - # only test POSCAR as INCAR, KPOINTS and POTCAR not written on GitHub actions, - # but tested locally + kpoints = Kpoints.from_file(f"{Int_Cd_2_Bond_Distortion_folder}/KPOINTS") + self.assertEqual(kpoints.kpts, [[1, 1, 1]]) + + if _potcars_available(): + assert not filecmp.cmp( # INCAR settings changed now + f"{Int_Cd_2_Bond_Distortion_folder}/INCAR", self.V_Cd_INCAR_file + ) + assert self.V_Cd_INCAR != Incar.from_file( + f"{Int_Cd_2_Bond_Distortion_folder}/INCAR" + ) + kwarged_INCAR = self.V_Cd_INCAR.copy() + kwarged_INCAR.update({"ENCUT": 212, "IBRION": 0, "EDIFF": 1e-4}) + assert kwarged_INCAR == Incar.from_file( + f"{Int_Cd_2_Bond_Distortion_folder}/INCAR" + ) + + # check if POTCARs have been written: + potcar = Potcar.from_file(f"{Int_Cd_2_Bond_Distortion_folder}/POTCAR") + assert set(potcar.as_dict()["symbols"]) == { + input.default_potcar_dict["POTCAR"][el_symbol] + for el_symbol in V_Cd_POSCAR.structure.symbol_set + } + + if _potcars_available(): + assert not filecmp.cmp( # INCAR settings changed now + f"{Int_Cd_2_Bond_Distortion_folder}/INCAR", self.V_Cd_INCAR_file + ) + kwarged_INCAR = self.V_Cd_INCAR.copy() + kwarged_INCAR.update({"ENCUT": 212, "IBRION": 0, "EDIFF": 1e-4}) + kwarged_INCAR.pop("NELECT") # different NELECT for Cd_i_+2 + Int_Cd_2_INCAR = Incar.from_file(f"{Int_Cd_2_Bond_Distortion_folder}/INCAR") + Int_Cd_2_INCAR.pop("NELECT") + assert kwarged_INCAR == Int_Cd_2_INCAR + + # check if POTCARs have been written: + potcar = Potcar.from_file(f"{Int_Cd_2_Bond_Distortion_folder}/POTCAR") + assert set(potcar.as_dict()["symbols"]) == { + input.default_potcar_dict["POTCAR"][el_symbol] + for el_symbol in V_Cd_POSCAR.structure.symbol_set + } # Test `Rattled` folder not generated for non-fully-ionised defects, # and only `Rattled` and `Unperturbed` folders generated for fully-ionised defects From 017b80a4edbe6cc31faa9db1b6055a05dd9e6745 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 16:02:51 +0100 Subject: [PATCH 05/28] Add CLI POTCAR/KPOINTS/INCAR tests --- tests/test_cli.py | 63 +++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 56 insertions(+), 7 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 9176561..4e00d06 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1,5 +1,6 @@ import copy import datetime +import filecmp import json import os import re @@ -17,12 +18,14 @@ # Pymatgen from pymatgen.core.structure import Structure -from pymatgen.io.vasp.inputs import Poscar, UnknownPotcarWarning +from pymatgen.io.vasp.inputs import Poscar, UnknownPotcarWarning, Kpoints, Potcar, Incar from shakenbreak.cli import snb from shakenbreak.distortions import rattle from shakenbreak.input import generate_defect_object +from tests.test_input import _potcars_available + file_path = os.path.dirname(__file__) @@ -88,6 +91,12 @@ def setUp(self): warnings.filterwarnings("ignore", category=DeprecationWarning) warnings.filterwarnings("ignore", category=UnknownPotcarWarning) + # get example INCAR: + self.V_Cd_INCAR_file = os.path.join( + self.VASP_CDTE_DATA_DIR, "vac_1_Cd_0/default_INCAR" + ) + self.V_Cd_INCAR = Incar.from_file(self.V_Cd_INCAR_file) + def tearDown(self): os.chdir(os.path.dirname(__file__)) for i in [ @@ -261,18 +270,26 @@ def test_snb_generate(self): # check if correct files were created: V_Cd_Bond_Distortion_folder = f"{defect_name}_0/Bond_Distortion_-50.0%" self.assertTrue(os.path.exists(V_Cd_Bond_Distortion_folder)) - V_Cd_minus0pt5_rattled_POSCAR = Poscar.from_file( - V_Cd_Bond_Distortion_folder + "/POSCAR" - ) + V_Cd_minus0pt5_rattled_POSCAR = Poscar.from_file(f"{V_Cd_Bond_Distortion_folder}/POSCAR") self.assertEqual( V_Cd_minus0pt5_rattled_POSCAR.comment, - f"-50.0% N(Distort)=2 ~[0.0,0.0,0.0]", + "-50.0% N(Distort)=2 ~[0.0,0.0,0.0]", ) # default self.assertEqual( V_Cd_minus0pt5_rattled_POSCAR.structure, self.V_Cd_minus0pt5_struc_rattled, ) + kpoints = Kpoints.from_file(f"{V_Cd_Bond_Distortion_folder}/KPOINTS") + self.assertEqual(kpoints.kpts, [[1, 1, 1]]) + + if _potcars_available(): + assert filecmp.cmp(f"{V_Cd_Bond_Distortion_folder}/INCAR", self.V_Cd_INCAR_file) + + # check if POTCARs have been written: + potcar = Potcar.from_file(f"{V_Cd_Bond_Distortion_folder}/POTCAR") + assert set(potcar.as_dict()["symbols"]) == {"Cd", "Te"} + # Test recognises distortion_metadata.json: if_present_rm(f"{defect_name}_0") # but distortion_metadata.json still present result = runner.invoke( @@ -833,7 +850,10 @@ def test_snb_generate_config(self): max_attempts: 10000 max_disp: 1.0 seed: 20 -local_rattle: False""" +local_rattle: False +POTCAR_FUNCTIONAL: PBE_52 +POTCAR: + Te: Te_GW""" with open("test_config.yml", "w+") as fp: fp.write(test_yml) runner = CliRunner() @@ -859,6 +879,16 @@ def test_snb_generate_config(self): self.assertEqual( V_Cd_kwarged_POSCAR.structure, self.V_Cd_minus0pt5_struc_kwarged ) + kpoints = Kpoints.from_file(f"{defect_name}_0/Bond_Distortion_-50.0%/KPOINTS") + self.assertEqual(kpoints.kpts, [[1, 1, 1]]) + + if _potcars_available(): + assert filecmp.cmp(f"{defect_name}_0/Bond_Distortion_-50.0%/INCAR", self.V_Cd_INCAR_file) + + # check if POTCARs have been written: + potcar = Potcar.from_file(f"{defect_name}_0/Bond_Distortion_-50.0%/POTCAR") + assert set(potcar.as_dict()["symbols"]) == {"Cd", "Te_GW"} + test_yml = """ oxidation_states: @@ -1202,7 +1232,9 @@ def test_snb_generate_all(self): test_yml = """bond_distortions: [0.3,] local_rattle: True stdev: 0.25 -seed: 42""" # previous default +seed: 42 +POTCAR: + Cd: Cd_sv_GW""" # previous default rattle settings with open("test_config.yml", "w+") as fp: fp.write(test_yml) @@ -1286,6 +1318,23 @@ def test_snb_generate_all(self): Structure.from_file(f"{defect_name}_0/Bond_Distortion_30.0%/POSCAR"), self.V_Cd_0pt3_local_rattled, ) + kpoints = Kpoints.from_file(f"{defect_name}_0/Bond_Distortion_30.0%/KPOINTS") + self.assertEqual(kpoints.kpts, [[1, 1, 1]]) + + if _potcars_available(): + assert not filecmp.cmp(f"{defect_name}_0/Bond_Distortion_30.0%/INCAR", self.V_Cd_INCAR_file) + # NELECT has changed due to POTCARs + + v_Cd_INCAR = Incar.from_file(f"{defect_name}_0/Bond_Distortion_30.0%/INCAR") + v_Cd_INCAR.pop("NELECT") + test_INCAR = self.V_Cd_INCAR.copy() + test_INCAR.pop("NELECT") + assert v_Cd_INCAR == test_INCAR + + # check if POTCARs have been written: + potcar = Potcar.from_file(f"{defect_name}_0/Bond_Distortion_30.0%/POTCAR") + assert set(potcar.as_dict()["symbols"]) == {"Cd_sv", "Te"} + if_present_rm(defects_dir) for charge in range(-2, 3): if_present_rm(f"{defect_name}_{'+' if charge > 0 else ''}{charge}") From daee50b585c53d4602b412d42a9c6ef513cd194f Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 16:03:25 +0100 Subject: [PATCH 06/28] Update CLI config handling --- shakenbreak/cli.py | 32 ++++++++++++++------------------ 1 file changed, 14 insertions(+), 18 deletions(-) diff --git a/shakenbreak/cli.py b/shakenbreak/cli.py index 72e39c5..ddf36d0 100755 --- a/shakenbreak/cli.py +++ b/shakenbreak/cli.py @@ -196,8 +196,12 @@ def generate( for a given defect. """ user_settings = loadfn(config) if config is not None else {} + # Parse POTCARs/pseudopotentials from config file, if specified + user_potcar_functional = user_settings.pop("POTCAR_FUNCTIONAL", "PBE") + user_potcar_settings = user_settings.pop("POTCAR", None) + pseudopotentials = user_settings.pop("pseudopotentials", None) + func_args = list(locals().keys()) - pseudopotentials = None if user_settings: valid_args = [ "defect", @@ -233,13 +237,7 @@ def generate( for key in func_args: if key in user_settings: user_settings.pop(key, None) - # Parse pseudopotentials from config file, if specified - if "POTCAR" in user_settings.keys(): - pseudopotentials = {"POTCAR": deepcopy(user_settings["POTCAR"])} - user_settings.pop("POTCAR", None) - if "pseudopotentials" in user_settings.keys(): - pseudopotentials = deepcopy(user_settings["pseudopotentials"]) - user_settings.pop("pseudopotentials", None) + for key in list(user_settings.keys()): # remove non-sense keys from user_settings if key not in valid_args: @@ -331,7 +329,7 @@ def generate( user_incar_settings = None distorted_defects_dict, distortion_metadata = Dist.write_vasp_files( verbose=verbose, - user_potcar_settings=pseudopotentials, + user_potcar_settings=user_potcar_settings, user_incar_settings=user_incar_settings, ) elif code.lower() == "cp2k": @@ -487,9 +485,13 @@ def generate_all( else: defect_settings, user_settings = {}, {} + # Parse POTCARs/pseudopotentials from config file, if specified + user_potcar_functional = user_settings.pop("POTCAR_FUNCTIONAL", "PBE") + user_potcar_settings = user_settings.pop("POTCAR", None) + pseudopotentials = user_settings.pop("pseudopotentials", None) + func_args = list(locals().keys()) # Specified options take precedence over the ones in the config file - pseudopotentials = None if user_settings: valid_args = [ "defects", @@ -521,13 +523,7 @@ def generate_all( for key in func_args: if key in user_settings: user_settings.pop(key, None) - # Parse pseudopotentials from config file, if specified - if "POTCAR" in user_settings.keys(): - pseudopotentials = {"POTCAR": deepcopy(user_settings["POTCAR"])} - user_settings.pop("POTCAR", None) - if "pseudopotentials" in user_settings.keys(): - pseudopotentials = deepcopy(user_settings["pseudopotentials"]) - user_settings.pop("pseudopotentials", None) + for key in list(user_settings.keys()): # remove non-sense keys from user_settings if key not in valid_args: @@ -695,7 +691,7 @@ def parse_defect_position(defect_name, defect_settings): user_incar_settings = None distorted_defects_dict, distortion_metadata = Dist.write_vasp_files( verbose=verbose, - user_potcar_settings=pseudopotentials, + user_potcar_settings=user_potcar_settings, user_incar_settings=user_incar_settings, ) elif code.lower() == "cp2k": From baa73f5d906c6faef7d2791efd386560163e0482 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 16:41:14 +0100 Subject: [PATCH 07/28] Delete `test_local` as this testing functionality is now automatically run in `test_input`/`test_cli` if POTCARs available --- tests/test_local.py | 1004 ------------------------------------------- 1 file changed, 1004 deletions(-) delete mode 100644 tests/test_local.py diff --git a/tests/test_local.py b/tests/test_local.py deleted file mode 100644 index 7f368d0..0000000 --- a/tests/test_local.py +++ /dev/null @@ -1,1004 +0,0 @@ -""" -Python test file only to be run locally, when POTCARs are available and the .pmgrc.yaml file is -set up. This cannot be run on GitHub actions as it does not have the POTCARs, preventing POTCAR -and INCAR files from being written. -""" - -import copy -import json -import os -import shutil -import unittest -import warnings -from unittest.mock import patch - -import numpy as np - -# Click -from click.testing import CliRunner -from doped import vasp_input -from matplotlib.testing.compare import compare_images -from monty.serialization import dumpfn, loadfn -from pymatgen.analysis.defects.core import StructureMatcher -from pymatgen.core.structure import Structure -from pymatgen.io.vasp.inputs import Incar, Kpoints, Poscar, UnknownPotcarWarning - -from shakenbreak import cli, input, vasp -from shakenbreak.cli import snb - -_file_path = os.path.dirname(__file__) -_DATA_DIR = os.path.join(_file_path, "data") - - -def if_present_rm(path): - if os.path.exists(path): - if os.path.isfile(path): - os.remove(path) - elif os.path.isdir(path): - shutil.rmtree(path) - - -def _update_struct_defect_dict( - defect_dict: dict, structure: Structure, poscar_comment: str -) -> dict: - """ - Given a Structure object and POSCAR comment, update the folders dictionary - (generated with `doped.vasp_input.prepare_vasp_defect_inputs()`) with - the given values. - Args: - defect_dict (:obj:`dict`): - Dictionary with defect information, as generated with doped - `prepare_vasp_defect_inputs()` - structure (:obj:`~pymatgen.core.structure.Structure`): - Defect structure as a pymatgen object - poscar_comment (:obj:`str`): - Comment to include in the top line of the POSCAR file - Returns: - single defect dict in the `doped` format. - """ - defect_dict_copy = copy.deepcopy(defect_dict) - defect_dict_copy["Defect Structure"] = structure - defect_dict_copy["POSCAR Comment"] = poscar_comment - return defect_dict_copy - - -class DistortionLocalTestCase(unittest.TestCase): - """Test ShakeNBreak structure distortion helper functions""" - - def setUp(self): - warnings.filterwarnings("ignore", category=UnknownPotcarWarning) - self.DATA_DIR = os.path.join(os.path.dirname(__file__), "data") - self.VASP_CDTE_DATA_DIR = os.path.join(self.DATA_DIR, "vasp/CdTe") - self.EXAMPLE_RESULTS = os.path.join(self.DATA_DIR, "example_results") - - # Refactor doped defect dict to dict of Defect() objects - self.cdte_doped_defect_dict = loadfn( - os.path.join(self.VASP_CDTE_DATA_DIR, "CdTe_defects_dict.json") - ) - self.cdte_defects = { - defect_dict["name"]: input.generate_defect_object( - single_defect_dict=defect_dict, - bulk_dict=self.cdte_doped_defect_dict["bulk"], - ) - for defects_type, defect_dict_list in self.cdte_doped_defect_dict.items() - if "bulk" not in defects_type - for defect_dict in defect_dict_list - } # with doped/PyCDT names - - self.V_Cd_dict = self.cdte_doped_defect_dict["vacancies"][0] - self.Int_Cd_2_dict = self.cdte_doped_defect_dict["interstitials"][1] - # Refactor to Defect() objects - self.V_Cd = input.generate_defect_object( - self.V_Cd_dict, self.cdte_doped_defect_dict["bulk"] - ) - self.Int_Cd_2 = input.generate_defect_object( - self.Int_Cd_2_dict, self.cdte_doped_defect_dict["bulk"] - ) - - self.V_Cd_struc = Structure.from_file( - os.path.join(self.VASP_CDTE_DATA_DIR, "CdTe_V_Cd_POSCAR") - ) - self.V_Cd_minus0pt5_struc_rattled = Structure.from_file( - os.path.join( - self.VASP_CDTE_DATA_DIR, "CdTe_V_Cd_-50%_Distortion_Rattled_POSCAR" - ) - ) - self.V_Cd_minus0pt5_struc_0pt1_rattled = Structure.from_file( - os.path.join( - self.VASP_CDTE_DATA_DIR, - "CdTe_V_Cd_-50%_Distortion_stdev0pt1_Rattled_POSCAR", - ) - ) - self.V_Cd_minus0pt5_struc_kwarged = Structure.from_file( - os.path.join(self.VASP_CDTE_DATA_DIR, "CdTe_V_Cd_-50%_Kwarged_POSCAR") - ) - self.V_Cd_distortion_parameters = { - "unique_site": np.array([0.0, 0.0, 0.0]), - "num_distorted_neighbours": 2, - "distorted_atoms": [(33, "Te"), (42, "Te")], - } - self.Int_Cd_2_struc = Structure.from_file( - os.path.join(self.VASP_CDTE_DATA_DIR, "CdTe_Int_Cd_2_POSCAR") - ) - self.Int_Cd_2_minus0pt6_struc_rattled = Structure.from_file( - os.path.join( - self.VASP_CDTE_DATA_DIR, "CdTe_Int_Cd_2_-60%_Distortion_Rattled_POSCAR" - ) - ) - self.Int_Cd_2_minus0pt6_NN_10_struc_rattled = Structure.from_file( - os.path.join( - self.VASP_CDTE_DATA_DIR, "CdTe_Int_Cd_2_-60%_Distortion_NN_10_POSCAR" - ) - ) - self.Int_Cd_2_normal_distortion_parameters = { - "unique_site": self.Int_Cd_2_dict["unique_site"].frac_coords, - "num_distorted_neighbours": 2, - "distorted_atoms": [(10, "Cd"), (22, "Cd")], - "defect_site_index": 65, - } - self.Int_Cd_2_NN_10_distortion_parameters = { - "unique_site": self.Int_Cd_2_dict["unique_site"].frac_coords, - "num_distorted_neighbours": 10, - "distorted_atoms": [ - (10, "Cd"), - (22, "Cd"), - (29, "Cd"), - (1, "Cd"), - (14, "Cd"), - (24, "Cd"), - (30, "Cd"), - (38, "Te"), - (54, "Te"), - (62, "Te"), - ], - "defect_site_index": 65, - } - - # Note that Int_Cd_2 has been chosen as a test case, because the first few nonzero bond - # distances are the interstitial bonds, rather than the bulk bond length, so here we are - # also testing that the package correctly ignores these and uses the bulk bond length of - # 2.8333... for d_min in the structure rattling functions. - - self.cdte_defect_folders = [ - "as_1_Cd_on_Te_-1", - "as_1_Cd_on_Te_-2", - "as_1_Cd_on_Te_0", - "as_1_Cd_on_Te_1", - "as_1_Cd_on_Te_2", - "as_1_Cd_on_Te_3", - "as_1_Cd_on_Te_4", - "as_1_Te_on_Cd_-1", - "as_1_Te_on_Cd_-2", - "as_1_Te_on_Cd_0", - "as_1_Te_on_Cd_1", - "as_1_Te_on_Cd_2", - "as_1_Te_on_Cd_3", - "as_1_Te_on_Cd_4", - "Int_Cd_1_0", - "Int_Cd_1_1", - "Int_Cd_1_2", - "Int_Cd_2_0", - "Int_Cd_2_1", - "Int_Cd_2_2", - "Int_Cd_3_0", - "Int_Cd_3_1", - "Int_Cd_3_2", - "Int_Te_1_-1", - "Int_Te_1_-2", - "Int_Te_1_0", - "Int_Te_1_1", - "Int_Te_1_2", - "Int_Te_1_3", - "Int_Te_1_4", - "Int_Te_1_5", - "Int_Te_1_6", - "Int_Te_2_-1", - "Int_Te_2_-2", - "Int_Te_2_0", - "Int_Te_2_1", - "Int_Te_2_2", - "Int_Te_2_3", - "Int_Te_2_4", - "Int_Te_2_5", - "Int_Te_2_6", - "Int_Te_3_-1", - "Int_Te_3_-2", - "Int_Te_3_0", - "Int_Te_3_1", - "Int_Te_3_2", - "Int_Te_3_3", - "Int_Te_3_4", - "Int_Te_3_5", - "Int_Te_3_6", - "vac_1_Cd_-1", - "vac_1_Cd_-2", - "vac_1_Cd_0", - "vac_1_Cd_1", - "vac_1_Cd_2", - "vac_2_Te_-1", - "vac_2_Te_-2", - "vac_2_Te_0", - "vac_2_Te_1", - "vac_2_Te_2", - ] - - self.parsed_default_incar_settings = { - k: v for k, v in vasp.default_incar_settings.items() if "#" not in k - } # pymatgen doesn't parsed commented lines - self.parsed_incar_settings_wo_comments = { - k: v - for k, v in self.parsed_default_incar_settings.items() - if "#" not in str(v) - } # pymatgen ignores comments after values - - def tearDown(self) -> None: - for i in self.cdte_defect_folders: - if_present_rm(i) # remove test-generated vac_1_Cd_0 folder if present - if os.path.exists("distortion_metadata.json"): - os.remove("distortion_metadata.json") - - for i in [ - "parsed_defects_dict.json", - "distortion_metadata.json", - "test_config.yml", - ]: - if_present_rm(i) - - for i in os.listdir("."): - if "distortion_metadata" in i: - os.remove(i) - if ".png" in i: - os.remove(i) - elif ( - "Vac_Cd" in i - or "v_Cd" in i - or "vac_1_Cd" in i - or "Int_Cd" in i - or "Wally_McDoodle" in i - or "pesky_defects" in i - ): - shutil.rmtree(i) - - for defect_folder in [ - dir for dir in os.listdir(self.EXAMPLE_RESULTS) - if os.path.isdir(f"{self.EXAMPLE_RESULTS}/{dir}") - ]: - for file in os.listdir(f"{self.EXAMPLE_RESULTS}/{defect_folder}"): - if file.endswith(".png"): - os.remove(f"{self.EXAMPLE_RESULTS}/{defect_folder}/{file}") - - # test create_folder and create_vasp_input simultaneously: - def test_create_vasp_input(self): - """Test create_vasp_input function for INCARs and POTCARs""" - vasp_defect_inputs = vasp_input.prepare_vasp_defect_inputs( - copy.deepcopy(self.cdte_doped_defect_dict) - ) - V_Cd_updated_charged_defect_dict = _update_struct_defect_dict( - vasp_defect_inputs["vac_1_Cd_0"], - self.V_Cd_minus0pt5_struc_rattled, - "V_Cd Rattled", - ) - # make unperturbed defect entry: - V_Cd_unperturbed_dict = _update_struct_defect_dict( - vasp_defect_inputs["vac_1_Cd_0"], - self.V_Cd_struc, - "V_Cd Unperturbed", - ) - V_Cd_charged_defect_dict = { - "Unperturbed": V_Cd_unperturbed_dict, - "Bond_Distortion_-50.0%": V_Cd_updated_charged_defect_dict - } - self.assertFalse(os.path.exists("vac_1_Cd_0")) - input._create_vasp_input( - "vac_1_Cd_0", - distorted_defect_dict=V_Cd_charged_defect_dict, - user_incar_settings=vasp.default_incar_settings, - ) - V_Cd_minus50_folder = "vac_1_Cd_0/Bond_Distortion_-50.0%" - self.assertTrue(os.path.exists(V_Cd_minus50_folder)) - V_Cd_POSCAR = Poscar.from_file(V_Cd_minus50_folder + "/POSCAR") - self.assertEqual(V_Cd_POSCAR.comment, "V_Cd Rattled") - self.assertEqual(V_Cd_POSCAR.structure, self.V_Cd_minus0pt5_struc_rattled) - - V_Cd_INCAR = Incar.from_file(V_Cd_minus50_folder + "/INCAR") - # check if default INCAR is subset of INCAR: - self.assertTrue( - self.parsed_incar_settings_wo_comments.items() <= V_Cd_INCAR.items() - ) - - V_Cd_KPOINTS = Kpoints.from_file(V_Cd_minus50_folder + "/KPOINTS") - self.assertEqual(V_Cd_KPOINTS.kpts, [[1, 1, 1]]) - - # check if POTCARs have been written: - self.assertTrue(os.path.isfile(V_Cd_minus50_folder + "/POTCAR")) - - # test with kwargs: (except POTCAR settings because we can't have this on the GitHub test - # server) - kwarg_incar_settings = { - "NELECT": 3, - "IBRION": 42, - "LVHAR": True, - "LWAVE": True, - "LCHARG": True, - "ENCUT": 200, - } - kwarged_incar_settings = self.parsed_incar_settings_wo_comments.copy() - kwarged_incar_settings.update(kwarg_incar_settings) - input._create_vasp_input( - "vac_1_Cd_0", - distorted_defect_dict=V_Cd_charged_defect_dict, - user_incar_settings=kwarged_incar_settings, - ) - V_Cd_kwarg_minus50_folder = "vac_1_Cd_0/Bond_Distortion_-50.0%" - self.assertTrue(os.path.exists(V_Cd_kwarg_minus50_folder)) - V_Cd_POSCAR = Poscar.from_file(V_Cd_kwarg_minus50_folder + "/POSCAR") - self.assertEqual(V_Cd_POSCAR.comment, "V_Cd Rattled") - self.assertEqual(V_Cd_POSCAR.structure, self.V_Cd_minus0pt5_struc_rattled) - - V_Cd_INCAR = Incar.from_file(V_Cd_kwarg_minus50_folder + "/INCAR") - # check if default INCAR is subset of INCAR: - self.assertFalse( - self.parsed_incar_settings_wo_comments.items() <= V_Cd_INCAR.items() - ) - self.assertTrue(kwarged_incar_settings.items() <= V_Cd_INCAR.items()) - - V_Cd_KPOINTS = Kpoints.from_file(V_Cd_kwarg_minus50_folder + "/KPOINTS") - self.assertEqual(V_Cd_KPOINTS.kpts, [[1, 1, 1]]) - - # check if POTCARs have been written: - self.assertTrue(os.path.isfile(V_Cd_kwarg_minus50_folder + "/POTCAR")) - - @patch("builtins.print") - def test_write_vasp_files(self, mock_print): - """Test write_vasp_files method""" - oxidation_states = {"Cd": +2, "Te": -2} - bond_distortions = list(np.arange(-0.6, 0.601, 0.05)) - - dist = input.Distortions( - self.cdte_defects, - oxidation_states=oxidation_states, - bond_distortions=bond_distortions, - local_rattle=False, - ) - distorted_defect_dict, _ = dist.write_vasp_files( - user_incar_settings={"ENCUT": 212, "IBRION": 0, "EDIFF": 1e-4}, - verbose=False, - ) - - # check if expected folders were created: - self.assertTrue(set(self.cdte_defect_folders).issubset(set(os.listdir()))) - # check expected info printing: - mock_print.assert_any_call( - "Applying ShakeNBreak...", - "Will apply the following bond distortions:", - "['-0.6', '-0.55', '-0.5', '-0.45', '-0.4', '-0.35', '-0.3', " - "'-0.25', '-0.2', '-0.15', '-0.1', '-0.05', '0.0', '0.05', " - "'0.1', '0.15', '0.2', '0.25', '0.3', '0.35', '0.4', '0.45', " - "'0.5', '0.55', '0.6'].", - "Then, will rattle with a std dev of 0.28 Å \n", - ) - mock_print.assert_any_call( - "\033[1m" + "\nDefect: vac_1_Cd" + "\033[0m" - ) # bold print - mock_print.assert_any_call( - "\033[1m" + "Number of missing electrons in neutral state: 2" + "\033[0m" - ) - mock_print.assert_any_call( - "\nDefect vac_1_Cd in charge state: -2. Number of distorted " - "neighbours: 0" - ) - mock_print.assert_any_call( - "\nDefect vac_1_Cd in charge state: -1. Number of distorted " - "neighbours: 1" - ) - mock_print.assert_any_call( - "\nDefect vac_1_Cd in charge state: 0. Number of distorted " "neighbours: 2" - ) - # test correct distorted neighbours based on oxidation states: - mock_print.assert_any_call( - "\nDefect vac_2_Te in charge state: -2. Number of distorted " - "neighbours: 4" - ) - mock_print.assert_any_call( - "\nDefect as_1_Cd_on_Te in charge state: -2. Number of " - "distorted neighbours: 2" - ) - mock_print.assert_any_call( - "\nDefect as_1_Te_on_Cd in charge state: -2. Number of " - "distorted neighbours: 2" - ) - mock_print.assert_any_call( - "\nDefect Int_Cd_1 in charge state: 0. Number of distorted " "neighbours: 2" - ) - mock_print.assert_any_call( - "\nDefect Int_Te_1 in charge state: -2. Number of distorted " - "neighbours: 0" - ) - - # check if correct files were created: - V_Cd_minus50_folder = "vac_1_Cd_0/Bond_Distortion_-50.0%" - self.assertTrue(os.path.exists(V_Cd_minus50_folder)) - V_Cd_POSCAR = Poscar.from_file(V_Cd_minus50_folder + "/POSCAR") - self.assertEqual( - V_Cd_POSCAR.comment, - "-50.0%__num_neighbours=2__vac_1_Cd", - ) # default - V_Cd_POSCAR.structure.remove_oxidation_states() - self.assertNotEqual(V_Cd_POSCAR.structure, self.V_Cd_minus0pt5_struc_rattled) - # V_Cd_minus0pt5_struc_rattled was with old default seed = 42 and stdev = 0.25 - - # Check INCAR - V_Cd_INCAR = Incar.from_file(V_Cd_minus50_folder + "/INCAR") - # check if default INCAR is subset of INCAR: (not here because we set ENCUT) - self.assertFalse( - self.parsed_incar_settings_wo_comments.items() <= V_Cd_INCAR.items() - ) - self.assertEqual(V_Cd_INCAR.pop("ENCUT"), 212) - self.assertEqual(V_Cd_INCAR.pop("IBRION"), 0) - self.assertEqual(V_Cd_INCAR.pop("EDIFF"), 1e-4) - self.assertEqual(V_Cd_INCAR.pop("ROPT"), "1e-3 1e-3") - parsed_settings = self.parsed_incar_settings_wo_comments.copy() - parsed_settings.pop("ENCUT") - self.assertTrue( - parsed_settings.items() - <= V_Cd_INCAR.items() # matches after - # removing kwarg settings - ) - # Check KPOINTS - V_Cd_KPOINTS = Kpoints.from_file(V_Cd_minus50_folder + "/KPOINTS") - self.assertEqual(V_Cd_KPOINTS.kpts, [[1, 1, 1]]) - - # check if POTCARs have been written: - self.assertTrue(os.path.isfile(V_Cd_minus50_folder + "/POTCAR")) - - # Check POSCARs - Int_Cd_2_minus60_folder = "Int_Cd_2_0/Bond_Distortion_-60.0%" - self.assertTrue(os.path.exists(Int_Cd_2_minus60_folder)) - Int_Cd_2_POSCAR = Poscar.from_file(Int_Cd_2_minus60_folder + "/POSCAR") - self.assertEqual( - Int_Cd_2_POSCAR.comment, - "-60.0%__num_neighbours=2__Int_Cd_2", - ) - struc = Int_Cd_2_POSCAR.structure - struc.remove_oxidation_states() - self.assertEqual(struc, self.Int_Cd_2_minus0pt6_struc_rattled) - - # check INCAR - V_Cd_INCAR = Incar.from_file(V_Cd_minus50_folder + "/INCAR") - Int_Cd_2_INCAR = Incar.from_file(Int_Cd_2_minus60_folder + "/INCAR") - # neutral even-electron INCARs the same except for NELECT: - for incar in [V_Cd_INCAR, Int_Cd_2_INCAR]: - incar.pop("NELECT") # https://tenor.com/bgVv9.gif - self.assertEqual(V_Cd_INCAR, Int_Cd_2_INCAR) - # Kpoints - Int_Cd_2_KPOINTS = Kpoints.from_file(Int_Cd_2_minus60_folder + "/KPOINTS") - self.assertEqual(Int_Cd_2_KPOINTS.kpts, [[1, 1, 1]]) - # check if POTCARs have been written: - self.assertTrue(os.path.isfile(Int_Cd_2_minus60_folder + "/POTCAR")) - - def test_plot(self): - """ - Test plot() function. - The plots used for comparison have been generated with the Montserrat font - (available in the fonts directory). - """ - # Test the following options: - # --defect, --path, --format, --units, --colorbar, --metric, --no_title, --verbose - defect = "v_Ti_0" - dumpfn( - { - "distortions": {-0.4: -1176.28458753}, - "Unperturbed": -1173.02056574, - }, - f"{self.EXAMPLE_RESULTS}/{defect}/{defect}.yaml", - ) - if os.path.exists(f"{self.EXAMPLE_RESULTS}/distortion_metadata.json"): - os.remove(f"{self.EXAMPLE_RESULTS}/distortion_metadata.json") - runner = CliRunner() - with warnings.catch_warnings(record=True) as w: - result = runner.invoke( - snb, - [ - "plot", - "-d", - defect, - "-p", - self.EXAMPLE_RESULTS, - "--units", - "meV", - "--format", - "png", - "--colorbar", - "--metric", - "disp", - "-nt", # No title - "-v", - ], - catch_exceptions=False, - ) - self.assertTrue( - os.path.exists(os.path.join(self.EXAMPLE_RESULTS, f"{defect}/{defect}.png")) - ) - compare_images( - os.path.join(self.EXAMPLE_RESULTS, f"{defect}/{defect}.png"), - f"{_DATA_DIR}/local_baseline_plots/vac_1_Ti_0_cli_colorbar_disp.png", - tol=2.0, - ) # only locally (on Github Actions, saved image has a different size) - self.tearDown() - [ - os.remove(os.path.join(self.EXAMPLE_RESULTS, defect, file)) - for file in os.listdir(os.path.join(self.EXAMPLE_RESULTS, defect)) - if "yaml" in file or "png" in file - ] - - # Test --all option, with the distortion_metadata.json file present to parse number of - # distorted neighbours and their identities - fake_distortion_metadata = { - "defects": { - "v_Cd": { - "charges": { - "0": { - "num_nearest_neighbours": 2, - "distorted_atoms": [[33, "Te"], [42, "Te"]], - }, - "-1": { - "num_nearest_neighbours": 1, - "distorted_atoms": [ - [33, "Te"], - ], - }, - } - }, - "v_Ti": { - "charges": { - "0": { - "num_nearest_neighbours": 3, - "distorted_atoms": [[33, "O"], [42, "O"], [40, "O"]], - }, - } - }, - } - } - with open(f"{self.EXAMPLE_RESULTS}/distortion_metadata.json", "w") as f: - f.write(json.dumps(fake_distortion_metadata, indent=4)) - result = runner.invoke( - snb, - [ - "plot", - "--all", - "-p", - self.EXAMPLE_RESULTS, - "-f", - "png", - ], - catch_exceptions=False, - ) - self.assertTrue( - os.path.exists(os.path.join(self.EXAMPLE_RESULTS, f"{defect}/{defect}.png")) - ) - self.assertTrue( - os.path.exists(os.path.join(self.EXAMPLE_RESULTS, "v_Cd_0/v_Cd_0.png")) - ) - self.assertTrue( - os.path.exists(os.path.join(self.EXAMPLE_RESULTS, "v_Cd_-1/v_Cd_-1.png")) - ) - compare_images( - os.path.join(self.EXAMPLE_RESULTS, "v_Cd_0/v_Cd_0.png"), - f"{_DATA_DIR}/local_baseline_plots/vac_1_Cd_0_cli_default.png", - tol=2.0, - ) # only locally (on Github Actions, saved image has a different size) - [ - os.remove(os.path.join(self.EXAMPLE_RESULTS, defect, file)) - for file in os.listdir(os.path.join(self.EXAMPLE_RESULTS, defect)) - if "yaml" in file or "png" in file - ] - - # generate docs example plots: - shutil.copytree( - f"{self.EXAMPLE_RESULTS}/v_Cd_0", f"{self.EXAMPLE_RESULTS}/orig_v_Cd_0" - ) - for i in range(1,7): - shutil.copyfile( - f"{self.EXAMPLE_RESULTS}/v_Cd_0/Unperturbed/CONTCAR", - f"{self.EXAMPLE_RESULTS}/v_Cd_0/Bond_Distortion_{i}0.0%/CONTCAR", - ) - energies_dict = loadfn(f"{self.EXAMPLE_RESULTS}/v_Cd_0/v_Cd_0.yaml") - energies_dict["distortions"][-0.5] = energies_dict["distortions"][-0.6] - dumpfn(energies_dict, f"{self.EXAMPLE_RESULTS}/v_Cd_0/v_Cd_0.yaml") - shutil.copyfile( - f"{self.EXAMPLE_RESULTS}/v_Cd_0/Bond_Distortion_-60.0%/CONTCAR", - f"{self.EXAMPLE_RESULTS}/v_Cd_0/Bond_Distortion_-50.0%/CONTCAR", - ) - - result = runner.invoke( - snb, - [ - "plot", - "-d", - "v_Cd_0", - "-cb", - "-p", - self.EXAMPLE_RESULTS, - "-f", - "svg", - ], - ) - shutil.copyfile(f"{self.EXAMPLE_RESULTS}/v_Cd_0/v_Cd_0.svg", - "../docs/v_Cd_0_colorbar.svg") - result = runner.invoke( - snb, - [ - "plot", - "-d", - "v_Cd_0", - "-p", - self.EXAMPLE_RESULTS, - "-f", - "svg", - ], - ) - shutil.copyfile(f"{self.EXAMPLE_RESULTS}/v_Cd_0/v_Cd_0.svg", "../docs/v_Cd_0.svg") - shutil.rmtree(f"{self.EXAMPLE_RESULTS}/v_Cd_0") - shutil.move( - f"{self.EXAMPLE_RESULTS}/orig_v_Cd_0", f"{self.EXAMPLE_RESULTS}/v_Cd_0" - ) - os.remove(f"{self.EXAMPLE_RESULTS}/distortion_metadata.json") - self.tearDown() - - def test_generate_all_input_file(self): - """Test generate_all() function when user specifies input_file""" - defects_dir = f"pesky_defects" - defect_name = "vac_1_Cd" - os.mkdir(defects_dir) - os.mkdir(f"{defects_dir}/{defect_name}") # non-standard defect name - shutil.copyfile( - f"{self.VASP_CDTE_DATA_DIR}/CdTe_V_Cd_POSCAR", - f"{defects_dir}/{defect_name}/POSCAR", - ) - test_yml = f""" - defects: - {defect_name}: - charges: [0,] - defect_coords: [0.0, 0.0, 0.0] - bond_distortions: [0.3,] - POTCAR: - Cd: Cd_GW - """ - with open("test_config.yml", "w") as fp: - fp.write(test_yml) - - # Test VASP - with open("INCAR", "w") as fp: - fp.write("IBRION = 1 \n GGA = PS") - runner = CliRunner() - result = runner.invoke( - snb, - [ - "generate_all", - "-d", - f"{defects_dir}/", - "-b", - f"{self.VASP_CDTE_DATA_DIR}/CdTe_Bulk_Supercell_POSCAR", - "--code", - "vasp", - "--input_file", - "INCAR", - "--config", - "test_config.yml", - ], - catch_exceptions=True, - ) - dist = "Unperturbed" - incar_dict = Incar.from_file(f"{defect_name}_0/{dist}/INCAR").as_dict() - self.assertEqual(incar_dict["GGA"].lower(), "PS".lower()) - self.assertEqual(incar_dict["IBRION"], 1) - for file in ["KPOINTS", "POTCAR", "POSCAR"]: - self.assertTrue(os.path.exists(f"{defect_name}_0/{dist}/{file}")) - # Check POTCAR generation - with open(f"{defect_name}_0/{dist}/POTCAR") as myfile: - first_line = myfile.readline() - self.assertIn("PAW_PBE Cd_GW", first_line) - - shutil.rmtree(f"{defect_name}_0") - os.remove("INCAR") - - # test warning when input file doesn't match expected format: - os.remove("distortion_metadata.json") - with warnings.catch_warnings(record=True) as w: - result = runner.invoke( - snb, - [ - "generate_all", - "-d", - f"{defects_dir}/", - "-b", - f"{self.VASP_CDTE_DATA_DIR}/CdTe_Bulk_Supercell_POSCAR", - "--code", - "vasp", - "--input_file", - "test_config.yml", - "--config", - "test_config.yml", - ], - catch_exceptions=True, - ) - dist = "Unperturbed" - incar_dict = Incar.from_file(f"{defect_name}_0/{dist}/INCAR").as_dict() - self.assertEqual(incar_dict["IBRION"], 2) # default setting - # assert UserWarning about unparsed input file - user_warnings = [warning for warning in w if warning.category == UserWarning] - self.assertEqual(len(user_warnings), 1) - self.assertEqual( - "Input file test_config.yml specified but no valid INCAR tags found. " - "Should be in the format of VASP INCAR file.", - str(user_warnings[-1].message), - ) - for file in ["KPOINTS", "POTCAR", "POSCAR"]: - self.assertTrue(os.path.exists(f"{defect_name}_0/{dist}/{file}")) - shutil.rmtree(f"{defect_name}_0") - - # Test CASTEP - with open("castep.param", "w") as fp: - fp.write("XC_FUNCTIONAL: PBE \n MAX_SCF_CYCLES: 100 \n CHARGE: 0") - runner = CliRunner() - result = runner.invoke( - snb, - [ - "generate_all", - "-d", - f"{defects_dir}/", - "-b", - f"{self.VASP_CDTE_DATA_DIR}/CdTe_Bulk_Supercell_POSCAR", - "--code", - "castep", - "--input_file", - "castep.param", - "--config", - "test_config.yml", - ], - catch_exceptions=True, - ) - dist = "Unperturbed" - with open(f"{defect_name}_0/{dist}/castep.param") as fp: - castep_lines = [line.strip() for line in fp.readlines()[-3:]] - self.assertEqual( - ["XC_FUNCTIONAL: PBE", "MAX_SCF_CYCLES: 100", "CHARGE: 0"], castep_lines - ) - shutil.rmtree(f"{defect_name}_0") - os.remove("castep.param") - - # Test CP2K - runner = CliRunner() - result = runner.invoke( - snb, - [ - "generate_all", - "-d", - f"{defects_dir}/", - "-b", - f"{self.VASP_CDTE_DATA_DIR}/CdTe_Bulk_Supercell_POSCAR", - "--code", - "cp2k", - "--input_file", - f"{self.DATA_DIR}/cp2k/cp2k_input_mod.inp", - "--config", - "test_config.yml", - ], - catch_exceptions=False, - ) - dist = "Unperturbed" - self.assertTrue(os.path.exists(f"{defect_name}_0/{dist}")) - with open(f"{defect_name}_0/{dist}/cp2k_input.inp") as fp: - input_cp2k = fp.readlines() - self.assertEqual( - "CUTOFF [eV] 800 ! PW cutoff", - input_cp2k[15].strip(), - ) - shutil.rmtree(f"{defect_name}_0") - - # Test Quantum Espresso - test_yml = f""" - defects: - {defect_name}: - charges: [0,] - defect_coords: [0.0, 0.0, 0.0] - bond_distortions: [0.3,] - pseudopotentials: - 'Cd': 'Cd_pbe_v1.uspp.F.UPF' - 'Te': 'Te.pbe-n-rrkjus_psl.1.0.0.UPF' - """ - with open("test_config.yml", "w") as fp: - fp.write(test_yml) - runner = CliRunner() - result = runner.invoke( - snb, - [ - "generate_all", - "-d", - f"{defects_dir}/", - "-b", - f"{self.VASP_CDTE_DATA_DIR}/CdTe_Bulk_Supercell_POSCAR", - "--code", - "espresso", - "--input_file", - f"{self.DATA_DIR}/quantum_espresso/qe.in", - "--config", - "test_config.yml", - ], - catch_exceptions=False, - ) - dist = "Unperturbed" - with open(f"{defect_name}_0/{dist}/espresso.pwi") as fp: - input_qe = fp.readlines() - self.assertEqual( - "title = 'Si bulk'", - input_qe[2].strip(), - ) - shutil.rmtree(f"{defect_name}_0") - - # Test FHI-aims - runner = CliRunner() - result = runner.invoke( - snb, - [ - "generate_all", - "-d", - f"{defects_dir}/", - "-b", - f"{self.VASP_CDTE_DATA_DIR}/CdTe_Bulk_Supercell_POSCAR", - "--code", - "fhiaims", - "--input_file", - f"{self.DATA_DIR}/fhi_aims/control.in", - "--config", - "test_config.yml", - ], - catch_exceptions=False, - ) - dist = "Unperturbed" - with open(f"{defect_name}_0/{dist}/control.in") as fp: - input_aims = fp.readlines() - self.assertEqual( - "xc pbe", - input_aims[6].strip(), - ) - self.assertEqual( - "sc_iter_limit 100.0", - input_aims[10].strip(), - ) - shutil.rmtree(f"{defect_name}_0") - self.tearDown() - - def test_generate(self): - "Test generate command" - - test_yml = """ -bond_distortions: [-0.5,] -stdev: 0.15 -d_min: 2.1250262890187375 # 0.75 * 2.8333683853583165 -nbr_cutoff: 3.4 -n_iter: 3 -active_atoms: [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30] # np.arange(0,31) -width: 0.3 -max_attempts: 10000 -max_disp: 1.0 -seed: 20 -local_rattle: False -POTCAR: - Cd: Cd_GW -""" - with open("test_config.yml", "w+") as fp: - fp.write(test_yml) - defect_name = "v_Cd" # SnB default name - runner = CliRunner() - result = runner.invoke( - snb, - [ - "generate", - "-d", - f"{self.VASP_CDTE_DATA_DIR}/CdTe_V_Cd_POSCAR", - "-b", - f"{self.VASP_CDTE_DATA_DIR}/CdTe_Bulk_Supercell_POSCAR", - "-c 0", - "-v", - "--config", - f"test_config.yml", - ], - catch_exceptions=False, - ) - self.assertEqual(result.exit_code, 0) - self.assertTrue(os.path.exists(f"./{defect_name}_0")) - self.assertTrue(os.path.exists(f"./{defect_name}_0/Bond_Distortion_-50.0%")) - V_Cd_kwarged_POSCAR = Poscar.from_file( - f"./{defect_name}_0/Bond_Distortion_-50.0%/POSCAR" - ) - self.assertEqual( - V_Cd_kwarged_POSCAR.structure, self.V_Cd_minus0pt5_struc_kwarged - ) - for file in ["KPOINTS", "POTCAR", "INCAR"]: - self.assertTrue( - os.path.exists(f"{defect_name}_0/Bond_Distortion_-50.0%/{file}") - ) - # Check POTCAR file - with open(f"{defect_name}_0/Bond_Distortion_-50.0%/POTCAR") as myfile: - first_line = myfile.readline() - self.assertIn("PAW_PBE Cd_GW", first_line) - # Check KPOINTS file - kpoints = Kpoints.from_file( - f"{defect_name}_0/Bond_Distortion_-50.0%/" + "KPOINTS" - ) - self.assertEqual(kpoints.kpts, [[1, 1, 1]]) - # Check INCAR - incar = Incar.from_file(f"{defect_name}_0/Bond_Distortion_-50.0%/" + "INCAR") - self.assertEqual(incar.pop("IBRION"), 2) - self.assertEqual(incar.pop("EDIFF"), 1e-5) - self.assertEqual(incar.pop("ROPT"), "1e-3 1e-3") - - # Test custom name - defect_name = "vac_1_Cd" - result = runner.invoke( - snb, - [ - "generate", - "-d", - f"{self.VASP_CDTE_DATA_DIR}/CdTe_V_Cd_POSCAR", - "-b", - f"{self.VASP_CDTE_DATA_DIR}/CdTe_Bulk_Supercell_POSCAR", - "-c 0", - "-n", - "vac_1_Cd", - "--config", - f"test_config.yml", - ], - catch_exceptions=False, - ) - cwd = os.getcwd() - self.assertEqual(result.exit_code, 0) - # self.assertTrue(os.path.exists(f"{cwd}/vac_1_Cd_0")) - self.assertTrue(os.path.exists(f"{cwd}/vac_1_Cd_0/Bond_Distortion_-50.0%")) - - # test warning when input file doesn't match expected format: - os.remove("distortion_metadata.json") - with warnings.catch_warnings(record=True) as w: - result = runner.invoke( - snb, - [ - "generate", - "-d", - f"{self.VASP_CDTE_DATA_DIR}/CdTe_V_Cd_POSCAR", - "-b", - f"{self.VASP_CDTE_DATA_DIR}/CdTe_Bulk_Supercell_POSCAR", - "-c 0", - "-v", - "--input_file", - f"test_config.yml", - ], - catch_exceptions=False, - ) - incar_dict = Incar.from_file( - f"{defect_name}_0/Bond_Distortion_-50.0%/INCAR" - ).as_dict() - self.assertEqual(incar_dict["IBRION"], 2) # default setting - # assert UserWarning about unparsed input file - user_warnings = [warning for warning in w if warning.category == UserWarning] - self.assertEqual(len(user_warnings), 2) # wrong INCAR format and overwriting folder - self.assertTrue( - any("Input file test_config.yml specified but no valid INCAR tags found. " - "Should be in the format of VASP INCAR file." - in str(warning.message) for warning in user_warnings) - ) - self.assertTrue( # here we get this warning because no Unperturbed structures were - # written so couldn't be compared - any(f"The previously-generated defect folder v_Cd_0 in " - f"{os.path.basename(os.path.abspath('.'))} has the same Unperturbed defect structure " - f"as the current defect species: v_Cd_0. ShakeNBreak files in v_Cd_0 will be " - f"overwritten." in str(warning.message) for warning in user_warnings) - ) - for file in ["KPOINTS", "POTCAR", "POSCAR"]: - self.assertTrue( - os.path.exists(f"{defect_name}_0/Bond_Distortion_-50.0%/{file}") - ) - - -if __name__ == "__main__": - unittest.main() From 538d6ebecafaf55359cc7c38c86377ce3633360d Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 16:43:24 +0100 Subject: [PATCH 08/28] Simplify GitHub Actions workflows --- .github/workflows/build_and_test.yml | 10 ++-------- .github/workflows/pip_install_test.yml | 8 +------- .github/workflows/release.yml | 8 +------- 3 files changed, 4 insertions(+), 22 deletions(-) diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 55f26df..9f5002e 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -43,14 +43,8 @@ jobs: pip show -V pytest - name: Test run: | - for test in tests/test_*py - do if [[ "$test" != *"test_local"* ]] - then pytest $test - fi - done # Ignore local tests file, which tests INCAR and POTCAR file writing but not possible on GitHub Actions - pytest --mpl tests/test_shakenbreak.py # test output plots - pytest --mpl tests/test_plotting.py # test output plots - # To generate the test plots on GA: + pytest --mpl tests/test_*py # test output plots + # To generate the test plots: # pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py # generate output plots # pytest --mpl-generate-path=tests/remote_baseline tests/test_shakenbreak.py # generate output plots diff --git a/.github/workflows/pip_install_test.yml b/.github/workflows/pip_install_test.yml index a52aea0..0fdf3cd 100644 --- a/.github/workflows/pip_install_test.yml +++ b/.github/workflows/pip_install_test.yml @@ -35,12 +35,6 @@ jobs: - name: Test run: | - for test in tests/test_*py - do if [[ "$test" != *"test_local"* ]] - then pytest $test - fi - done # Ignore local tests file, which tests INCAR and POTCAR file writing but not possible on GitHub Actions - pytest --mpl tests/test_shakenbreak.py # test output plots - pytest --mpl tests/test_plotting.py # test output plots + pytest --mpl tests/test_*.py # test output plots # pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py # generate output plots # pytest --mpl-generate-path=tests/remote_baseline tests/test_shakenbreak.py # generate output plots diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index f95e00a..f3d0448 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -28,13 +28,7 @@ jobs: - name: Test run: | - for test in tests/test_*py - do if [[ "$test" != *"test_local"* ]] - then pytest $test - fi - done # Ignore local tests file, which tests INCAR and POTCAR file writing but not possible on GitHub Actions - pytest --mpl tests/test_shakenbreak.py # test output plots - pytest --mpl tests/test_plotting.py # test output plots + pytest --mpl tests/test_*.py # test output plots - name: Build packages run: | From 3d66e2535da8d470f1ca5dd612af40baa8849c6b Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 17:20:27 +0100 Subject: [PATCH 09/28] Test POTCAR warnings --- shakenbreak/input.py | 38 ++++++---- tests/test_input.py | 169 ++++++++++++++++++++++--------------------- 2 files changed, 108 insertions(+), 99 deletions(-) diff --git a/shakenbreak/input.py b/shakenbreak/input.py index 48706a8..88d9f7b 100755 --- a/shakenbreak/input.py +++ b/shakenbreak/input.py @@ -19,6 +19,7 @@ from ase.calculators.aims import Aims from ase.calculators.castep import Castep from ase.calculators.espresso import Espresso +from doped import _ignore_pmg_warnings from doped.generation import ( DefectsGenerator, get_defect_name_from_entry, @@ -52,10 +53,7 @@ ) -warnings.filterwarnings( - "ignore", category=UnknownPotcarWarning -) # Ignore pymatgen POTCAR warnings -warnings.filterwarnings("ignore", message=".*Ignoring unknown variable type.*") +_ignore_pmg_warnings() # Ignore pymatgen POTCAR warnings def _warning_on_one_line( @@ -216,8 +214,7 @@ def _create_vasp_input( (Default is current directory = "./") **kwargs: Keyword arguments to pass to `DefectDictSet.write_input()` (e.g. - `potcar_spec`). If `potcars` in `kwargs`, then this is passed to - `DefectDictSet()`. Mainly for POTCAR testing on GH Actions. + `potcar_spec`). Returns: None @@ -357,7 +354,6 @@ def _create_vasp_input( user_potcar_functional=user_potcar_functional, user_potcar_settings=potcar_settings, poscar_comment=None, - potcars=kwargs.pop("potcars", None), ) for ( @@ -368,14 +364,26 @@ def _create_vasp_input( ): # for each distortion, create sub-subfolder folder dds._structure = single_defect_dict["Defect Structure"] dds.poscar_comment = single_defect_dict.get("POSCAR Comment", None) - dds.write_input(f"{output_path}/{defect_name}/{distortion}", **kwargs) - # TODO: Should be able to add tests with potcar_spec? By adding kwargs here? - # TODO: Edit output warning when user POTCARs not set up (and check this) - # warnings.warn( - # "POTCAR directory not set up with pymatgen, so only POSCAR files " - # "will be generated (POTCARs also needed to determine appropriate " - # "NELECT setting in INCAR files)" - # ) + + try: + dds._check_user_potcars(unperturbed_poscar=False) + dds.write_input(f"{output_path}/{defect_name}/{distortion}", **kwargs) + + except ValueError: + # POTCARs not set up, warn and write other files + warnings.warn( + "POTCAR directory not set up with pymatgen (see the doped docs Installation page: " + "https://doped.readthedocs.io/en/latest/Installation.html for instructions on setting " + "this up). This is required to generate `POTCAR` files and set `NELECT` (i.e. charge " + "state) and `NUPDOWN` in the `INCAR` files!\n" + "No `POTCAR` files will be written, and `NELECT` and `NUPDOWN` will not be set in " + "`INCAR`s. Beware!" + ) + + os.makedirs(f"{output_path}/{defect_name}/{distortion}", exist_ok=True) + dds.incar.write_file(f"{output_path}/{defect_name}/{distortion}/INCAR") + dds.poscar.write_file(f"{output_path}/{defect_name}/{distortion}/POSCAR") + dds.kpoints.write_file(f"{output_path}/{defect_name}/{distortion}/KPOINTS") def _get_bulk_comp(defect_object) -> Composition: diff --git a/tests/test_input.py b/tests/test_input.py index 7cd4762..9b545b1 100755 --- a/tests/test_input.py +++ b/tests/test_input.py @@ -10,6 +10,7 @@ import numpy as np from ase.build import bulk, make_supercell from ase.calculators.aims import Aims +from doped import _ignore_pmg_warnings from doped.vasp import _test_potcar_functional_choice from monty.serialization import dumpfn, loadfn from pymatgen.analysis.defects.generators import VacancyGenerator @@ -916,11 +917,12 @@ def test_create_vasp_input(self): "Bond_Distortion_-50.0%": V_Cd_updated_charged_defect_dict } self.assertFalse(os.path.exists("vac_1_Cd_0")) - input._create_vasp_input( - "vac_1_Cd_0", - distorted_defect_dict=V_Cd_charged_defect_dict, - potcars=_potcars_available(), # to allow testing on GH Actions - ) + with warnings.catch_warnings(record=True) as w: + _ignore_pmg_warnings() + input._create_vasp_input( + "vac_1_Cd_0", + distorted_defect_dict=V_Cd_charged_defect_dict, + ) V_Cd_POSCAR = self._check_V_Cd_rattled_poscar( "vac_1_Cd_0/Bond_Distortion_-50.0%" ) @@ -938,6 +940,19 @@ def test_create_vasp_input(self): input.default_potcar_dict["POTCAR"][el_symbol] for el_symbol in V_Cd_POSCAR.structure.symbol_set } + else: # test POTCAR warning + assert ( + len(w) == 2 + ) # general POTCAR warning and NELECT/NUPDOWN INCAR warning + assert any( + str(warning.message) + == "POTCAR directory not set up with pymatgen (see the doped docs Installation page: " + "https://doped.readthedocs.io/en/latest/Installation.html for instructions on setting " + "this up). This is required to generate `POTCAR` files and set `NELECT` (i.e. charge " + "state) and `NUPDOWN` in the `INCAR` files!\nNo `POTCAR` files will be written, and " + "`NELECT` and `NUPDOWN` will not be set in `INCAR`s. Beware!" + for warning in w + ) # test with kwargs: kwarg_incar_settings = { @@ -953,7 +968,6 @@ def test_create_vasp_input(self): distorted_defect_dict=V_Cd_charged_defect_dict, user_incar_settings=kwarg_incar_settings, user_potcar_settings={"Cd": "Cd_sv_GW", "Te": "Te_GW"}, - potcars=_potcars_available(), # to allow testing on GH Actions ) self._check_V_Cd_folder_renaming( w, @@ -983,8 +997,18 @@ def test_create_vasp_input(self): assert set(potcar.as_dict()["symbols"]) == { "Cd_sv", "Te_GW", - } # Cd_sv_GW POTCAR has Cd_sv - # symbol, checked + } # Cd_sv_GW POTCAR has Cd_sv symbol, checked + else: # test POTCAR warning + print([warning.message for warning in w]) + assert any( + str(warning.message) + == "POTCAR directory not set up with pymatgen (see the doped docs Installation page: " + "https://doped.readthedocs.io/en/latest/Installation.html for instructions on setting " + "this up). This is required to generate `POTCAR` files and set `NELECT` (i.e. charge " + "state) and `NUPDOWN` in the `INCAR` files!\nNo `POTCAR` files will be written, and " + "`NELECT` and `NUPDOWN` will not be set in `INCAR`s. Beware!" + for warning in w + ) # test output_path option input._create_vasp_input( @@ -992,7 +1016,6 @@ def test_create_vasp_input(self): distorted_defect_dict=V_Cd_charged_defect_dict, user_incar_settings=kwarg_incar_settings, output_path="test_path", - potcars=_potcars_available(), # to allow testing on GH Actions ) V_Cd_POSCAR = self._check_V_Cd_rattled_poscar( "test_path/vac_1_Cd_0/Bond_Distortion_-50.0%" @@ -1049,7 +1072,6 @@ def test_create_vasp_input(self): "vac_1_Cd_0", distorted_defect_dict=V_Cd_charged_defect_dict, user_incar_settings={}, - potcars=_potcars_available(), # to allow testing on GH Actions user_potcar_functional="PBE_54", # check setting POTCAR functional to one that isn't # present locally ) @@ -1058,6 +1080,17 @@ def test_create_vasp_input(self): "The previously-generated defect folder vac_1_Cdb_0 in ", " has the same Unperturbed defect structure as the current defect species: vac_1_Cd_0. ShakeNBreak files in vac_1_Cdb_0 will be overwritten.", ) + if not _potcars_available(): # test POTCAR warning + assert any( + str(warning.message) + == "POTCAR directory not set up with pymatgen (see the doped docs Installation page: " + "https://doped.readthedocs.io/en/latest/Installation.html for instructions on setting " + "this up). This is required to generate `POTCAR` files and set `NELECT` (i.e. charge " + "state) and `NUPDOWN` in the `INCAR` files!\nNo `POTCAR` files will be written, and " + "`NELECT` and `NUPDOWN` will not be set in `INCAR`s. Beware!" + for warning in w + ) + self.assertFalse(os.path.exists("vac_1_Cdc_0")) V_Cd_POSCAR = Poscar.from_file("vac_1_Cdb_0/Unperturbed/POSCAR") self.assertEqual(V_Cd_POSCAR.comment, "V_Cd Unperturbed, Overwritten") @@ -1370,54 +1403,6 @@ def test_write_vasp_files(self): oxidation_states = {"Cd": +2, "Te": -2} bond_distortions = list(np.arange(-0.6, 0.601, 0.05)) - # test input file kwargs: # TODO: Move to bottom after this works! - reduced_Int_Cd_2 = copy.deepcopy(self.Int_Cd_2) - reduced_Int_Cd_2.user_charges = [ - 1, - ] - reduced_Int_Cd_2_entries = [ - input._get_defect_entry_from_defect(reduced_Int_Cd_2, c) - for c in reduced_Int_Cd_2.user_charges - ] - dist = input.Distortions( - {"Int_Cd_2": reduced_Int_Cd_2_entries}, - oxidation_states=oxidation_states, - distortion_increment=0.25, - distorted_elements={"Int_Cd_2": ["Cd"]}, - dict_number_electrons_user={"Int_Cd_2": 3}, - local_rattle=False, - stdev=0.25, # old default - seed=42, # old default - ) - _, distortion_metadata = dist.write_vasp_files( - verbose=True, - user_potcar_settings={"Cd": "Cd_sv_GW", "Te": "Te_GW"}, - user_potcar_functional="PBE_52", - potcars=_potcars_available(), # to allow testing on GH Actions - ) - self.assertTrue(os.path.exists("Int_Cd_2_+1/Unperturbed")) - _int_Cd_2_POSCAR = Poscar.from_file( - "Int_Cd_2_+1/Unperturbed/POSCAR" - ) # test POSCAR loaded fine - kpoints = Kpoints.from_file("Int_Cd_2_+1/Unperturbed/KPOINTS") - self.assertEqual(kpoints.kpts, [[1, 1, 1]]) - - if _potcars_available(): - assert not filecmp.cmp( # INCAR settings changed now - "Int_Cd_2_+1/Unperturbed/INCAR", self.V_Cd_INCAR_file - ) - int_Cd_2_INCAR = Incar.from_file("Int_Cd_2_+1/Unperturbed/INCAR") - v_Cd_INCAR = self.V_Cd_INCAR.copy() - v_Cd_INCAR.pop("NELECT") # NELECT and NUPDOWN differs for the two defects - v_Cd_INCAR.pop("NUPDOWN") - int_Cd_2_INCAR.pop("NELECT") - int_Cd_2_INCAR.pop("NUPDOWN") - assert v_Cd_INCAR == int_Cd_2_INCAR - - # check if POTCARs have been written: - potcar = Potcar.from_file("Int_Cd_2_+1/Unperturbed/POTCAR") - assert set(potcar.as_dict()["symbols"]) == {"Cd_sv", "Te_GW"} - # Use customised names for defects dist = input.Distortions( self.cdte_defects, @@ -1428,11 +1413,11 @@ def test_write_vasp_files(self): seed=42, # old default ) with patch("builtins.print") as mock_print: - _, distortion_metadata = dist.write_vasp_files( - user_incar_settings={"ENCUT": 212, "IBRION": 0, "EDIFF": 1e-4}, - verbose=False, - potcars=_potcars_available(), # to allow testing on GH Actions - ) + with warnings.catch_warnings(record=True) as w: + _, distortion_metadata = dist.write_vasp_files( + user_incar_settings={"ENCUT": 212, "IBRION": 0, "EDIFF": 1e-4}, + verbose=False, + ) # check if expected folders were created: self.assertTrue( @@ -1517,6 +1502,19 @@ def test_write_vasp_files(self): input.default_potcar_dict["POTCAR"][el_symbol] for el_symbol in V_Cd_POSCAR.structure.symbol_set } + else: # test POTCAR warning + assert ( + len(w) == 2 + ) # general POTCAR warning and NELECT/NUPDOWN INCAR warning + assert issubclass(w[0].category, UserWarning) + assert ( + str(w[0].message) + == "POTCAR directory not set up with pymatgen (see the doped docs Installation page: " + "https://doped.readthedocs.io/en/latest/Installation.html for instructions on setting " + "this up). This is required to generate `POTCAR` files and set `NELECT` (i.e. charge " + "state) and `NUPDOWN` in the `INCAR` files!\nNo `POTCAR` files will be written, and " + "`NELECT` and `NUPDOWN` will not be set in `INCAR`s. Beware!" + ) Int_Cd_2_Bond_Distortion_folder = "Int_Cd_2_0/Bond_Distortion_-60.0%" self.assertTrue(os.path.exists(Int_Cd_2_Bond_Distortion_folder)) @@ -1531,26 +1529,6 @@ def test_write_vasp_files(self): kpoints = Kpoints.from_file(f"{Int_Cd_2_Bond_Distortion_folder}/KPOINTS") self.assertEqual(kpoints.kpts, [[1, 1, 1]]) - if _potcars_available(): - assert not filecmp.cmp( # INCAR settings changed now - f"{Int_Cd_2_Bond_Distortion_folder}/INCAR", self.V_Cd_INCAR_file - ) - assert self.V_Cd_INCAR != Incar.from_file( - f"{Int_Cd_2_Bond_Distortion_folder}/INCAR" - ) - kwarged_INCAR = self.V_Cd_INCAR.copy() - kwarged_INCAR.update({"ENCUT": 212, "IBRION": 0, "EDIFF": 1e-4}) - assert kwarged_INCAR == Incar.from_file( - f"{Int_Cd_2_Bond_Distortion_folder}/INCAR" - ) - - # check if POTCARs have been written: - potcar = Potcar.from_file(f"{Int_Cd_2_Bond_Distortion_folder}/POTCAR") - assert set(potcar.as_dict()["symbols"]) == { - input.default_potcar_dict["POTCAR"][el_symbol] - for el_symbol in V_Cd_POSCAR.structure.symbol_set - } - if _potcars_available(): assert not filecmp.cmp( # INCAR settings changed now f"{Int_Cd_2_Bond_Distortion_folder}/INCAR", self.V_Cd_INCAR_file @@ -1674,6 +1652,8 @@ def test_write_vasp_files(self): ) _, distortion_metadata = dist.write_vasp_files( verbose=True, + user_potcar_settings={"Cd": "Cd_sv_GW", "Te": "Te_GW"}, + user_potcar_functional="PBE_52", ) kwarged_Int_Cd_2_dict = { @@ -1847,6 +1827,27 @@ def test_write_vasp_files(self): ) # Defect added at index 0, so atom indexing + 1 wrt original structure # check correct folder was created: self.assertTrue(os.path.exists("Int_Cd_2_+1/Unperturbed")) + _int_Cd_2_POSCAR = Poscar.from_file( + "Int_Cd_2_+1/Unperturbed/POSCAR" + ) # test POSCAR loaded fine + kpoints = Kpoints.from_file("Int_Cd_2_+1/Unperturbed/KPOINTS") + self.assertEqual(kpoints.kpts, [[1, 1, 1]]) + + if _potcars_available(): + assert not filecmp.cmp( # INCAR settings changed now + "Int_Cd_2_+1/Unperturbed/INCAR", self.V_Cd_INCAR_file + ) + int_Cd_2_INCAR = Incar.from_file("Int_Cd_2_+1/Unperturbed/INCAR") + v_Cd_INCAR = self.V_Cd_INCAR.copy() + v_Cd_INCAR.pop("NELECT") # NELECT and NUPDOWN differs for the two defects + v_Cd_INCAR.pop("NUPDOWN") + int_Cd_2_INCAR.pop("NELECT") + int_Cd_2_INCAR.pop("NUPDOWN") + assert v_Cd_INCAR == int_Cd_2_INCAR + + # check if POTCARs have been written: + potcar = Potcar.from_file("Int_Cd_2_+1/Unperturbed/POTCAR") + assert set(potcar.as_dict()["symbols"]) == {"Cd_sv", "Te_GW"} # check correct output for "extra" electrons and positive charge state: with patch("builtins.print") as mock_Int_Cd_2_print: @@ -2150,7 +2151,7 @@ def test_write_vasp_files_from_list(self): mock_print.assert_any_call( "\nDefect Te_i_Td_Te2.83 in charge state: 0. Number of distorted " "neighbours: 2" - ) # TODO: this is not created + ) # check if correct files were created: V_Cd_Bond_Distortion_folder = "v_Cd_0/Bond_Distortion_-50.0%" From 1be1de5b1d42c29b9f1bb974f827c12bf2f928d2 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 17:28:35 +0100 Subject: [PATCH 10/28] Test POTCAR warning in `test_write_vasp_files()` --- tests/test_input.py | 58 ++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 25 deletions(-) diff --git a/tests/test_input.py b/tests/test_input.py index 9b545b1..ba2196e 100755 --- a/tests/test_input.py +++ b/tests/test_input.py @@ -1084,10 +1084,10 @@ def test_create_vasp_input(self): assert any( str(warning.message) == "POTCAR directory not set up with pymatgen (see the doped docs Installation page: " - "https://doped.readthedocs.io/en/latest/Installation.html for instructions on setting " - "this up). This is required to generate `POTCAR` files and set `NELECT` (i.e. charge " - "state) and `NUPDOWN` in the `INCAR` files!\nNo `POTCAR` files will be written, and " - "`NELECT` and `NUPDOWN` will not be set in `INCAR`s. Beware!" + "https://doped.readthedocs.io/en/latest/Installation.html for instructions on setting " + "this up). This is required to generate `POTCAR` files and set `NELECT` (i.e. charge " + "state) and `NUPDOWN` in the `INCAR` files!\nNo `POTCAR` files will be written, and " + "`NELECT` and `NUPDOWN` will not be set in `INCAR`s. Beware!" for warning in w ) @@ -1503,17 +1503,14 @@ def test_write_vasp_files(self): for el_symbol in V_Cd_POSCAR.structure.symbol_set } else: # test POTCAR warning - assert ( - len(w) == 2 - ) # general POTCAR warning and NELECT/NUPDOWN INCAR warning - assert issubclass(w[0].category, UserWarning) - assert ( - str(w[0].message) + assert any( + str(warning.message) == "POTCAR directory not set up with pymatgen (see the doped docs Installation page: " "https://doped.readthedocs.io/en/latest/Installation.html for instructions on setting " "this up). This is required to generate `POTCAR` files and set `NELECT` (i.e. charge " "state) and `NUPDOWN` in the `INCAR` files!\nNo `POTCAR` files will be written, and " "`NELECT` and `NUPDOWN` will not be set in `INCAR`s. Beware!" + for warning in w ) Int_Cd_2_Bond_Distortion_folder = "Int_Cd_2_0/Bond_Distortion_-60.0%" @@ -1640,21 +1637,22 @@ def test_write_vasp_files(self): ] with patch("builtins.print") as mock_Int_Cd_2_print: - dist = input.Distortions( - {"Int_Cd_2": reduced_Int_Cd_2_entries}, - oxidation_states=oxidation_states, - distortion_increment=0.25, - distorted_elements={"Int_Cd_2": ["Cd"]}, - dict_number_electrons_user={"Int_Cd_2": 3}, - local_rattle=False, - stdev=0.25, # old default - seed=42, # old default - ) - _, distortion_metadata = dist.write_vasp_files( - verbose=True, - user_potcar_settings={"Cd": "Cd_sv_GW", "Te": "Te_GW"}, - user_potcar_functional="PBE_52", - ) + with warnings.catch_warnings(record=True) as w: + dist = input.Distortions( + {"Int_Cd_2": reduced_Int_Cd_2_entries}, + oxidation_states=oxidation_states, + distortion_increment=0.25, + distorted_elements={"Int_Cd_2": ["Cd"]}, + dict_number_electrons_user={"Int_Cd_2": 3}, + local_rattle=False, + stdev=0.25, # old default + seed=42, # old default + ) + _, distortion_metadata = dist.write_vasp_files( + verbose=True, + user_potcar_settings={"Cd": "Cd_sv_GW", "Te": "Te_GW"}, + user_potcar_functional="PBE_52", + ) kwarged_Int_Cd_2_dict = { "distortion_parameters": { @@ -1848,6 +1846,16 @@ def test_write_vasp_files(self): # check if POTCARs have been written: potcar = Potcar.from_file("Int_Cd_2_+1/Unperturbed/POTCAR") assert set(potcar.as_dict()["symbols"]) == {"Cd_sv", "Te_GW"} + else: # test POTCAR warning + assert any( + str(warning.message) + == "POTCAR directory not set up with pymatgen (see the doped docs Installation page: " + "https://doped.readthedocs.io/en/latest/Installation.html for instructions on setting " + "this up). This is required to generate `POTCAR` files and set `NELECT` (i.e. charge " + "state) and `NUPDOWN` in the `INCAR` files!\nNo `POTCAR` files will be written, and " + "`NELECT` and `NUPDOWN` will not be set in `INCAR`s. Beware!" + for warning in w + ) # check correct output for "extra" electrons and positive charge state: with patch("builtins.print") as mock_Int_Cd_2_print: From a6eaf015a8661f23097253aeaed7299528f9b86c Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 17:34:10 +0100 Subject: [PATCH 11/28] Add workflow dispatch options to `pip_install_test` and `release.yml` --- .github/workflows/pip_install_test.yml | 1 + .github/workflows/release.yml | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/workflows/pip_install_test.yml b/.github/workflows/pip_install_test.yml index 0fdf3cd..b2ba8a1 100644 --- a/.github/workflows/pip_install_test.yml +++ b/.github/workflows/pip_install_test.yml @@ -6,6 +6,7 @@ on: branches: [main] types: - completed # only test when new release has been deployed to PyPI + workflow_dispatch: jobs: build-linux: diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index f3d0448..298f4ef 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -6,6 +6,7 @@ on: - main tags: - 'v*' # Push events to matching v*, i.e. v1.0, v20.15.10 + workflow_dispatch: jobs: release: From c69046ad9d5ca104d0c6abba6e67d8477ac66441 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 17:56:50 +0100 Subject: [PATCH 12/28] Update `write_retest_inputs` to copy over all available files (rather than only if INCAR present) --- shakenbreak/energy_lowering_distortions.py | 48 +++++++++++++--------- tests/test_cli.py | 23 +++++++---- tests/test_energy_lowering_distortions.py | 38 ++++++++++------- tests/test_input.py | 1 - 4 files changed, 66 insertions(+), 44 deletions(-) diff --git a/shakenbreak/energy_lowering_distortions.py b/shakenbreak/energy_lowering_distortions.py index 69335e3..2617936 100644 --- a/shakenbreak/energy_lowering_distortions.py +++ b/shakenbreak/energy_lowering_distortions.py @@ -958,31 +958,39 @@ def _copy_vasp_files( """ distorted_structure.to(fmt="poscar", filename=f"{distorted_dir}/POSCAR") - if os.path.exists(f"{output_path}/{defect_species}/Unperturbed/INCAR"): - for i in ["INCAR", "KPOINTS", "POTCAR"]: + for i in ["INCAR", "KPOINTS", "POTCAR"]: + if os.path.exists(f"{output_path}/{defect_species}/Unperturbed/{i}"): shutil.copyfile( f"{output_path}/{defect_species}/Unperturbed/{i}", f"{distorted_dir}/{i}", ) # copy input files from Unperturbed directory - else: - subfolders_with_input_files = [] + + file_dict = { + filename: os.path.exists(f"{distorted_dir}/{filename}") + for filename in ["INCAR", "KPOINTS", "POTCAR"] + } + if not all(file_dict.values()): + # try other distortion directories for subfolder in os.listdir(f"{output_path}/{defect_species}"): - if os.path.exists(f"{output_path}/{defect_species}/{subfolder}/INCAR"): - subfolders_with_input_files.append(subfolder) - if len(subfolders_with_input_files) > 0: - for i in ["INCAR", "KPOINTS", "POTCAR"]: - shutil.copyfile( - f"{output_path}/{defect_species}/" - f"{subfolders_with_input_files[0]}/" - f"{i}", - f"{distorted_dir}/{i}", - ) - else: - print( - f"No subfolders with VASP input files found in " - f"{output_path}/{defect_species}, so just writing distorted " - f"POSCAR file to {distorted_dir} directory." - ) + for filename in ["INCAR", "KPOINTS", "POTCAR"]: + if ( + os.path.exists( + f"{output_path}/{defect_species}/{subfolder}/{filename}" + ) + and not file_dict[filename] + ): + shutil.copyfile( + f"{output_path}/{defect_species}/{subfolder}/{filename}", + f"{distorted_dir}/{filename}", + ) + + if not all(file_dict.values()): + warnings.warn( + f"Subfolders with VASP input files ({[k for k,v in file_dict.items() if not v]} not found in " + f"{output_path}/{defect_species}, so just writing distorted POSCAR file" + f"{f' and {[k for k,v in file_dict.items() if v]}' if any(file_dict.values()) else ''} to " + f"{distorted_dir} directory." + ) def _copy_espresso_files( diff --git a/tests/test_cli.py b/tests/test_cli.py index 4e00d06..7d0c4e6 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -193,6 +193,9 @@ def tearDown(self): if_present_rm(f"{self.VASP_TIO2_DATA_DIR}/Bond_Distortion_20.0%") if_present_rm(f"{self.EXAMPLE_RESULTS}/v_Ti_0/Unperturbed/INCAR") + for i in ["castep", "cp2k", "fhi_aims", "quantum_espresso"]: + if_present_rm(f"{self.DATA_DIR}/{i}/vac_1_Cd_0/default_INCAR") + def copy_v_Ti_OUTCARs(self): """ Copy the OUTCAR files from the `v_Ti_0` `example_results` directory to the `vac_1_Ti_0` `vasp` @@ -3531,8 +3534,9 @@ def test_regenerate(self): catch_exceptions=False, ) defect = "v_Cd" # in example results + non_ignored_warnings = [warning for warning in w if "Subfolders with" not in str(warning.message)] self.assertEqual( - len([warning for warning in w if warning.category == UserWarning]), 0 + len([warning for warning in non_ignored_warnings if warning.category == UserWarning]), 0 ) self.assertIn( @@ -3614,9 +3618,18 @@ def test_regenerate(self): ], catch_exceptions=False, ) + non_ignored_warnings = [warning for warning in w if "Subfolders with" not in str(warning.message)] self.assertEqual( - len([warning for warning in w if warning.category == UserWarning]), 0 + len([warning for warning in non_ignored_warnings if warning.category == UserWarning]), 0 + ) + assert any( + f"Subfolders with VASP input files (['INCAR', 'KPOINTS', 'POTCAR'] not found in " + f"{self.EXAMPLE_RESULTS}/{defect}_-2, so just writing distorted POSCAR file to " + f"{self.EXAMPLE_RESULTS}/{defect}_-2/Bond_Distortion_-60.0%_from_0 directory." + in str(warning.message) + for warning in w ) + self.assertIn( "Comparing structures to specified ref_structure (Cd31 Te32)...", result.output, @@ -3630,12 +3643,6 @@ def test_regenerate(self): f" {self.EXAMPLE_RESULTS}/{defect}_0/Bond_Distortion_20.0%_from_-1\n", result.output, ) - self.assertIn( - f"No subfolders with VASP input files found in {self.EXAMPLE_RESULTS}/{defect}_-2," - f" so just writing distorted POSCAR file to " - f"{self.EXAMPLE_RESULTS}/{defect}_-2/Bond_Distortion_-60.0%_from_0 directory.\n", - result.output, - ) self.assertFalse("High_Energy" in result.output) # test FileNotFoundError raised when no defect folders found diff --git a/tests/test_energy_lowering_distortions.py b/tests/test_energy_lowering_distortions.py index ca32d16..6c54bf8 100644 --- a/tests/test_energy_lowering_distortions.py +++ b/tests/test_energy_lowering_distortions.py @@ -950,7 +950,9 @@ def test_compare_struct_to_distortions(self): os.path.join( self.VASP_CDTE_DATA_DIR, "vac_1_Cd_0/Bond_Distortion_-55.0%/CONTCAR" ), - os.path.join(self.VASP_CDTE_DATA_DIR, "vac_1_Cd_-1/Rattled_from_+1/CONTCAR"), + os.path.join( + self.VASP_CDTE_DATA_DIR, "vac_1_Cd_-1/Rattled_from_+1/CONTCAR" + ), ) shutil.copy( os.path.join( @@ -1010,28 +1012,19 @@ def test_write_retest_inputs(self): ) ) with patch("builtins.print") as mock_print: - energy_lowering_distortions.write_retest_inputs( - low_energy_defects=low_energy_defects_dict, - output_path=self.VASP_CDTE_DATA_DIR, - ) + with warnings.catch_warnings(record=True) as w: + energy_lowering_distortions.write_retest_inputs( + low_energy_defects=low_energy_defects_dict, + output_path=self.VASP_CDTE_DATA_DIR, + ) mock_print.assert_any_call( "Writing low-energy distorted structure to" f" {self.VASP_CDTE_DATA_DIR}/vac_1_Cd_-1/Bond_Distortion_-55.0%_from_0" ) - mock_print.assert_any_call( - f"No subfolders with VASP input files found in {self.VASP_CDTE_DATA_DIR}/vac_1_Cd_-1, " - "so just writing distorted POSCAR file to " - f"{self.VASP_CDTE_DATA_DIR}/vac_1_Cd_-1/Bond_Distortion_-55.0%_from_0 directory." - ) # No VASP input files in distortion directories mock_print.assert_any_call( "Writing low-energy distorted structure to" f" {self.VASP_CDTE_DATA_DIR}/vac_1_Cd_-2/Bond_Distortion_-55.0%_from_0" ) - mock_print.assert_any_call( - f"No subfolders with VASP input files found in {self.VASP_CDTE_DATA_DIR}/vac_1_Cd_-2, " - "so just writing distorted POSCAR file to " - f"{self.VASP_CDTE_DATA_DIR}/vac_1_Cd_-2/Bond_Distortion_-55.0%_from_0 directory." - ) self.assertEqual( self.V_Cd_minus_0pt55_structure, Structure.from_file( @@ -1039,6 +1032,21 @@ def test_write_retest_inputs(self): ), ) + assert any( + f"Subfolders with VASP input files (['INCAR', 'KPOINTS', 'POTCAR'] not found in " + f"{self.VASP_CDTE_DATA_DIR}/vac_1_Cd_-1, so just writing distorted POSCAR file to " + f"{self.VASP_CDTE_DATA_DIR}/vac_1_Cd_-1/Bond_Distortion_-55.0%_from_0 directory." + in str(warning.message) + for warning in w + ) + assert any( + f"Subfolders with VASP input files (['INCAR', 'KPOINTS', 'POTCAR'] not found in " + f"{self.VASP_CDTE_DATA_DIR}/vac_1_Cd_-2, so just writing distorted POSCAR file to " + f"{self.VASP_CDTE_DATA_DIR}/vac_1_Cd_-2/Bond_Distortion_-55.0%_from_0 directory." + in str(warning.message) + for warning in w + ) + # Test for copying over VASP input files (INCAR, KPOINTS and (empty) # POTCAR files) if_present_rm( diff --git a/tests/test_input.py b/tests/test_input.py index ba2196e..4823ff1 100755 --- a/tests/test_input.py +++ b/tests/test_input.py @@ -999,7 +999,6 @@ def test_create_vasp_input(self): "Te_GW", } # Cd_sv_GW POTCAR has Cd_sv symbol, checked else: # test POTCAR warning - print([warning.message for warning in w]) assert any( str(warning.message) == "POTCAR directory not set up with pymatgen (see the doped docs Installation page: " From 29faefb8f24afe3ac412e6d8dc4f1949b0f55142 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 18:41:28 +0100 Subject: [PATCH 13/28] Update workflow files --- .github/workflows/build_and_test.yml | 6 +++--- .github/workflows/lint.yml | 7 +------ .github/workflows/pip_install_test.yml | 9 +++++---- .github/workflows/release.yml | 5 +---- 4 files changed, 10 insertions(+), 17 deletions(-) diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 9f5002e..e20affc 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -33,17 +33,17 @@ jobs: run: | python -m pip install --upgrade pip pip install setuptools setuptools_scm wheel - pip install numpy - pip install -e . pip install -e .[tests] + - name: Check package versions run: | pip show -V pymatgen-analysis-defects pip show -V pymatgen pip show -V pytest + - name: Test run: | - pytest --mpl tests/test_*py # test output plots + pytest --mpl tests # test everything # To generate the test plots: # pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py # generate output plots # pytest --mpl-generate-path=tests/remote_baseline tests/test_shakenbreak.py # generate output plots diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml index c0a71ce..c48f08b 100644 --- a/.github/workflows/lint.yml +++ b/.github/workflows/lint.yml @@ -31,12 +31,7 @@ jobs: run: | python -m pip install --upgrade pip pip install setuptools setuptools_scm wheel - pip install numpy - pip install pydocstyle - pip install black - pip install flake8 - pip install isort - pip install -e . + pip install pydocstyle black flake8 isort pip install -e .[tests] - name: sort imports diff --git a/.github/workflows/pip_install_test.yml b/.github/workflows/pip_install_test.yml index b2ba8a1..1425985 100644 --- a/.github/workflows/pip_install_test.yml +++ b/.github/workflows/pip_install_test.yml @@ -11,7 +11,9 @@ on: jobs: build-linux: runs-on: ubuntu-latest - if: ${{ github.event.workflow_run.conclusion == 'success' }} + if: ${{ github.event_name == 'workflow_dispatch' || (github.event_name == 'workflow_run' && github.event.workflow_run.conclusion == 'success') }} + # only run when tests have passed (or manually triggered) + strategy: max-parallel: 5 @@ -31,11 +33,10 @@ jobs: run: | sleep 10m # wait 10 minutes for PyPI to update with the new release python -m pip install --upgrade pip - pip install shakenbreak # install only from PyPI - pip install shakenbreak[tests] + pip install shakenbreak[tests] # install only from PyPI - name: Test run: | - pytest --mpl tests/test_*.py # test output plots + pytest --mpl tests # test everything # pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py # generate output plots # pytest --mpl-generate-path=tests/remote_baseline tests/test_shakenbreak.py # generate output plots diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 298f4ef..e1dbb4a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -22,14 +22,11 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - pip install setuptools setuptools_scm wheel - pip install numpy - pip install -e . pip install -e .[tests] - name: Test run: | - pytest --mpl tests/test_*.py # test output plots + pytest --mpl tests # test everything - name: Build packages run: | From 935496634f248760a1c9141b156735d454158a63 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 19:04:13 +0100 Subject: [PATCH 14/28] Sequentially run tests to avoid memory issue --- .github/workflows/build_and_test.yml | 7 ++++++- .github/workflows/pip_install_test.yml | 7 ++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index e20affc..4bca6b3 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -43,7 +43,12 @@ jobs: - name: Test run: | - pytest --mpl tests # test everything + for test in tests/test_*py # test everything + do + pytest --mpl $test + fi + done + # To generate the test plots: # pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py # generate output plots # pytest --mpl-generate-path=tests/remote_baseline tests/test_shakenbreak.py # generate output plots diff --git a/.github/workflows/pip_install_test.yml b/.github/workflows/pip_install_test.yml index 1425985..a9d8016 100644 --- a/.github/workflows/pip_install_test.yml +++ b/.github/workflows/pip_install_test.yml @@ -37,6 +37,11 @@ jobs: - name: Test run: | - pytest --mpl tests # test everything + for test in tests/test_*py # test everything + do + pytest --mpl $test + fi + done + # pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py # generate output plots # pytest --mpl-generate-path=tests/remote_baseline tests/test_shakenbreak.py # generate output plots From 431438a7cfbb4270d3a45ca392f9f4feca62690d Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 19:08:17 +0100 Subject: [PATCH 15/28] =?UTF-8?q?Fix=20typo=20=F0=9F=98=85?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .github/workflows/build_and_test.yml | 4 +--- .github/workflows/pip_install_test.yml | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 4bca6b3..5c49e2a 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -44,9 +44,7 @@ jobs: - name: Test run: | for test in tests/test_*py # test everything - do - pytest --mpl $test - fi + do pytest --mpl $test done # To generate the test plots: diff --git a/.github/workflows/pip_install_test.yml b/.github/workflows/pip_install_test.yml index a9d8016..c5e1467 100644 --- a/.github/workflows/pip_install_test.yml +++ b/.github/workflows/pip_install_test.yml @@ -38,9 +38,7 @@ jobs: - name: Test run: | for test in tests/test_*py # test everything - do - pytest --mpl $test - fi + do pytest --mpl $test done # pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py # generate output plots From 297ebc3db11e6b982c65c72cffbdd1608bda5cf5 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 19:34:43 +0100 Subject: [PATCH 16/28] Try avoid apparent GH Actions memory issue --- .github/workflows/build_and_test.yml | 6 +++--- .github/workflows/pip_install_test.yml | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 5c49e2a..5f0b577 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -43,9 +43,9 @@ jobs: - name: Test run: | - for test in tests/test_*py # test everything - do pytest --mpl $test - done + pytest tests # test everything + pytest --mpl tests/test_plotting.py # plotting tests + pytest --mpl tests/test_shakenbreak.py # plotting tests # To generate the test plots: # pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py # generate output plots diff --git a/.github/workflows/pip_install_test.yml b/.github/workflows/pip_install_test.yml index c5e1467..74c0ef7 100644 --- a/.github/workflows/pip_install_test.yml +++ b/.github/workflows/pip_install_test.yml @@ -37,9 +37,9 @@ jobs: - name: Test run: | - for test in tests/test_*py # test everything - do pytest --mpl $test - done + pytest tests # test everything + pytest --mpl tests/test_plotting.py # plotting tests + pytest --mpl tests/test_shakenbreak.py # plotting tests # pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py # generate output plots # pytest --mpl-generate-path=tests/remote_baseline tests/test_shakenbreak.py # generate output plots From b5618696f7f28b9a81ce8f581e31918d3c77a153 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 20:29:27 +0100 Subject: [PATCH 17/28] Install `numpy` before running tests --- .github/workflows/build_and_test.yml | 2 +- .github/workflows/pip_install_test.yml | 1 + docs/shakenbreak.rst | 1 - docs/shakenbreak.vasp.rst | 6 ------ shakenbreak/cli.py | 6 ++---- 5 files changed, 4 insertions(+), 12 deletions(-) delete mode 100644 docs/shakenbreak.vasp.rst diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/build_and_test.yml index 5f0b577..c1d5675 100644 --- a/.github/workflows/build_and_test.yml +++ b/.github/workflows/build_and_test.yml @@ -32,7 +32,7 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - pip install setuptools setuptools_scm wheel + pip install numpy pip install -e .[tests] - name: Check package versions diff --git a/.github/workflows/pip_install_test.yml b/.github/workflows/pip_install_test.yml index 74c0ef7..d5d4211 100644 --- a/.github/workflows/pip_install_test.yml +++ b/.github/workflows/pip_install_test.yml @@ -33,6 +33,7 @@ jobs: run: | sleep 10m # wait 10 minutes for PyPI to update with the new release python -m pip install --upgrade pip + pip install numpy pip install shakenbreak[tests] # install only from PyPI - name: Test diff --git a/docs/shakenbreak.rst b/docs/shakenbreak.rst index b371490..231028c 100644 --- a/docs/shakenbreak.rst +++ b/docs/shakenbreak.rst @@ -21,5 +21,4 @@ Submodules shakenbreak.analysis shakenbreak.plotting shakenbreak.energy_lowering_distortions - shakenbreak.vasp shakenbreak.cli \ No newline at end of file diff --git a/docs/shakenbreak.vasp.rst b/docs/shakenbreak.vasp.rst deleted file mode 100644 index f04df88..0000000 --- a/docs/shakenbreak.vasp.rst +++ /dev/null @@ -1,6 +0,0 @@ -shakenbreak.vasp module -============================ -.. automodule:: shakenbreak.vasp - :members: - :undoc-members: - :show-inheritance: diff --git a/shakenbreak/cli.py b/shakenbreak/cli.py index ddf36d0..cd9ef6a 100755 --- a/shakenbreak/cli.py +++ b/shakenbreak/cli.py @@ -29,10 +29,8 @@ def _parse_defect_dirs(path) -> list: for dir in os.listdir(path) if os.path.isdir(f"{path}/{dir}") and any( - [ - fnmatch.filter(os.listdir(f"{path}/{dir}"), f"{dist}*") - for dist in ["Rattled", "Unperturbed", "Bond_Distortion"] - ] + fnmatch.filter(os.listdir(f"{path}/{dir}"), f"{dist}*") + for dist in ["Rattled", "Unperturbed", "Bond_Distortion"] ) # only parse defect directories that contain distortion folders ] From 5b2102759ad961fd16fe6f632d7ba948c04c9fe4 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 20:30:59 +0100 Subject: [PATCH 18/28] Refactor `build_and_test.yml` to `test.yml` --- .github/workflows/{build_and_test.yml => test.yml} | 0 README.md | 2 +- docs/index.rst | 2 +- 3 files changed, 2 insertions(+), 2 deletions(-) rename .github/workflows/{build_and_test.yml => test.yml} (100%) diff --git a/.github/workflows/build_and_test.yml b/.github/workflows/test.yml similarity index 100% rename from .github/workflows/build_and_test.yml rename to .github/workflows/test.yml diff --git a/README.md b/README.md index 03bb7ee..111636e 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -[![Build status](https://github.com/SMTG-UCL/ShakeNBreak/actions/workflows/build_and_test.yml/badge.svg)](https://github.com/SMTG-UCL/ShakeNBreak/actions) +[![Build status](https://github.com/SMTG-UCL/ShakeNBreak/actions/workflows/test.yml/badge.svg)](https://github.com/SMTG-UCL/ShakeNBreak/actions) [![Documentation Status](https://readthedocs.org/projects/shakenbreak/badge/?version=latest&style=flat)](https://shakenbreak.readthedocs.io/en/latest/) [![JOSS](https://joss.theoj.org/papers/10.21105/joss.04817/status.svg)](https://doi.org/10.21105/joss.04817) [![PyPI](https://img.shields.io/pypi/v/shakenbreak)](https://pypi.org/project/shakenbreak) diff --git a/docs/index.rst b/docs/index.rst index 4aa8931..c038136 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -3,7 +3,7 @@ You can adapt this file completely to your liking, but it should at least contain the root `toctree` directive. -.. image:: https://github.com/SMTG-UCL/ShakeNBreak/actions/workflows/build_and_test.yml/badge.svg +.. image:: https://github.com/SMTG-UCL/ShakeNBreak/actions/workflows/test.yml/badge.svg :target: https://github.com/SMTG-UCL/ShakeNBreak/actions .. image:: https://readthedocs.org/projects/shakenbreak/badge/?version=latest&style=flat From c9e3a1f9ab1c5c87887ae8c8f60f61f1fb5e9df7 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 20:43:12 +0100 Subject: [PATCH 19/28] Didn't fix it. Make tests verbose --- .github/workflows/pip_install_test.yml | 7 +++---- .github/workflows/test.yml | 7 +++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/.github/workflows/pip_install_test.yml b/.github/workflows/pip_install_test.yml index d5d4211..ea923cd 100644 --- a/.github/workflows/pip_install_test.yml +++ b/.github/workflows/pip_install_test.yml @@ -33,14 +33,13 @@ jobs: run: | sleep 10m # wait 10 minutes for PyPI to update with the new release python -m pip install --upgrade pip - pip install numpy pip install shakenbreak[tests] # install only from PyPI - name: Test run: | - pytest tests # test everything - pytest --mpl tests/test_plotting.py # plotting tests - pytest --mpl tests/test_shakenbreak.py # plotting tests + pytest tests -vv # test everything + pytest --mpl tests/test_plotting.py -vv # plotting tests + pytest --mpl tests/test_shakenbreak.py -vv # plotting tests # pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py # generate output plots # pytest --mpl-generate-path=tests/remote_baseline tests/test_shakenbreak.py # generate output plots diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c1d5675..872142d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -32,7 +32,6 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - pip install numpy pip install -e .[tests] - name: Check package versions @@ -43,9 +42,9 @@ jobs: - name: Test run: | - pytest tests # test everything - pytest --mpl tests/test_plotting.py # plotting tests - pytest --mpl tests/test_shakenbreak.py # plotting tests + pytest tests -vv # test everything + pytest --mpl tests/test_plotting.py -vv # plotting tests + pytest --mpl tests/test_shakenbreak.py -vv # plotting tests # To generate the test plots: # pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py # generate output plots From 593c01d726a3444ae2b110fc370907510e114944 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 21:35:35 +0100 Subject: [PATCH 20/28] Move `_potcars_available` function to `test_cli` --- tests/test_cli.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tests/test_cli.py b/tests/test_cli.py index 7d0c4e6..42707ef 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -20,14 +20,25 @@ from pymatgen.core.structure import Structure from pymatgen.io.vasp.inputs import Poscar, UnknownPotcarWarning, Kpoints, Potcar, Incar +from doped.vasp import _test_potcar_functional_choice + from shakenbreak.cli import snb from shakenbreak.distortions import rattle from shakenbreak.input import generate_defect_object -from tests.test_input import _potcars_available - file_path = os.path.dirname(__file__) +def _potcars_available() -> bool: + """ + Check if the POTCARs are available for the tests (i.e. testing locally). + + If not (testing on GitHub Actions), POTCAR testing will be skipped. + """ + try: + _test_potcar_functional_choice("PBE") + return True + except ValueError: + return False def if_present_rm(path): if os.path.exists(path): From 18803b1fef090a487ff040b82a3732e6124cb671 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 21:38:22 +0100 Subject: [PATCH 21/28] Update GitHub workflows --- .github/workflows/pip_install_test.yml | 4 +--- .github/workflows/test.yml | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/.github/workflows/pip_install_test.yml b/.github/workflows/pip_install_test.yml index ea923cd..69f32b8 100644 --- a/.github/workflows/pip_install_test.yml +++ b/.github/workflows/pip_install_test.yml @@ -37,9 +37,7 @@ jobs: - name: Test run: | - pytest tests -vv # test everything - pytest --mpl tests/test_plotting.py -vv # plotting tests - pytest --mpl tests/test_shakenbreak.py -vv # plotting tests + pytest --mpl tests -vv # test everything # pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py # generate output plots # pytest --mpl-generate-path=tests/remote_baseline tests/test_shakenbreak.py # generate output plots diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 872142d..45e944f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -42,9 +42,7 @@ jobs: - name: Test run: | - pytest tests -vv # test everything - pytest --mpl tests/test_plotting.py -vv # plotting tests - pytest --mpl tests/test_shakenbreak.py -vv # plotting tests + pytest --mpl tests -vv # test everything # To generate the test plots: # pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py # generate output plots From c19c2f64bef98f6819c45ca454931ae51cb4dd34 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 22:36:47 +0100 Subject: [PATCH 22/28] Add debug lines to `test_CLI` (temporarily) --- .github/workflows/test.yml | 1 + tests/test_cli.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 45e944f..5c55bcf 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -42,6 +42,7 @@ jobs: - name: Test run: | + pytest --mpl tests/test_cli.py -vv -k "generate" # test generate first to find issue pytest --mpl tests -vv # test everything # To generate the test plots: diff --git a/tests/test_cli.py b/tests/test_cli.py index 42707ef..edbdc9a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -6,6 +6,7 @@ import re import shutil import subprocess +import inspect import unittest import warnings @@ -225,7 +226,9 @@ def test_snb_generate(self): """Implicitly, the `snb-generate` tests also test the functionality of `input.identify_defect()` """ + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number runner = CliRunner() + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number result = runner.invoke( snb, [ @@ -241,6 +244,7 @@ def test_snb_generate(self): ], catch_exceptions=False, ) + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number self.assertEqual(result.exit_code, 0) self.assertIn( f"Auto site-matching identified {self.VASP_CDTE_DATA_DIR}/CdTe_V_Cd_POSCAR " @@ -294,18 +298,23 @@ def test_snb_generate(self): self.V_Cd_minus0pt5_struc_rattled, ) + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number kpoints = Kpoints.from_file(f"{V_Cd_Bond_Distortion_folder}/KPOINTS") self.assertEqual(kpoints.kpts, [[1, 1, 1]]) + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number if _potcars_available(): + print(f"Hre: Line: {inspect.currentframe().f_lineno}") # print current line number assert filecmp.cmp(f"{V_Cd_Bond_Distortion_folder}/INCAR", self.V_Cd_INCAR_file) # check if POTCARs have been written: potcar = Potcar.from_file(f"{V_Cd_Bond_Distortion_folder}/POTCAR") assert set(potcar.as_dict()["symbols"]) == {"Cd", "Te"} + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number # Test recognises distortion_metadata.json: if_present_rm(f"{defect_name}_0") # but distortion_metadata.json still present + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number result = runner.invoke( snb, [ @@ -319,6 +328,7 @@ def test_snb_generate(self): ], catch_exceptions=False, ) # non-verbose this time + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number self.assertEqual(result.exit_code, 0) self.assertNotIn( "Auto site-matching identified" @@ -365,7 +375,9 @@ def test_snb_generate(self): ) # test defect_index option: + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number self.tearDown() + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number result = runner.invoke( snb, [ @@ -381,6 +393,7 @@ def test_snb_generate(self): "-v", ], ) + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number self.assertEqual(result.exit_code, 0) self.assertNotIn("Auto site-matching", result.output) self.assertIn("Oxidation states were not explicitly set", result.output) @@ -454,11 +467,14 @@ def test_snb_generate(self): with open("distortion_metadata.json", "r") as metadata_file: metadata = json.load(metadata_file) np.testing.assert_equal(metadata, wrong_site_V_Cd_dict) + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number # test warning with defect_coords option but wrong site: (matches Cd site in bulk) # using Int_Cd because V_Cd is at (0,0,0) so fractional and Cartesian coordinates the same self.tearDown() + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number with warnings.catch_warnings(record=True) as w: + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number result = runner.invoke( snb, [ @@ -477,6 +493,7 @@ def test_snb_generate(self): ], catch_exceptions=False, ) + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number self.assertEqual(result.exit_code, 0) warning_message = ( "Coordinates (0.0, 0.0, 0.0) were specified for (auto-determined) interstitial " @@ -503,7 +520,9 @@ def test_snb_generate(self): ) # test defect_coords working even when slightly off correct site + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number self.tearDown() + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number with warnings.catch_warnings(record=True) as w: result = runner.invoke( snb, @@ -522,8 +541,11 @@ def test_snb_generate(self): "-v", ], ) + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number self.assertEqual(result.exit_code, 0) + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number if w: + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number # Check no problems in identifying the defect site self.assertFalse( any(str(warning.message) == warning_message for warning in w) @@ -544,10 +566,12 @@ def test_snb_generate(self): Structure.from_file(f"{defect_name}_0/Bond_Distortion_-60.0%/POSCAR"), self.Int_Cd_2_minus0pt6_struc_rattled, ) + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number # test defect_coords working even when significantly off (~2.2 Å) correct site, # with rattled bulk self.tearDown() + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number with warnings.catch_warnings(record=True) as w: rattled_bulk = rattle( self.CdTe_bulk_struc, stdev=0.25, d_min=2.25 @@ -594,6 +618,7 @@ def test_snb_generate(self): # test defect_coords working even when slightly off correct site with V_Cd and rattled bulk self.tearDown() + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number with warnings.catch_warnings(record=True) as w: rattled_bulk = rattle( self.CdTe_bulk_struc, stdev=0.25, d_min=2.25 @@ -731,6 +756,7 @@ def test_snb_generate(self): np.testing.assert_equal(metadata, spec_coords_V_Cd_dict) # test defect ID with tricky DX centre defect + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number result = runner.invoke( snb, [ @@ -782,6 +808,7 @@ def test_snb_generate(self): # test padding functionality: # default padding = 1 + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number result = runner.invoke( snb, [ @@ -849,6 +876,7 @@ def test_snb_generate(self): self.assertTrue(os.path.exists(f"{defect_name}_-6")) self.assertFalse(os.path.exists(f"{defect_name}_+5")) self.assertFalse(os.path.exists(f"{defect_name}_-7")) + print(f"Line: {inspect.currentframe().f_lineno}") # print current line number def test_snb_generate_config(self): # test config file: From 4582b42d2268f41b3512df17aad4477cf97d8328 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 23:05:55 +0100 Subject: [PATCH 23/28] Save matplotlib figures for debuggin --- .github/workflows/pip_install_test.yml | 12 ++++++++++-- .github/workflows/test.yml | 18 +++++++++--------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/.github/workflows/pip_install_test.yml b/.github/workflows/pip_install_test.yml index 69f32b8..4e54440 100644 --- a/.github/workflows/pip_install_test.yml +++ b/.github/workflows/pip_install_test.yml @@ -39,5 +39,13 @@ jobs: run: | pytest --mpl tests -vv # test everything - # pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py # generate output plots - # pytest --mpl-generate-path=tests/remote_baseline tests/test_shakenbreak.py # generate output plots + # Generate the test plots in case there were any failures: + pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py + pytest --mpl-generate-path=tests/remote_baseline tests/test_shakenbreak.py + + # Upload test plots + - name: Archive test plots + uses: actions/upload-artifact@v3 + with: + name: output-plots + path: tests/remote_baseline diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 5c55bcf..6e8301e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -45,16 +45,16 @@ jobs: pytest --mpl tests/test_cli.py -vv -k "generate" # test generate first to find issue pytest --mpl tests -vv # test everything - # To generate the test plots: - # pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py # generate output plots - # pytest --mpl-generate-path=tests/remote_baseline tests/test_shakenbreak.py # generate output plots + # Generate the test plots in case there were any failures: + pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py + pytest --mpl-generate-path=tests/remote_baseline tests/test_shakenbreak.py - # Download test plots - # - name: Archive test plots - # uses: actions/upload-artifact@v3 - # with: - # name: output-plots - # path: tests/remote_baseline + # Upload test plots + - name: Archive test plots + uses: actions/upload-artifact@v3 + with: + name: output-plots + path: tests/remote_baseline # - name: Download a single artifact # uses: actions/download-artifact@v3 From 009dbdbaf8bf87675a8732a031f066958438ff2b Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 23:26:15 +0100 Subject: [PATCH 24/28] Refactor backend handling to separate function `_get_backend` to use with `doped` --- shakenbreak/plotting.py | 30 ++++++++++++++++++------------ tests/test_cli.py | 27 --------------------------- 2 files changed, 18 insertions(+), 39 deletions(-) diff --git a/shakenbreak/plotting.py b/shakenbreak/plotting.py index 1412881..f407c4e 100644 --- a/shakenbreak/plotting.py +++ b/shakenbreak/plotting.py @@ -72,6 +72,23 @@ def _install_custom_font(): warnings.warn(warning_msg) +def _get_backend(save_format: str) -> str: + """Try use pycairo as backend if installed, and save_format is pdf.""" + backend = None + if "pdf" in save_format: + try: + import cairo + + backend = "cairo" + except ImportError: + warnings.warn( + "pycairo not installed. Defaulting to matplotlib's pdf backend, so default " + "ShakeNBreak fonts may not be used – try setting `save_format` to 'png' or " + "`pip install pycairo` if you want ShakeNBreak's default font." + ) + return backend + + # Helper functions for formatting plots def _verify_data_directories_exist( output_path: str, @@ -501,18 +518,7 @@ def _save_plot( ) # use pycairo as backend if installed and save_format is pdf: - backend = None - if "pdf" in save_format: - try: - import cairo - - backend = "cairo" - except ImportError: - warnings.warn( - "pycairo not installed. Defaulting to matplotlib's pdf backend, so default " - "ShakeNBreak fonts may not be used – try setting `save_format` to 'png' or " - "`pip install pycairo` if you want ShakeNBreak's default font." - ) + backend = _get_backend(save_format) fig.savefig( plot_filepath, diff --git a/tests/test_cli.py b/tests/test_cli.py index edbdc9a..60ddedb 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -226,9 +226,7 @@ def test_snb_generate(self): """Implicitly, the `snb-generate` tests also test the functionality of `input.identify_defect()` """ - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number runner = CliRunner() - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number result = runner.invoke( snb, [ @@ -244,7 +242,6 @@ def test_snb_generate(self): ], catch_exceptions=False, ) - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number self.assertEqual(result.exit_code, 0) self.assertIn( f"Auto site-matching identified {self.VASP_CDTE_DATA_DIR}/CdTe_V_Cd_POSCAR " @@ -298,23 +295,18 @@ def test_snb_generate(self): self.V_Cd_minus0pt5_struc_rattled, ) - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number kpoints = Kpoints.from_file(f"{V_Cd_Bond_Distortion_folder}/KPOINTS") self.assertEqual(kpoints.kpts, [[1, 1, 1]]) - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number if _potcars_available(): - print(f"Hre: Line: {inspect.currentframe().f_lineno}") # print current line number assert filecmp.cmp(f"{V_Cd_Bond_Distortion_folder}/INCAR", self.V_Cd_INCAR_file) # check if POTCARs have been written: potcar = Potcar.from_file(f"{V_Cd_Bond_Distortion_folder}/POTCAR") assert set(potcar.as_dict()["symbols"]) == {"Cd", "Te"} - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number # Test recognises distortion_metadata.json: if_present_rm(f"{defect_name}_0") # but distortion_metadata.json still present - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number result = runner.invoke( snb, [ @@ -328,7 +320,6 @@ def test_snb_generate(self): ], catch_exceptions=False, ) # non-verbose this time - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number self.assertEqual(result.exit_code, 0) self.assertNotIn( "Auto site-matching identified" @@ -375,9 +366,7 @@ def test_snb_generate(self): ) # test defect_index option: - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number self.tearDown() - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number result = runner.invoke( snb, [ @@ -393,7 +382,6 @@ def test_snb_generate(self): "-v", ], ) - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number self.assertEqual(result.exit_code, 0) self.assertNotIn("Auto site-matching", result.output) self.assertIn("Oxidation states were not explicitly set", result.output) @@ -467,14 +455,11 @@ def test_snb_generate(self): with open("distortion_metadata.json", "r") as metadata_file: metadata = json.load(metadata_file) np.testing.assert_equal(metadata, wrong_site_V_Cd_dict) - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number # test warning with defect_coords option but wrong site: (matches Cd site in bulk) # using Int_Cd because V_Cd is at (0,0,0) so fractional and Cartesian coordinates the same self.tearDown() - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number with warnings.catch_warnings(record=True) as w: - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number result = runner.invoke( snb, [ @@ -493,7 +478,6 @@ def test_snb_generate(self): ], catch_exceptions=False, ) - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number self.assertEqual(result.exit_code, 0) warning_message = ( "Coordinates (0.0, 0.0, 0.0) were specified for (auto-determined) interstitial " @@ -520,9 +504,7 @@ def test_snb_generate(self): ) # test defect_coords working even when slightly off correct site - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number self.tearDown() - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number with warnings.catch_warnings(record=True) as w: result = runner.invoke( snb, @@ -541,11 +523,8 @@ def test_snb_generate(self): "-v", ], ) - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number self.assertEqual(result.exit_code, 0) - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number if w: - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number # Check no problems in identifying the defect site self.assertFalse( any(str(warning.message) == warning_message for warning in w) @@ -566,12 +545,10 @@ def test_snb_generate(self): Structure.from_file(f"{defect_name}_0/Bond_Distortion_-60.0%/POSCAR"), self.Int_Cd_2_minus0pt6_struc_rattled, ) - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number # test defect_coords working even when significantly off (~2.2 Å) correct site, # with rattled bulk self.tearDown() - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number with warnings.catch_warnings(record=True) as w: rattled_bulk = rattle( self.CdTe_bulk_struc, stdev=0.25, d_min=2.25 @@ -618,7 +595,6 @@ def test_snb_generate(self): # test defect_coords working even when slightly off correct site with V_Cd and rattled bulk self.tearDown() - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number with warnings.catch_warnings(record=True) as w: rattled_bulk = rattle( self.CdTe_bulk_struc, stdev=0.25, d_min=2.25 @@ -756,7 +732,6 @@ def test_snb_generate(self): np.testing.assert_equal(metadata, spec_coords_V_Cd_dict) # test defect ID with tricky DX centre defect - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number result = runner.invoke( snb, [ @@ -808,7 +783,6 @@ def test_snb_generate(self): # test padding functionality: # default padding = 1 - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number result = runner.invoke( snb, [ @@ -876,7 +850,6 @@ def test_snb_generate(self): self.assertTrue(os.path.exists(f"{defect_name}_-6")) self.assertFalse(os.path.exists(f"{defect_name}_+5")) self.assertFalse(os.path.exists(f"{defect_name}_-7")) - print(f"Line: {inspect.currentframe().f_lineno}") # print current line number def test_snb_generate_config(self): # test config file: From f1ff54b7ca22d9fbfd0a40aac0f3668a31bcec41 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Tue, 29 Aug 2023 23:29:07 +0100 Subject: [PATCH 25/28] Update `CHANGELOG.rst` and `setup.py` --- CHANGELOG.rst | 7 +++++++ docs/conf.py | 2 +- setup.py | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 1887f89..87e06a6 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -1,6 +1,13 @@ Change Log ========== +v3.2.1 +---------- +- Update CLI config handling. +- Remove `shakenbreak.vasp` module and use `doped` VASP file writing functions directly. +- Add INCAR/KPOINTS/POTCAR file writing tests. `test_local.py` now deleted as these tests are now + automatically run in `test_input.py`/`test_cli.py` if `POTCAR`s available. + v3.2.0 ---------- - Following the major release of `doped` `v2.0`, now compatible with the new `pymatgen` diff --git a/docs/conf.py b/docs/conf.py index 85d0616..e6dc239 100644 --- a/docs/conf.py +++ b/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.2.0' +release = '3.2.1' # -- General configuration --------------------------------------------------- diff --git a/setup.py b/setup.py index 7e2f4d2..e6434c2 100644 --- a/setup.py +++ b/setup.py @@ -130,7 +130,7 @@ def package_files(directory): setup( name="shakenbreak", - version="3.2.0", + version="3.2.1", description="Package to generate and analyse distorted defect structures, in order to " "identify ground-state and metastable defect configurations.", long_description=long_description, From 3b294daea138dde1c9848503c9efe72be3ac693b Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Wed, 30 Aug 2023 10:13:26 +0100 Subject: [PATCH 26/28] Always archive test plots, even if tests fail --- .github/workflows/pip_install_test.yml | 3 ++- .github/workflows/test.yml | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/pip_install_test.yml b/.github/workflows/pip_install_test.yml index 4e54440..ab367b2 100644 --- a/.github/workflows/pip_install_test.yml +++ b/.github/workflows/pip_install_test.yml @@ -37,7 +37,7 @@ jobs: - name: Test run: | - pytest --mpl tests -vv # test everything + pytest --mpl -vv tests # test everything # Generate the test plots in case there were any failures: pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py @@ -45,6 +45,7 @@ jobs: # Upload test plots - name: Archive test plots + if: always() # always upload the plots, even if the tests fail uses: actions/upload-artifact@v3 with: name: output-plots diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 6e8301e..99e465f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -42,8 +42,7 @@ jobs: - name: Test run: | - pytest --mpl tests/test_cli.py -vv -k "generate" # test generate first to find issue - pytest --mpl tests -vv # test everything + pytest --mpl -vv tests # test everything # Generate the test plots in case there were any failures: pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py @@ -51,6 +50,7 @@ jobs: # Upload test plots - name: Archive test plots + if: always() # always upload the plots, even if the tests fail uses: actions/upload-artifact@v3 with: name: output-plots From 1e7e438838473f62fd3bd3325aef81988a48ea71 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Wed, 30 Aug 2023 10:13:44 +0100 Subject: [PATCH 27/28] Typing --- shakenbreak/plotting.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shakenbreak/plotting.py b/shakenbreak/plotting.py index f407c4e..2502c25 100644 --- a/shakenbreak/plotting.py +++ b/shakenbreak/plotting.py @@ -72,7 +72,7 @@ def _install_custom_font(): warnings.warn(warning_msg) -def _get_backend(save_format: str) -> str: +def _get_backend(save_format: str) -> Optional[str]: """Try use pycairo as backend if installed, and save_format is pdf.""" backend = None if "pdf" in save_format: From 6d87f8d6051fc64fe775157012050a8865d603a0 Mon Sep 17 00:00:00 2001 From: Sean Kavanagh Date: Wed, 30 Aug 2023 11:45:28 +0100 Subject: [PATCH 28/28] Always generate GH Actions test plots --- .github/workflows/pip_install_test.yml | 3 +++ .github/workflows/test.yml | 3 +++ shakenbreak/plotting.py | 2 +- 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pip_install_test.yml b/.github/workflows/pip_install_test.yml index ab367b2..5ae629c 100644 --- a/.github/workflows/pip_install_test.yml +++ b/.github/workflows/pip_install_test.yml @@ -39,6 +39,9 @@ jobs: run: | pytest --mpl -vv tests # test everything + - name: Generate GH Actions test plots + if: always() # always generate the plots, even if the tests fail + run: | # Generate the test plots in case there were any failures: pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py pytest --mpl-generate-path=tests/remote_baseline tests/test_shakenbreak.py diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 99e465f..2487c66 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -44,6 +44,9 @@ jobs: run: | pytest --mpl -vv tests # test everything + - name: Generate GH Actions test plots + if: always() # always generate the plots, even if the tests fail + run: | # Generate the test plots in case there were any failures: pytest --mpl-generate-path=tests/remote_baseline tests/test_plotting.py pytest --mpl-generate-path=tests/remote_baseline tests/test_shakenbreak.py diff --git a/shakenbreak/plotting.py b/shakenbreak/plotting.py index 2502c25..0c31944 100644 --- a/shakenbreak/plotting.py +++ b/shakenbreak/plotting.py @@ -77,7 +77,7 @@ def _get_backend(save_format: str) -> Optional[str]: backend = None if "pdf" in save_format: try: - import cairo + import cairo # noqa: F401 backend = "cairo" except ImportError: