Repository: Quantum-Accelerators/quacc
Pull request #1447 (2023-12-31 19:59:19)

Fix a few Espresso results parsing bugs
## Requirements

### Checklist

- [X] I have read the [Contributing Guide](https://quantum-accelerators.github.io/quacc/dev/contributing.html). Don't lie! 😉
- [X] My PR is on a custom branch and is _not_ named `main`.
- [X] I have used `black`, `isort`, and `ruff` as described in the [style guide](https://quantum-accelerators.github.io/quacc/dev/contributing.html#style).
- [X] I have added relevant, comprehensive [unit tests](https://quantum-accelerators.github.io/quacc/dev/contributing.html#unit-tests).

### Notes

- Your PR will likely not be merged without proper and thorough tests.
- If you are an external contributor, you will see a comment from [@buildbot-princeton](https://github.com/buildbot-princeton). This is solely for the maintainers.
- When your code is ready for review, ping one of the [active maintainers](https://quantum-accelerators.github.io/quacc/about/contributors.html#active-maintainers).

## Summary of Changes

This PR addresses several existing issues with the Espresso calculator and recipes, as enumerated below in decreasing order of importance.

1. The Espresso recipes never updated the `Atoms` object in the returned output when a structure relaxation occurs. That is because a `geom_file` keyword argument needed to be passed to `run_calc()`. There is inconsistent behavior in ASE where some calculators update the `Atoms` object when an "internal" relaxation is done, whereas others don't (see ASE [issue 1368](https://gitlab.com/ase/ase/-/issues/1368)). In practice, what this means is that we must explicitly tell quacc which file is the geometry log file, which is then parsed at the end of a run and used to update the `Atoms` object in the returned schema. I have set this to be `pw.out` if `pw` is the binary, otherwise no geometry file is read (same behavior as before).

2. All quacc recipes dump out the `calc.results` dictionary in the output schema (see [here](https://github.com/Quantum-Accelerators/quacc/blob/606617717dc334219d8432046cd99a2addd99851/src/quacc/schemas/_aliases/ase.py#L14-L22)). This, by default, typically contains properties like "energy" and "forces" (i.e. the `implemented_properties` from the calculator). However, before this PR, this does not appear to be done for `pw.x` runs, as there was no "energy" key (for instance). This is now addressed by fixing the `read_results` method of the `Espresso` calculator. I then added tests to evaluate the energy from the static job runs.

3. If the preset has `kspacing` in it but the user specifies `kpts` explicitly when calling a recipe, an error is raised that both `kpts` and `kspacing` can't be set. This PR now checks to see if the user has specified `kpts`, and if so, `kspacing` will be dropped from the preset (and vice versa).

TODO:
- Speed up the cell relaxation test because it takes too long

Andrew-S-Rosen: @tomdemeyere if you have a moment, it would be great to get your feedback on these bugfixes.
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/Quantum-Accelerators/quacc/pull/1447?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Quantum-Accelerators) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Comparison is base [(`6066177`)](https://app.codecov.io/gh/Quantum-Accelerators/quacc/commit/606617717dc334219d8432046cd99a2addd99851?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Quantum-Accelerators) 99.14% compared to head [(`0d19997`)](https://app.codecov.io/gh/Quantum-Accelerators/quacc/pull/1447?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Quantum-Accelerators) 99.14%.


<details><summary>Additional details and impacted files</summary>


```diff
@@           Coverage Diff           @@
##             main    #1447   +/-   ##
=======================================
  Coverage   99.14%   99.14%           
=======================================
  Files          76       76           
  Lines        2795     2801    +6     
=======================================
+ Hits         2771     2777    +6     
  Misses         24       24           
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/Quantum-Accelerators/quacc/pull/1447?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Quantum-Accelerators).   
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Quantum-Accelerators).


Patch:
diff --git a/src/quacc/calculators/espresso/espresso.py b/src/quacc/calculators/espresso/espresso.py
index 6aa0b9dc..c5db2ba7 100644
--- a/src/quacc/calculators/espresso/espresso.py
+++ b/src/quacc/calculators/espresso/espresso.py
@@ -122,11 +122,8 @@ class EspressoTemplate(EspressoTemplate_):
         """
 
         if self.binary == "pw":
-            results = read(
-                filename=directory / self.outputname,
-                format="espresso-out",
-                full_output=True,
-            )
+            atoms = read(directory / self.outputname, format="espresso-out")
+            results = dict(atoms.calc.properties())
         elif self.binary == "ph":
             with Path.open(directory / self.outputname, "r") as fd:
                 results = read_espresso_ph(fd)
@@ -288,6 +285,10 @@ class Espresso(Espresso_):
                     calc_preset["pseudopotentials"], self.input_atoms
                 )
                 calc_preset.pop("pseudopotentials", None)
+                if "kpts" in self.kwargs:
+                    calc_preset.pop("kspacing", None)
+                if "kspacing" in self.kwargs:
+                    calc_preset.pop("kpts", None)
                 self._user_calc_params = recursive_dict_merge(
                     calc_preset,
                     {
@@ -297,9 +298,12 @@ class Espresso(Espresso_):
                         "pseudopotentials": pseudopotentials,
                     },
                     self.kwargs,
+                    allowed_nones=["kpts"],
                 )
             else:
-                self._user_calc_params = recursive_dict_merge(calc_preset, self.kwargs)
+                self._user_calc_params = recursive_dict_merge(
+                    calc_preset, self.kwargs, allowed_nones=["kpts"]
+                )
         else:
             self._user_calc_params = self.kwargs
 
diff --git a/src/quacc/recipes/espresso/_base.py b/src/quacc/recipes/espresso/_base.py
index d8237ccb..ad214074 100644
--- a/src/quacc/recipes/espresso/_base.py
+++ b/src/quacc/recipes/espresso/_base.py
@@ -64,7 +64,7 @@ def base_fn(
     """
 
     atoms = Atoms() if atoms is None else atoms
-    calc_flags = recursive_dict_merge(calc_defaults, calc_swaps)
+    calc_flags = recursive_dict_merge(calc_defaults, calc_swaps, allowed_nones=["kpts"])
 
     atoms.calc = Espresso(
         input_atoms=atoms,
@@ -75,7 +75,9 @@ def base_fn(
         **calc_flags,
     )
 
-    final_atoms = run_calc(atoms, copy_files=copy_files)
+    geom_file = template.outputname if template.binary == "pw" else None
+
+    final_atoms = run_calc(atoms, geom_file=geom_file, copy_files=copy_files)
 
     return summarize_run(
         final_atoms, input_atoms=atoms, additional_fields=additional_fields
diff --git a/src/quacc/utils/dicts.py b/src/quacc/utils/dicts.py
index 818e3e9f..7f7a93d1 100644
--- a/src/quacc/utils/dicts.py
+++ b/src/quacc/utils/dicts.py
@@ -8,16 +8,22 @@ if TYPE_CHECKING:
     from typing import Any
 
 
-def recursive_dict_merge(*args, remove_nones: bool = True) -> dict[str, Any]:
+def recursive_dict_merge(
+    *args, remove_nones: bool = True, allowed_nones: list[Any] | None = None
+) -> dict[str, Any]:
     """
     Recursively merge several dictionaries, taking the latter in the list as higher preference.
+    It will also remove any keys that are None in the final, merged dictionary if `remove_nones`
+    is True.
 
     Parameters
     ----------
     *args
         Dictionaries to merge
     remove_nones
-        If True, recursively remove all items that are None.
+        If True, recursively remove all items that are None in the final, merged dictionary.
+    allowed_nones
+        List of keys to never remove if `remove_nones` is True.
 
     Returns
     -------
@@ -30,7 +36,7 @@ def recursive_dict_merge(*args, remove_nones: bool = True) -> dict[str, Any]:
         old_dict = safe_dict_copy(merged)
 
     if remove_nones:
-        merged = remove_dict_nones(merged)
+        merged = remove_dict_nones(merged, allowed_nones=allowed_nones)
     return merged
 
 
@@ -93,7 +99,9 @@ def safe_dict_copy(d: dict) -> dict:
         return d.copy()
 
 
-def remove_dict_nones(start_dict: dict[str, Any]) -> dict[str, Any]:
+def remove_dict_nones(
+    start_dict: dict[str, Any], allowed_nones: list[Any] | None = None
+) -> dict[str, Any]:
     """
     For a given dictionary, recursively remove all items that are None.
 
@@ -101,6 +109,8 @@ def remove_dict_nones(start_dict: dict[str, Any]) -> dict[str, Any]:
     ----------
     start_dict
         Dictionary to clean
+    allowed_nones
+        List of keys to never remove
 
     Returns
     -------
@@ -108,10 +118,16 @@ def remove_dict_nones(start_dict: dict[str, Any]) -> dict[str, Any]:
         Cleaned dictionary
     """
 
+    if allowed_nones is None:
+        allowed_nones = []
     if isinstance(start_dict, dict):
-        return {k: remove_dict_nones(v) for k, v in start_dict.items() if v is not None}
+        return {
+            k: remove_dict_nones(v, allowed_nones=allowed_nones)
+            for k, v in start_dict.items()
+            if (v is not None or k in allowed_nones)
+        }
     return (
-        [remove_dict_nones(v) for v in start_dict]
+        [remove_dict_nones(v, allowed_nones=allowed_nones) for v in start_dict]
         if isinstance(start_dict, list)
         else start_dict
     )


Test patch:
diff --git a/tests/core/calculators/espresso/test_espresso.py b/tests/core/calculators/espresso/test_espresso.py
index 0edb31c3..3b40facf 100644
--- a/tests/core/calculators/espresso/test_espresso.py
+++ b/tests/core/calculators/espresso/test_espresso.py
@@ -44,11 +44,11 @@ def test_espresso_kwargs_handler_v2():
 
     atoms = Atoms(symbols="LiLaOZr")
 
-    calc = Espresso(input_atoms=atoms, preset=preset, kpts=(1, 1, 1))
+    calc = Espresso(input_atoms=atoms, preset=preset, kpts=None)
 
     expected_parameters = {
         "input_data": {"system": {"ecutwfc": 50.0, "ecutrho": 400.0}},
-        "kpts": (1, 1, 1),
+        "kpts": None,
         "pseudopotentials": {
             "O": "O.pbe-n-kjpaw_psl.0.1.UPF",
             "Zr": "Zr_pbe_v1.uspp.F.UPF",
diff --git a/tests/core/recipes/espresso_recipes/Li.upf.gz b/tests/core/recipes/espresso_recipes/data/Li.upf.gz
similarity index 100%
rename from tests/core/recipes/espresso_recipes/Li.upf.gz
rename to tests/core/recipes/espresso_recipes/data/Li.upf.gz
diff --git a/tests/core/recipes/espresso_recipes/Si.upf.gz b/tests/core/recipes/espresso_recipes/data/Si.upf.gz
similarity index 100%
rename from tests/core/recipes/espresso_recipes/Si.upf.gz
rename to tests/core/recipes/espresso_recipes/data/Si.upf.gz
diff --git a/tests/core/recipes/espresso_recipes/test_espresso.py b/tests/core/recipes/espresso_recipes/test_espresso.py
index 59a3fde3..9223fd5f 100644
--- a/tests/core/recipes/espresso_recipes/test_espresso.py
+++ b/tests/core/recipes/espresso_recipes/test_espresso.py
@@ -3,8 +3,7 @@ from shutil import which
 
 import pytest
 from ase.build import bulk
-from ase.io.espresso import construct_namelist
-from numpy.testing import assert_allclose
+from numpy.testing import assert_allclose, assert_array_equal
 
 from quacc import SETTINGS
 from quacc.recipes.espresso.core import post_processing_job, relax_job, static_job
@@ -14,14 +13,13 @@ from quacc.utils.files import copy_decompress_files
 pytestmark = pytest.mark.skipif(which("pw.x") is None, reason="QE not installed")
 
 DEFAULT_SETTINGS = SETTINGS.model_copy()
+DATA_DIR = Path(__file__).parent / "data"
 
 
 def test_static_job(tmp_path, monkeypatch):
     monkeypatch.chdir(tmp_path)
 
-    pp_dir = Path(__file__).parent
-
-    copy_decompress_files([pp_dir / "Si.upf.gz"], tmp_path)
+    copy_decompress_files([DATA_DIR / "Si.upf.gz"], tmp_path)
 
     atoms = bulk("Si")
 
@@ -29,28 +27,31 @@ def test_static_job(tmp_path, monkeypatch):
     input_data = {"control": {"pseudo_dir": tmp_path}}
 
     results = static_job(
-        atoms, input_data=input_data, pseudopotentials=pseudopotentials, kspacing=0.5
+        atoms, input_data=input_data, pseudopotentials=pseudopotentials, kpts=None
     )
 
-    assert_allclose(results["atoms"].positions, atoms.positions, atol=1.0e-4)
-
-    assert_allclose(results["atoms"].cell, atoms.cell, atol=1.0e-3)
-    assert (results["atoms"].symbols == atoms.symbols).all()
+    assert_allclose(
+        results["atoms"].get_positions(), atoms.get_positions(), atol=1.0e-4
+    )
+    assert_allclose(results["atoms"].get_cell(), atoms.get_cell(), atol=1.0e-3)
+    assert_array_equal(
+        results["atoms"].get_chemical_symbols(), atoms.get_chemical_symbols()
+    )
+    assert results["results"]["energy"] == pytest.approx(-293.71195934404255)
 
     new_input_data = results["parameters"]["input_data"]
-
     assert new_input_data["system"]["degauss"] == 0.001
     assert new_input_data["system"]["occupations"] == "smearing"
     assert new_input_data["system"]["ecutwfc"] == 30.0
     assert new_input_data["system"]["ecutrho"] == 240.0
+    assert "kspacing" not in results["parameters"]
+    assert results["parameters"].get("kpts") is None
 
 
 def test_static_job_v2(tmp_path, monkeypatch):
     monkeypatch.chdir(tmp_path)
 
-    pp_dir = Path(__file__).parent
-
-    copy_decompress_files([pp_dir / "Si.upf.gz"], tmp_path)
+    copy_decompress_files([DATA_DIR / "Si.upf.gz"], tmp_path)
 
     atoms = bulk("Si")
 
@@ -66,29 +67,31 @@ def test_static_job_v2(tmp_path, monkeypatch):
         atoms, input_data=input_data, pseudopotentials=pseudopotentials, kspacing=0.5
     )
 
-    assert_allclose(results["atoms"].positions, atoms.positions, atol=1.0e-4)
-
-    assert_allclose(results["atoms"].cell, atoms.cell, atol=1.0e-3)
-    assert (results["atoms"].symbols == atoms.symbols).all()
+    assert_allclose(
+        results["atoms"].get_positions(), atoms.get_positions(), atol=1.0e-4
+    )
+    assert_allclose(results["atoms"].get_cell(), atoms.get_cell(), atol=1.0e-3)
+    assert_array_equal(
+        results["atoms"].get_chemical_symbols(), atoms.get_chemical_symbols()
+    )
+    assert results["results"]["energy"] == pytest.approx(-293.71195934404255)
 
     new_input_data = results["parameters"]["input_data"]
-
     assert new_input_data["system"]["degauss"] == 0.005
     assert new_input_data["system"]["occupations"] == "smearing"
     assert new_input_data["electrons"]["conv_thr"] == 1.0e-6
     assert new_input_data["control"]["calculation"] == "scf"
+    assert "kpts" not in results["parameters"]
+    assert results["parameters"]["kspacing"] == 0.5
 
     pp_results = post_processing_job(prev_dir=results["dir_name"])
-
     assert Path(pp_results["dir_name"], "pseudo_charge_density.cube.gz").is_file()
 
 
 def test_static_job_outdir(tmp_path, monkeypatch):
     monkeypatch.chdir(tmp_path)
 
-    pp_dir = Path(__file__).parent
-
-    copy_decompress_files([pp_dir / "Si.upf.gz"], tmp_path)
+    copy_decompress_files([DATA_DIR / "Si.upf.gz"], tmp_path)
 
     atoms = bulk("Si")
 
@@ -101,18 +104,19 @@ def test_static_job_outdir(tmp_path, monkeypatch):
     pseudopotentials = {"Si": "Si.upf"}
 
     results = static_job(
-        atoms, input_data=input_data, pseudopotentials=pseudopotentials, kspacing=0.5
+        atoms, input_data=input_data, pseudopotentials=pseudopotentials, kpts=None
     )
 
-    input_data = dict(construct_namelist(input_data))
-
-    assert_allclose(results["atoms"].positions, atoms.positions, atol=1.0e-4)
-
-    assert_allclose(results["atoms"].cell, atoms.cell, atol=1.0e-3)
-    assert (results["atoms"].symbols == atoms.symbols).all()
+    assert_allclose(
+        results["atoms"].get_positions(), atoms.get_positions(), atol=1.0e-4
+    )
+    assert_allclose(results["atoms"].get_cell(), atoms.get_cell(), atol=1.0e-3)
+    assert_array_equal(
+        results["atoms"].get_chemical_symbols(), atoms.get_chemical_symbols()
+    )
+    assert results["results"]["energy"] == pytest.approx(-293.71195934404255)
 
     new_input_data = results["parameters"]["input_data"]
-
     assert new_input_data["system"]["degauss"] == 0.005
     assert new_input_data["system"]["occupations"] == "smearing"
     assert new_input_data["electrons"]["conv_thr"] == 1.0e-6
@@ -122,9 +126,7 @@ def test_static_job_outdir(tmp_path, monkeypatch):
 def test_static_job_outdir_abs(tmp_path, monkeypatch):
     monkeypatch.chdir(tmp_path)
 
-    pp_dir = Path(__file__).parent
-
-    copy_decompress_files([pp_dir / "Si.upf.gz"], tmp_path)
+    copy_decompress_files([DATA_DIR / "Si.upf.gz"], tmp_path)
 
     atoms = bulk("Si")
 
@@ -139,18 +141,19 @@ def test_static_job_outdir_abs(tmp_path, monkeypatch):
     pseudopotentials = {"Si": "Si.upf"}
 
     results = static_job(
-        atoms, input_data=input_data, pseudopotentials=pseudopotentials, kspacing=0.5
+        atoms, input_data=input_data, pseudopotentials=pseudopotentials, kpts=None
     )
 
-    input_data = dict(construct_namelist(input_data))
-
-    assert_allclose(results["atoms"].positions, atoms.positions, atol=1.0e-4)
-
-    assert_allclose(results["atoms"].cell, atoms.cell, atol=1.0e-3)
-    assert (results["atoms"].symbols == atoms.symbols).all()
+    assert_allclose(
+        results["atoms"].get_positions(), atoms.get_positions(), atol=1.0e-4
+    )
+    assert_allclose(results["atoms"].get_cell(), atoms.get_cell(), atol=1.0e-3)
+    assert_array_equal(
+        results["atoms"].get_chemical_symbols(), atoms.get_chemical_symbols()
+    )
+    assert results["results"]["energy"] == pytest.approx(-293.71195934404255)
 
     new_input_data = results["parameters"]["input_data"]
-
     assert new_input_data["system"]["degauss"] == 0.005
     assert new_input_data["system"]["occupations"] == "smearing"
     assert new_input_data["electrons"]["conv_thr"] == 1.0e-6
@@ -160,9 +163,7 @@ def test_static_job_outdir_abs(tmp_path, monkeypatch):
 def test_static_job_dir_fail(tmp_path, monkeypatch):
     monkeypatch.chdir(tmp_path)
 
-    pp_dir = Path(__file__).parent
-
-    copy_decompress_files([pp_dir / "Si.upf.gz"], tmp_path)
+    copy_decompress_files([DATA_DIR / "Si.upf.gz"], tmp_path)
 
     atoms = bulk("Si")
 
@@ -173,19 +174,24 @@ def test_static_job_dir_fail(tmp_path, monkeypatch):
 def test_relax_job(tmp_path, monkeypatch):
     monkeypatch.chdir(tmp_path)
 
-    pp_dir = Path(__file__).parent
-
-    copy_decompress_files([pp_dir / "Si.upf.gz"], tmp_path)
+    copy_decompress_files([DATA_DIR / "Si.upf.gz"], tmp_path)
 
     atoms = bulk("Si")
+    atoms[0].position += 0.05
 
     pseudopotentials = {"Si": "Si.upf"}
     input_data = {"control": {"pseudo_dir": tmp_path}}
 
     results = relax_job(
-        atoms, input_data=input_data, pseudopotentials=pseudopotentials, kspacing=0.5
+        atoms, input_data=input_data, pseudopotentials=pseudopotentials, kpts=None
     )
 
+    with pytest.raises(AssertionError):
+        assert_allclose(
+            results["atoms"].get_positions(), atoms.get_positions(), atol=1.0e-4
+        )
+    assert_allclose(results["atoms"].get_cell(), atoms.get_cell(), atol=1.0e-3)
+
     new_input_data = results["parameters"]["input_data"]
     assert new_input_data["control"]["calculation"] == "relax"
 
@@ -193,9 +199,7 @@ def test_relax_job(tmp_path, monkeypatch):
 def test_relax_job_cell(tmp_path, monkeypatch):
     monkeypatch.chdir(tmp_path)
 
-    pp_dir = Path(__file__).parent
-
-    copy_decompress_files([pp_dir / "Si.upf.gz"], tmp_path)
+    copy_decompress_files([DATA_DIR / "Si.upf.gz"], tmp_path)
 
     atoms = bulk("Si")
 
@@ -207,10 +211,17 @@ def test_relax_job_cell(tmp_path, monkeypatch):
         relax_cell=True,
         input_data=input_data,
         pseudopotentials=pseudopotentials,
-        kspacing=0.5,
+        kpts=None,
     )
-    new_input_data = results["parameters"]["input_data"]
 
+    with pytest.raises(AssertionError):
+        assert_allclose(
+            results["atoms"].get_positions(), atoms.get_positions(), atol=1.0e-4
+        )
+    with pytest.raises(AssertionError):
+        assert_allclose(results["atoms"].get_cell(), atoms.get_cell(), atol=1.0e-3)
+
+    new_input_data = results["parameters"]["input_data"]
     assert new_input_data["control"]["calculation"] == "vc-relax"
 
 
@@ -219,9 +230,7 @@ def test_phonon_job(tmp_path, monkeypatch):
 
     atoms = bulk("Li")
 
-    pp_dir = Path(__file__).parent
-
-    copy_decompress_files([pp_dir / "Li.upf.gz"], tmp_path)
+    copy_decompress_files([DATA_DIR / "Li.upf.gz"], tmp_path)
 
     SETTINGS.ESPRESSO_PSEUDO = tmp_path
 
@@ -236,23 +245,28 @@ def test_phonon_job(tmp_path, monkeypatch):
     pseudopotentials = {"Li": "Li.upf"}
 
     pw_results = static_job(
-        atoms, input_data=input_data, pseudopotentials=pseudopotentials, kspacing=0.25
+        atoms, input_data=input_data, pseudopotentials=pseudopotentials, kspacing=0.5
     )
 
     ph_results = phonon_job(pw_results["dir_name"], input_data=ph_loose)
 
     assert (0, 0, 0) in ph_results["results"]
     assert_allclose(
-        ph_results["results"][(0, 0, 0)]["atoms"].positions,
-        atoms.positions,
+        ph_results["results"][(0, 0, 0)]["atoms"].get_positions(),
+        atoms.get_positions(),
         atol=1.0e-4,
     )
     # ph.x cell param are not defined to a very high level of accuracy,
     # atol = 1.0e-3 is needed here...
     assert_allclose(
-        ph_results["results"][(0, 0, 0)]["atoms"].cell, atoms.cell, atol=1.0e-3
+        ph_results["results"][(0, 0, 0)]["atoms"].get_cell(),
+        atoms.get_cell(),
+        atol=1.0e-3,
+    )
+    assert_array_equal(
+        ph_results["results"][(0, 0, 0)]["atoms"].get_chemical_symbols(),
+        atoms.get_chemical_symbols(),
     )
-    assert (ph_results["results"][(0, 0, 0)]["atoms"].symbols == atoms.symbols).all()
 
     sections = ["atoms", "eqpoints", "freqs", "kpoints", "mode_symmetries", "modes"]
 
@@ -265,9 +279,7 @@ def test_phonon_job_list_to_do(tmp_path, monkeypatch):
 
     atoms = bulk("Li")
 
-    pp_dir = Path(__file__).parent
-
-    copy_decompress_files([pp_dir / "Li.upf.gz"], tmp_path)
+    copy_decompress_files([DATA_DIR / "Li.upf.gz"], tmp_path)
 
     SETTINGS.ESPRESSO_PSEUDO = tmp_path
 
@@ -276,15 +288,13 @@ def test_phonon_job_list_to_do(tmp_path, monkeypatch):
         "system": {"occupations": "smearing", "smearing": "cold", "degauss": 0.02},
         "electrons": {"mixing_mode": "TF", "mixing_beta": 0.7, "conv_thr": 1.0e-6},
     }
-
     ph_loose = {
         "inputph": {"tr2_ph": 1e-8, "qplot": True, "nat_todo": 1, "ldisp": True}
     }
-
     pseudopotentials = {"Li": "Li.upf"}
 
     pw_results = static_job(
-        atoms, input_data=input_data, pseudopotentials=pseudopotentials, kspacing=0.25
+        atoms, input_data=input_data, pseudopotentials=pseudopotentials, kspacing=0.5
     )
 
     qpts = [(0, 0, 0, 1), (1 / 3, 0, 0, 1), (1 / 2, 0, 0, 1)]
@@ -297,16 +307,21 @@ def test_phonon_job_list_to_do(tmp_path, monkeypatch):
 
     assert (0, 0, 0) in ph_results["results"]
     assert_allclose(
-        ph_results["results"][(0, 0, 0)]["atoms"].positions,
-        atoms.positions,
+        ph_results["results"][(0, 0, 0)]["atoms"].get_positions(),
+        atoms.get_positions(),
         atol=1.0e-4,
     )
     # ph.x cell param are not defined to a very high level of accuracy,
     # atol = 1.0e-3 is needed here...
     assert_allclose(
-        ph_results["results"][(0, 0, 0)]["atoms"].cell, atoms.cell, atol=1.0e-3
+        ph_results["results"][(0, 0, 0)]["atoms"].get_cell(),
+        atoms.get_cell(),
+        atol=1.0e-3,
+    )
+    assert_array_equal(
+        ph_results["results"][(0, 0, 0)]["atoms"].get_chemical_symbols(),
+        atoms.get_chemical_symbols(),
     )
-    assert (ph_results["results"][(0, 0, 0)]["atoms"].symbols == atoms.symbols).all()
 
     sections = ["atoms", "eqpoints", "freqs", "kpoints", "mode_symmetries", "modes"]
 
diff --git a/tests/core/utils/test_dicts.py b/tests/core/utils/test_dicts.py
index e93a04a0..62b875f2 100644
--- a/tests/core/utils/test_dicts.py
+++ b/tests/core/utils/test_dicts.py
@@ -20,6 +20,33 @@ def test_remove_dict_nones():
     }
 
 
+def test_remove_dict_nones2():
+    d = {
+        "output": {
+            "output": {
+                "test": [1, None],
+                "test2": 1,
+                "test3": None,
+                "test4": {},
+                "test5": [],
+            }
+        },
+        "test": None,
+    }
+    d = remove_dict_nones(d, allowed_nones=["test3"])
+    assert d == {
+        "output": {
+            "output": {
+                "test": [1, None],
+                "test2": 1,
+                "test3": None,
+                "test4": {},
+                "test5": [],
+            }
+        }
+    }
+
+
 def test_recursive_dict_merge():
     defaults = {"a": 1, "b": {"a": 1, "b": 2}}
     calc_swaps = {"c": 3, "b": {"b": 3, "d": 1}}
@@ -28,3 +55,32 @@ def test_recursive_dict_merge():
         "b": {"a": 1, "b": 3, "d": 1},
         "c": 3,
     }
+
+
+def test_recursive_dict_merge2():
+    defaults = {"a": 1, "b": {"a": 1, "b": 2}}
+    calc_swaps = {"c": None, "b": {"b": 3, "d": 1}}
+    assert recursive_dict_merge(defaults, calc_swaps) == {
+        "a": 1,
+        "b": {"a": 1, "b": 3, "d": 1},
+    }
+
+
+def test_recursive_dict_merge3():
+    defaults = {"a": 1, "b": {"a": 1, "b": 2}}
+    calc_swaps = {"c": None, "b": {"b": 3, "d": 1}}
+    assert recursive_dict_merge(defaults, calc_swaps, remove_nones=False) == {
+        "a": 1,
+        "b": {"a": 1, "b": 3, "d": 1},
+        "c": None,
+    }
+
+
+def test_recursive_dict_merge4():
+    defaults = {"a": 1, "b": {"a": 1, "b": 2}}
+    calc_swaps = {"c": None, "b": {"b": 3, "d": 1}}
+    assert recursive_dict_merge(defaults, calc_swaps, allowed_nones=["c"]) == {
+        "a": 1,
+        "b": {"a": 1, "b": 3, "d": 1},
+        "c": None,
+    }