Repository: Quantum-Accelerators/quacc
Pull request #1682 (2024-02-10 06:53:49)

Ensure all results are DB-compatible
Requires:
- https://github.com/materialsvirtuallab/matgl/pull/225

Andrew-S-Rosen: @tomdemeyere: 

I am adding a test in this PR that ensures all results can safely be deposited in a database. There are two minor instances in the Espresso recipes that fail this test, as you can see [here](https://github.com/Quantum-Accelerators/quacc/actions/runs/7853478814/job/21433140349?pr=1682#step:7:545). Basically, it boils down to the fact that one can't have a "." in the key of a dictionary for most database formats, as this messes with querying.

The first instance of this is, as you can see in the aforementioned report, has to do with the lines [here](https://github.com/Quantum-Accelerators/quacc/blob/70dee9824febd49fec27db57a5064207899d9ab9/src/quacc/calculators/espresso/espresso.py#L210-L222). The `pwscf.dos` and `pwscf.pdos_tot` are stored as keys since `fildos.name` and `filpdos.name` are keys to the results dictionary. Any chance we can rename this for data storage purposes? I imagine this one might be okay if we do something like replace the "." with a more DB-friendly delimiter like a "_"? Let me know what you'd prefer, and I can make the change here.

The second instance is a bit trickier. Namely, when running the phonon workflow, the k-point info is stored like `(0.0, 0.0, 0.0)` as a key. This causes some trouble. Any idea on how we might get around this one?

For a minimal test, see below:

```python
from maggma.stores import MemoryStore

results = {"uuid": 12345, "pwscf.dos": "test"}
with MemoryStore() as store:
    store.update(results, key="uuid") # should not error out
```
tomdemeyere: Feel free to change the naming stuff to an underscore

For phonons: https://gitlab.com/ase/ase/-/merge_requests/3252

tomdemeyere: For the database there are a few things that came up in the office while working with Quacc:

- Would there be a way to add a SETTINGS that control the verbosity of results? from 1 to 3, default would be 3, 1 would only return plain results (atoms, energy, forces...) without any additional information.

- Would it be possible to send a label in static_job (maybe in additional_fields?) that would get carried down to the results?

Of course I explained that this can all be done manually, just giving some feedback here.
Andrew-S-Rosen: > Feel free to change the naming stuff to an underscore

Thanks. Done!

> For phonons: https://gitlab.com/ase/ase/-/merge_requests/3252

Thanks. Merged! I just need to patch the new keys in `quacc` now.

> Would there be a way to add a SETTINGS that control the verbosity of results? from 1 to 3, default would be 3, 1 would only return plain results (atoms, energy, forces...) without any additional information.

I am hesitant to enable this because it will increase the fragility in writing new recipes. By having a constant schema, it's always known what keys are available to be used when designing a new `Flow`. If we have a dynamic schema, however, it means developers of new recipes will have to check what verbosity the user is running with and then, potentially, write the `Flow` accordingly, which is messy. It also means that the type hints would all be wrong (in the code and in the documentation). The principle of least surprise here wins out.

> Would it be possible to send a label in static_job (maybe in additional_fields?) that would get carried down to the results?

Ah, this was planned at some point but I had forgotten to do so. Yes, this should be done and is why `additional_fields` exists.
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/Quantum-Accelerators/quacc/pull/1682?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 [(`f213ffc`)](https://app.codecov.io/gh/Quantum-Accelerators/quacc/commit/f213ffc8acd9c939e43d040fb0a81042d03dc1e1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Quantum-Accelerators) 99.29% compared to head [(`224e04e`)](https://app.codecov.io/gh/Quantum-Accelerators/quacc/pull/1682?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Quantum-Accelerators) 99.23%.
> Report is 3 commits behind head on main.


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


```diff
@@            Coverage Diff             @@
##             main    #1682      +/-   ##
==========================================
- Coverage   99.29%   99.23%   -0.07%     
==========================================
  Files          76       76              
  Lines        2994     2994              
==========================================
- Hits         2973     2971       -2     
- Misses         21       23       +2     
```



</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/Quantum-Accelerators/quacc/pull/1682?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).

Andrew-S-Rosen: Merging this in. Feel free to let me know if there are any problems!

Patch:
diff --git a/src/quacc/calculators/espresso/espresso.py b/src/quacc/calculators/espresso/espresso.py
index a42d2a95..c65b24bf 100644
--- a/src/quacc/calculators/espresso/espresso.py
+++ b/src/quacc/calculators/espresso/espresso.py
@@ -211,7 +211,7 @@ class EspressoTemplate(EspressoTemplate_):
                 lines = fd.readlines()
                 fermi = float(re.search(r"-?\d+\.?\d*", lines[0]).group(0))
                 dos = np.loadtxt(lines[1:])
-            results = {fildos.name: {"dos": dos, "fermi": fermi}}
+            results = {fildos.name.replace(".", "_"): {"dos": dos, "fermi": fermi}}
         elif self.binary == "projwfc":
             filpdos = self.outfiles["filpdos"]
             with Path(filpdos).open("r") as fd:
@@ -219,7 +219,13 @@ class EspressoTemplate(EspressoTemplate_):
                 energy = lines[1:, 0]
                 dos = lines[1:, 1]
                 pdos = lines[1:, 2]
-            results = {filpdos.name: {"energy": energy, "dos": dos, "pdos": pdos}}
+            results = {
+                filpdos.name.replace(".", "_"): {
+                    "energy": energy,
+                    "dos": dos,
+                    "pdos": pdos,
+                }
+            }
         else:
             results = {}
 
diff --git a/src/quacc/recipes/espresso/phonons.py b/src/quacc/recipes/espresso/phonons.py
index 50c37870..799335d8 100644
--- a/src/quacc/recipes/espresso/phonons.py
+++ b/src/quacc/recipes/espresso/phonons.py
@@ -212,12 +212,12 @@ def grid_phonon_flow(
         ph_input_data.to_nested(binary="ph")
 
         grid_results = []
-        for qpoint, qdata in ph_init_job_results["results"].items():
-            ph_input_data["inputph"]["start_q"] = qdata["qnum"]
-            ph_input_data["inputph"]["last_q"] = qdata["qnum"]
+        for qnum, qdata in ph_init_job_results["results"].items():
+            ph_input_data["inputph"]["start_q"] = qnum
+            ph_input_data["inputph"]["last_q"] = qnum
             repr_to_do = grid_prepare_repr(qdata["representations"], nblocks)
             file_to_copy = grid_copy_files(
-                ph_input_data, ph_init_job_results["dir_name"], qdata["qnum"], qpoint
+                ph_input_data, ph_init_job_results["dir_name"], qnum, qdata["qpoint"]
             )
             for representation in repr_to_do:
                 ph_input_data["inputph"]["start_irr"] = representation[0]


Test patch:
diff --git a/tests/core/.quacc.yaml b/tests/core/.quacc.yaml
index 53aceb54..e6bdf69b 100644
--- a/tests/core/.quacc.yaml
+++ b/tests/core/.quacc.yaml
@@ -1,2 +1,5 @@
 DEBUG: true
 WORKFLOW_ENGINE: null
+STORE:
+  MemoryStore:
+    collection_name: test_store
diff --git a/tests/core/calculators/espresso/test_io.py b/tests/core/calculators/espresso/test_io.py
index 1ee5c8af..97ada39a 100644
--- a/tests/core/calculators/espresso/test_io.py
+++ b/tests/core/calculators/espresso/test_io.py
@@ -30,17 +30,15 @@ def test_read_espresso_ph_1():
     results = read_espresso_ph(fd)
 
     assert len(results) == 8
-    assert (0, 0, 0) in results
-    assert np.unique(results[(0, 0, 0)]["freqs"]).shape[0] == 1
-    assert np.unique(results[(0, 0, 0)]["freqs"])[0] == 0.173268
-    assert len(results[(0, 0, 0)]["eqpoints"]) == 1
-    assert results[(0, 0, 0)]["atoms"].symbols == ["Al"]
+    assert np.unique(results[1]["freqs"]).shape[0] == 1
+    assert np.unique(results[1]["freqs"])[0] == 0.173268
+    assert len(results[1]["eqpoints"]) == 1
+    assert results[1]["atoms"].symbols == ["Al"]
 
-    assert (0.75, -0.25, 0.75) in results
-    assert np.unique(results[(0.75, -0.25, 0.75)]["freqs"]).shape[0] == 3
-    assert np.unique(results[(0.75, -0.25, 0.75)]["freqs"])[2] == 8.791383
-    assert len(results[(0.75, -0.25, 0.75)]["eqpoints"]) == 24
-    assert results[(0.75, -0.25, 0.75)]["atoms"].symbols == ["Al"]
+    assert np.unique(results[5]["freqs"]).shape[0] == 3
+    assert np.unique(results[5]["freqs"])[2] == 8.791383
+    assert len(results[5]["eqpoints"]) == 24
+    assert results[5]["atoms"].symbols == ["Al"]
 
 
 def test_dos_output(tmp_path, monkeypatch):
@@ -48,4 +46,4 @@ def test_dos_output(tmp_path, monkeypatch):
     template = EspressoTemplate(binary="dos")
     template.outfiles = {"fildos": Path(RUN_PATH, "test.dos")}
     results = template.read_results(directory=".")
-    assert results["test.dos"]["fermi"] == pytest.approx(5.98)
+    assert results["test_dos"]["fermi"] == pytest.approx(5.98)
diff --git a/tests/core/recipes/espresso_recipes/test_dos.py b/tests/core/recipes/espresso_recipes/test_dos.py
index 6db4c8b7..12675026 100644
--- a/tests/core/recipes/espresso_recipes/test_dos.py
+++ b/tests/core/recipes/espresso_recipes/test_dos.py
@@ -21,7 +21,7 @@ def test_dos_job(tmp_path, monkeypatch):
     copy_decompress_files([DATA_DIR / "Si.upf.gz"], tmp_path)
     output = dos_job(tmp_path)
 
-    assert output["results"]["pwscf.dos"]["fermi"] == pytest.approx(7.199)
+    assert output["results"]["pwscf_dos"]["fermi"] == pytest.approx(7.199)
 
 
 def test_projwfc_job(tmp_path, monkeypatch):
@@ -85,7 +85,7 @@ def test_dos_flow(tmp_path, monkeypatch):
     assert output["non_scf_job"]["results"]["nbands"] == 8
     assert output["non_scf_job"]["results"]["nspins"] == 1
 
-    assert output["dos_job"]["results"]["pwscf.dos"]["fermi"] == pytest.approx(6.772)
+    assert output["dos_job"]["results"]["pwscf_dos"]["fermi"] == pytest.approx(6.772)
 
 
 def test_projwfc_flow(tmp_path, monkeypatch):
diff --git a/tests/core/recipes/espresso_recipes/test_phonons.py b/tests/core/recipes/espresso_recipes/test_phonons.py
index b865a602..1a3ae013 100644
--- a/tests/core/recipes/espresso_recipes/test_phonons.py
+++ b/tests/core/recipes/espresso_recipes/test_phonons.py
@@ -48,21 +48,18 @@ def test_phonon_job(tmp_path, monkeypatch):
 
     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"].get_positions(),
+        ph_results["results"][1]["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"].get_cell(),
-        atoms.get_cell(),
-        atol=1.0e-3,
+        ph_results["results"][1]["atoms"].get_cell(), atoms.get_cell(), atol=1.0e-3
     )
     assert_array_equal(
-        ph_results["results"][(0, 0, 0)]["atoms"].get_chemical_symbols(),
+        ph_results["results"][1]["atoms"].get_chemical_symbols(),
         atoms.get_chemical_symbols(),
     )
 
@@ -76,7 +73,7 @@ def test_phonon_job(tmp_path, monkeypatch):
     ]
 
     for key in sections:
-        assert key in ph_results["results"][(0, 0, 0)]
+        assert key in ph_results["results"][1]
 
     SETTINGS.ESPRESSO_PSEUDO = DEFAULT_SETTINGS.ESPRESSO_PSEUDO
 
@@ -115,21 +112,18 @@ def test_phonon_job_list_to_do(tmp_path, monkeypatch):
         nat_todo_indices=nat_todo,
     )
 
-    assert (0, 0, 0) in ph_results["results"]
     assert_allclose(
-        ph_results["results"][(0, 0, 0)]["atoms"].get_positions(),
+        ph_results["results"][1]["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"].get_cell(),
-        atoms.get_cell(),
-        atol=1.0e-3,
+        ph_results["results"][1]["atoms"].get_cell(), atoms.get_cell(), atol=1.0e-3
     )
     assert_array_equal(
-        ph_results["results"][(0, 0, 0)]["atoms"].get_chemical_symbols(),
+        ph_results["results"][1]["atoms"].get_chemical_symbols(),
         atoms.get_chemical_symbols(),
     )
 
@@ -143,6 +137,6 @@ def test_phonon_job_list_to_do(tmp_path, monkeypatch):
     ]
 
     for key in sections:
-        assert key in ph_results["results"][(0, 0, 0)]
+        assert key in ph_results["results"][1]
 
     SETTINGS.ESPRESSO_PSEUDO = DEFAULT_SETTINGS.ESPRESSO_PSEUDO
diff --git a/tests/core/recipes/mlp_recipes/test_core_recipes.py b/tests/core/recipes/mlp_recipes/test_core_recipes.py
index 3fca8596..89c78e0a 100644
--- a/tests/core/recipes/mlp_recipes/test_core_recipes.py
+++ b/tests/core/recipes/mlp_recipes/test_core_recipes.py
@@ -15,12 +15,12 @@ try:
 
 except ImportError:
     mace = None
-try:
-    import matgl
+# try:
+#     import matgl
 
-    methods.append("m3gnet")
-except ImportError:
-    matgl = None
+#     methods.append("m3gnet")
+# except ImportError:
+#     matgl = None
 try:
     import chgnet
 
diff --git a/tests/dask/test_espresso_recipes.py b/tests/dask/test_espresso_recipes.py
index 4ff185e4..06dd74b4 100644
--- a/tests/dask/test_espresso_recipes.py
+++ b/tests/dask/test_espresso_recipes.py
@@ -66,7 +66,7 @@ def test_phonon_grid_single(tmp_path, monkeypatch):
     ]
 
     for key in sections:
-        assert key in grid_results["results"][(0.1, 0, 0)]
+        assert key in grid_results["results"][1]
 
 
 def test_phonon_grid_single_gamma(tmp_path, monkeypatch):
@@ -108,7 +108,7 @@ def test_phonon_grid_single_gamma(tmp_path, monkeypatch):
     ]
 
     for key in sections:
-        assert key in grid_results["results"][(0, 0, 0)]
+        assert key in grid_results["results"][1]
 
 
 def test_phonon_grid_qplot(tmp_path, monkeypatch):
@@ -151,8 +151,8 @@ def test_phonon_grid_qplot(tmp_path, monkeypatch):
     ]
 
     for key in sections:
-        assert key in grid_results["results"][(0.1, 0, 0)]
-        assert key in grid_results["results"][(0.2, 0, 0)]
+        assert key in grid_results["results"][1]
+        assert key in grid_results["results"][2]
 
 
 def test_phonon_grid_disp(tmp_path, monkeypatch):
@@ -203,7 +203,7 @@ def test_phonon_grid_disp(tmp_path, monkeypatch):
     ]
 
     for key in sections:
-        assert key in grid_results["results"][(0, 0, 0)]
+        assert key in grid_results["results"][1]
 
 
 def test_phonon_grid_v2(tmp_path, monkeypatch):
@@ -244,4 +244,4 @@ def test_phonon_grid_v2(tmp_path, monkeypatch):
     ]
 
     for key in sections:
-        assert key in grid_results["results"][(0, 0, 0)]
+        assert key in grid_results["results"][1]