Skip to content

Commit

Permalink
Add custom convergence options and --converge maxiter to `Geometric…
Browse files Browse the repository at this point in the history
…Procedure` (#314)

* updating documentation and field options to allow custom criteria

* adding validator for custom convergence criteria

* adding validator to resolve conflicts between convergence_set and converge

* remove unused convergence options from final optimization keyword dict

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* changing to is instance

* adding unit test for custom convergence options

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* trying to fix test

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* trying to fix test

* actually fixing test

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* one last update to test to ensure it only converges due to maxiter

* Fix docs builds

* Use `plotly =5` in examples

* Test now parses GeomeTRIC output to ensure convergence options are passed

* moving geometric procedure validation error tests to their own file

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* adding custom exceptions for Geometric convergence validation errors

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Lint

* Update dependencies

* Let docs build broken

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Matthew W. Thompson <mattwthompson@protonmail.com>
  • Loading branch information
3 people authored Feb 11, 2025
1 parent 637c823 commit dd0a93c
Show file tree
Hide file tree
Showing 8 changed files with 575 additions and 8 deletions.
3 changes: 1 addition & 2 deletions devtools/conda-envs/basic.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -39,14 +39,13 @@ dependencies:
# this should be changed to openff-toolkit-base

- openff-toolkit
- openff-units >=0.2.1
- openff-units ~=0.2.1
- pydantic >1.10.17,<3
- pyyaml
- torsiondrive
- basis_set_exchange
- typing-extensions
- h5py>=3.6.0
- psycopg2 # for qcfractal to pip-install without issue

# Optional
- openmmforcefields
Expand Down
1 change: 1 addition & 0 deletions devtools/conda-envs/docs.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ dependencies:
# Standard dependencies

- openff-toolkit-base
- openff-units =0.2
- rdkit
- pydantic
- pyyaml
Expand Down
4 changes: 1 addition & 3 deletions devtools/conda-envs/psi4.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ dependencies:
### Core dependencies.

- openff-toolkit-base
- openff-units >=0.2.1
- openff-units ~=0.2.1
- rdkit
- pydantic >1.10.17,<3
- pyyaml
Expand All @@ -50,6 +50,4 @@ dependencies:
- openff-fragmenter-base >=0.2.0
- openmm

- pint=0.21
- openff-units=0.2.1
- postgresql
197 changes: 197 additions & 0 deletions openff/qcsubmit/_tests/test_procedures.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,197 @@
import pytest

from openff.qcsubmit._pydantic import ValidationError
from openff.qcsubmit.procedures import GeometricProcedure


@pytest.mark.parametrize(
"convergence_set",
[
pytest.param(
"GAU",
id="Default convergence_set value",
),
pytest.param(
"GAU_VERYTIGHT",
id="Very tight convergence_set value",
),
pytest.param(
"TURBOMOLE",
id="Turbomole convergence_set value",
),
],
)
def test_convergence_set(convergence_set):
"""
Test that the convergence_set is set correctly.
"""
procedure = GeometricProcedure(convergence_set=convergence_set)
assert procedure.convergence_set == convergence_set
assert procedure.get_optimzation_keywords()["convergence_set"] == convergence_set
assert "converge" not in procedure.get_optimzation_keywords()


@pytest.mark.parametrize(
"conv_keywords",
[
pytest.param(
(
"CUSTOM",
[
"energy",
"1e-8",
],
),
id="Just energy",
),
pytest.param(
(
"CUSTOM",
[
"grms",
"3e-4",
"energy",
"1e-6",
"drms",
"1.2e-3",
"gmax",
"4.5e-4",
"dmax",
"1.8e-3",
],
),
id="All specified, out of order",
),
pytest.param(
(
"CUSTOM",
["grms", "3e-4", "energy", "1e-6"],
),
id="Two specified",
),
],
)
def test_custom_converge(conv_keywords):
"""
Test that the custom convergence criteria is set correctly.
"""
convergence_set, converge = conv_keywords
procedure = GeometricProcedure(convergence_set=convergence_set, converge=converge)
assert procedure.converge == converge
assert procedure.get_optimzation_keywords()["converge"] == converge
assert "convergence_set" not in procedure.get_optimzation_keywords()


@pytest.mark.parametrize(
"conv_keywords",
[
pytest.param(
("CUSTOM", ["energy", "1e-8", "maxiter"], 3),
id="Custom + maxiter",
),
pytest.param(
("GAU_VERYTIGHT", ["maxiter"], 3),
id="Regular + maxiter",
),
],
)
def test_converge_maxiter(conv_keywords):
"""
Test that --converge maxiter works with both custom convergence options and convergence_set.
"""
convergence_set, converge, maxiter = conv_keywords
procedure = GeometricProcedure(
convergence_set=convergence_set, converge=converge, maxiter=maxiter
)
assert procedure.converge == converge
assert procedure.get_optimzation_keywords()["converge"] == converge
assert procedure.convergence_set == convergence_set
if convergence_set != "CUSTOM":
assert (
procedure.get_optimzation_keywords()["convergence_set"] == convergence_set
)
else:
assert "convergence_set" not in procedure.get_optimzation_keywords()
assert procedure.maxiter == maxiter
assert procedure.get_optimzation_keywords()["maxiter"] == maxiter


@pytest.mark.parametrize(
"converge",
[
pytest.param(
"GAU",
),
pytest.param(
["energy", 1e-8],
),
pytest.param(["hello", "1e-8"]),
pytest.param(["energy", "grms", "maxiter"]),
pytest.param(["energy", "1e-8", "maxiter", "5"]),
pytest.param(["energy", "hello"]),
pytest.param(["GAU", "maxiter"]),
],
)
def test_converge_validation_errors(converge):
"""
Make sure that invalid values of converge raise a ValidationError
"""
with pytest.raises(ValidationError):
GeometricProcedure(converge=converge)


@pytest.mark.parametrize(
"convergence_set",
[
pytest.param(
"HELLO",
),
pytest.param("GAU maxiter"),
],
)
def test_convergence_set_validation_errors(convergence_set):
"""
Make sure that invalid values of convergence_set raise a ValidationError
"""
with pytest.raises(ValidationError):
GeometricProcedure(convergence_set=convergence_set)


def test_convergence_set_attribute_error():
"""
Make sure that an AttributeError is raised if the user tries to set a converge list as convergence_set
"""
with pytest.raises(AttributeError):
GeometricProcedure(convergence_set=["energy", "1e-8"])


@pytest.mark.parametrize(
"conv_keywords",
[
pytest.param(
(
"CUSTOM",
["maxiter"],
),
id="Custom but just maxiter",
),
pytest.param(
("CUSTOM", []),
id="Custom with empty list",
),
pytest.param(
(
"GAU",
["energy", "1e-8"],
),
id="Regular convergence_set but custom converge also provided",
),
],
)
def test_both_validation_errors(conv_keywords):
"""
Make sure that if incompatible values of converge and convergence_set are set, a ValidationError is raised
"""
convergence_set, converge = conv_keywords
with pytest.raises(ValidationError):
GeometricProcedure(convergence_set=convergence_set, converge=converge)
Loading

0 comments on commit dd0a93c

Please sign in to comment.