Merge pull request #58 from SMTG-UCL/develop

Develop

.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

.github/workflows/pip_install_test.yml

@@ -6,11 +6,14 @@ on:
     branches: [main]
     types:
       - completed  # only test when new release has been deployed to PyPI
+  workflow_dispatch:
 
 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
 
@@ -30,17 +33,23 @@ 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:  |
-          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-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
+          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
+
+      # 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
+          path: tests/remote_baseline

.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:
@@ -21,20 +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:  |
-          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 everything
 
       - name: Build packages
         run: |

.github/workflows/build_and_test.yml → .github/workflows/test.yml

@@ -32,34 +32,32 @@ 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: Check package versions
         run: |
           pip show -V pymatgen-analysis-defects
           pip show -V pymatgen
           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-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
+          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
 
-      # 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
+        if: always()  # always upload the plots, even if the tests fail
+        uses: actions/upload-artifact@v3
+        with:
+          name: output-plots
+          path: tests/remote_baseline
 
       # - name: Download a single artifact
       #   uses: actions/download-artifact@v3

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`

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)

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

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

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

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

docs/shakenbreak.rst

@@ -21,5 +21,4 @@ Submodules
   shakenbreak.analysis
   shakenbreak.plotting
   shakenbreak.energy_lowering_distortions
-  shakenbreak.vasp
   shakenbreak.cli

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,

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
     ]
 
@@ -196,8 +194,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 +235,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:
@@ -320,19 +316,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=user_potcar_settings,
+            user_incar_settings=user_incar_settings,
         )
     elif code.lower() == "cp2k":
         if input_file:
@@ -487,9 +483,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 +521,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:
@@ -684,19 +678,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=user_potcar_settings,
+            user_incar_settings=user_incar_settings,
         )
     elif code.lower() == "cp2k":
         if input_file: