Repository: Quantum-Accelerators/quacc
Pull request #2676 (2025-02-24 20:01:50)

Add filter kwargs
## Summary of Changes

The ASE-based run_opt used uses a FrechetCellFilter. That class can take kwargs, for example applying a pressure during the relaxation. This PR simply adds a filter_kwargs to run_opt so these can flow down to the FrechetCellFilter. 

### Requirements

- [x] My PR is focused on a [single feature addition or bugfix](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/getting-started/best-practices-for-pull-requests#write-small-prs).
- [x] My PR has relevant, comprehensive [unit tests](https://quantum-accelerators.github.io/quacc/dev/contributing.html#unit-tests).
- [x] My PR is on a [custom branch](https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-and-deleting-branches-within-your-repository) (i.e. is _not_ named `main`).

Note: 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.


buildbot-princeton: Can one of the admins verify this patch?
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/Quantum-Accelerators/quacc/pull/2676?dropdown=coverage&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:
> Project coverage is 97.32%. Comparing base [(`93b5e0e`)](https://app.codecov.io/gh/Quantum-Accelerators/quacc/commit/93b5e0e4b028dd14086923eab49dbbfc7c2f7f86?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Quantum-Accelerators) to head [(`3490a8d`)](https://app.codecov.io/gh/Quantum-Accelerators/quacc/commit/3490a8dc7593461c1d6f3159bddb4212922e7b2e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Quantum-Accelerators).

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


```diff
@@           Coverage Diff           @@
##             main    #2676   +/-   ##
=======================================
  Coverage   97.32%   97.32%           
=======================================
  Files          90       90           
  Lines        3846     3847    +1     
=======================================
+ Hits         3743     3744    +1     
  Misses        103      103           
```

</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/Quantum-Accelerators/quacc/pull/2676?dropdown=coverage&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: @buildbot-princeton add to whitelist
Andrew-S-Rosen: @zulissimeta Thanks for the change! Can you briefly explain to me what the following is testing? If the `filter_kwargs` were not passed through, wouldn't the two expressions be equal and therefore the assertion would still hold `True`? I think I am misunderstanding the intention with the test.

```python
    assert (
        output_fire_pressure["results"]["energy"] - output_fire["results"]["energy"]
        < 0.1
    )
```
zulissimeta: > @zulissimeta Thanks for the change! Can you briefly explain to me what the following is testing? If the `filter_kwargs` were not passed through, wouldn't the two expressions be equal and therefore the assertion would still hold `True`? I think I am misunderstanding the intention with the test. Are you trying to do a `pytest.approx()` here or does the sign convention matter, or...?
> 
> ```python
>     assert (
>         output_fire_pressure["results"]["energy"] - output_fire["results"]["energy"]
>         < 0.1
>     )
> ```

Good catch! Edited to use volume (which should definitely be smaller when you apply pressure!).

Patch:
diff --git a/src/quacc/runners/ase.py b/src/quacc/runners/ase.py
index 74be4a1c..de05ceac 100644
--- a/src/quacc/runners/ase.py
+++ b/src/quacc/runners/ase.py
@@ -162,6 +162,7 @@ class Runner(BaseRunner):
         store_intermediate_results: bool = False,
         fn_hook: Callable | None = None,
         run_kwargs: dict[str, Any] | None = None,
+        filter_kwargs: dict[str, Any] | None = None,
     ) -> Dynamics:
         """
         This is a wrapper around the optimizers in ASE.
@@ -190,6 +191,8 @@ class Runner(BaseRunner):
             its only argument.
         run_kwargs
             Dictionary of kwargs for the `run()` method of the optimizer.
+        filter_kwargs
+            Dictionary of kwargs for the `FrechetCellFilter` if relax_cell is True.
 
         Returns
         -------
@@ -202,6 +205,7 @@ class Runner(BaseRunner):
             optimizer_kwargs,
         )
         run_kwargs = run_kwargs or {}
+        filter_kwargs = filter_kwargs or {}
         traj_filename = "opt.traj"
 
         # Check if trajectory kwarg is specified
@@ -227,7 +231,7 @@ class Runner(BaseRunner):
 
         # Set volume relaxation constraints, if relevant
         if relax_cell and self.atoms.pbc.any():
-            self.atoms = FrechetCellFilter(self.atoms)
+            self.atoms = FrechetCellFilter(self.atoms, **filter_kwargs)
 
         # Run optimization
         full_run_kwargs = {"fmax": fmax, "steps": max_steps, **run_kwargs}


Test patch:
diff --git a/tests/core/recipes/emt_recipes/test_emt_recipes.py b/tests/core/recipes/emt_recipes/test_emt_recipes.py
index b1472d7c..5c25989b 100644
--- a/tests/core/recipes/emt_recipes/test_emt_recipes.py
+++ b/tests/core/recipes/emt_recipes/test_emt_recipes.py
@@ -86,12 +86,33 @@ def test_relax_job(tmp_path, monkeypatch):
     atoms[0].position += [0.1, 0.1, 0.1]
     c = FixAtoms(indices=[0, 1])
     atoms.set_constraint(c)
-    output = relax_job(
+    output_fire = relax_job(
         atoms, opt_params={"fmax": 0.03, "optimizer": FIRE}, asap_cutoff=True
     )
-    assert output["nsites"] == len(atoms)
-    assert output["parameters"]["asap_cutoff"] is True
-    assert output["results"]["energy"] == pytest.approx(0.04996032884581858)
+    assert output_fire["nsites"] == len(atoms)
+    assert output_fire["parameters"]["asap_cutoff"] is True
+    assert output_fire["results"]["energy"] == pytest.approx(0.04996032884581858)
+
+    # Add a test that passes through kwargs to the FrechetCellFilter
+    atoms = bulk("Cu") * (2, 2, 2)
+    atoms[0].position += [0.1, 0.1, 0.1]
+    c = FixAtoms(indices=[0, 1])
+    atoms.set_constraint(c)
+    output_fire_pressure = relax_job(
+        atoms,
+        relax_cell=True,
+        opt_params={
+            "fmax": 0.03,
+            "optimizer": FIRE,
+            "filter_kwargs": {"scalar_pressure": 0.01},
+        },
+        asap_cutoff=True,
+    )
+
+    # Check that volume is smaller with a pressure applied than without!
+    assert output_fire_pressure["volume"] < output_fire["volume"]
+    assert output_fire_pressure["nsites"] == len(atoms)
+    assert output_fire_pressure["parameters"]["asap_cutoff"] is True
 
 
 def test_md_job1():