Skip to content

Commit

Permalink
ForceFieldRelaxMaker change relax_cell default to True (#635)
Browse files Browse the repository at this point in the history
fix use_symmetrized_structure type hint
  • Loading branch information
janosh authored Dec 6, 2023
1 parent 6287ce4 commit 86d70c6
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 25 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ default_language_version:
python: python3
repos:
- repo: https://github.com/charliermarsh/ruff-pre-commit
rev: v0.1.6
rev: v0.1.7
hooks:
- id: ruff
args: [--fix]
Expand Down
4 changes: 2 additions & 2 deletions src/atomate2/forcefields/flows/phonons.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from __future__ import annotations

from dataclasses import dataclass, field
from typing import TYPE_CHECKING
from typing import TYPE_CHECKING, Literal

from jobflow import Flow, Maker

Expand Down Expand Up @@ -130,7 +130,7 @@ class PhononMaker(Maker):
min_length: float | None = 20.0
prefer_90_degrees: bool = True
get_supercell_size_kwargs: dict = field(default_factory=dict)
use_symmetrized_structure: str | None = None
use_symmetrized_structure: Literal["primitive", "conventional"] | None = None
bulk_relax_maker: BaseVaspMaker | ForceFieldRelaxMaker | None = field(
default_factory=lambda: CHGNetRelaxMaker(relax_kwargs={"fmax": 0.00001})
)
Expand Down
20 changes: 10 additions & 10 deletions src/atomate2/forcefields/jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class ForceFieldRelaxMaker(Maker):
The job name.
force_field_name : str
The name of the force field.
relax_cell : bool
relax_cell : bool = True
Whether to allow the cell shape/volume to change during relaxation.
steps : int
Maximum number of ionic steps allowed during relaxation.
Expand All @@ -45,7 +45,7 @@ class ForceFieldRelaxMaker(Maker):

name: str = "Forcefield relax"
force_field_name: str = "Forcefield"
relax_cell: bool = False
relax_cell: bool = True
steps: int = 500
relax_kwargs: dict = field(default_factory=dict)
optimizer_kwargs: dict = field(default_factory=dict)
Expand Down Expand Up @@ -153,7 +153,7 @@ class CHGNetRelaxMaker(ForceFieldRelaxMaker):
----------
force_field_name : str
The name of the force field.
relax_cell : bool
relax_cell : bool = True
Whether to allow the cell shape/volume to change during relaxation.
steps : int
Maximum number of ionic steps allowed during relaxation.
Expand All @@ -167,7 +167,7 @@ class CHGNetRelaxMaker(ForceFieldRelaxMaker):

name: str = "CHGNet relax"
force_field_name = "CHGNet"
relax_cell: bool = False
relax_cell: bool = True
steps: int = 500
relax_kwargs: dict = field(default_factory=dict)
optimizer_kwargs: dict = field(default_factory=dict)
Expand Down Expand Up @@ -217,7 +217,7 @@ class M3GNetRelaxMaker(ForceFieldRelaxMaker):
The job name.
force_field_name : str
The name of the force field.
relax_cell : bool
relax_cell : bool = True
Whether to allow the cell shape/volume to change during relaxation.
steps : int
Maximum number of ionic steps allowed during relaxation.
Expand All @@ -231,7 +231,7 @@ class M3GNetRelaxMaker(ForceFieldRelaxMaker):

name: str = "M3GNet relax"
force_field_name: str = "M3GNet"
relax_cell: bool = False
relax_cell: bool = True
steps: int = 500
relax_kwargs: dict = field(default_factory=dict)
optimizer_kwargs: dict = field(default_factory=dict)
Expand Down Expand Up @@ -295,7 +295,7 @@ class MACERelaxMaker(ForceFieldRelaxMaker):
The job name.
force_field_name : str
The name of the force field.
relax_cell : bool
relax_cell : bool = True
Whether to allow the cell shape/volume to change during relaxation.
steps : int
Maximum number of ionic steps allowed during relaxation.
Expand All @@ -317,7 +317,7 @@ class MACERelaxMaker(ForceFieldRelaxMaker):

name: str = "MACE relax"
force_field_name: str = "MACE"
relax_cell: bool = False
relax_cell: bool = True
steps: int = 500
relax_kwargs: dict = field(default_factory=dict)
optimizer_kwargs: dict = field(default_factory=dict)
Expand Down Expand Up @@ -383,7 +383,7 @@ class GAPRelaxMaker(ForceFieldRelaxMaker):
The job name.
force_field_name : str
The name of the force field.
relax_cell : bool
relax_cell : bool = True
Whether to allow the cell shape/volume to change during relaxation.
steps : int
Maximum number of ionic steps allowed during relaxation.
Expand All @@ -403,7 +403,7 @@ class GAPRelaxMaker(ForceFieldRelaxMaker):

name: str = "GAP relax"
force_field_name: str = "GAP"
relax_cell: bool = False
relax_cell: bool = True
steps: int = 500
relax_kwargs: dict = field(default_factory=dict)
optimizer_kwargs: dict = field(default_factory=dict)
Expand Down
2 changes: 1 addition & 1 deletion src/atomate2/forcefields/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ def from_ase_compatible_result(
"""
trajectory = result["trajectory"].__dict__

# NOTE: units for stresses were converted from eV/Angstrom³ to kbar
# NOTE: units for stresses were converted from eV/Angstrom³ to kBar
# (* -1 from standard output)
# and to 3x3 matrix to comply with MP convention
for i in range(len(trajectory["stresses"])):
Expand Down
39 changes: 28 additions & 11 deletions tests/forcefields/test_jobs.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,22 +34,27 @@ def test_chgnet_static_maker(si_structure):
assert output1.output.n_steps == 1


def test_chgnet_relax_maker(si_structure):
@pytest.mark.parametrize("relax_cell", [True, False])
def test_chgnet_relax_maker(si_structure, relax_cell: bool):
# translate one atom to ensure a small number of relaxation steps are taken
si_structure.translate_sites(0, [0, 0, 0.1])

# generate job
job = CHGNetRelaxMaker(steps=25).make(si_structure)
job = CHGNetRelaxMaker(steps=25, relax_cell=relax_cell).make(si_structure)

# run the flow or job and ensure that it finished running successfully
responses = run_locally(job, ensure_success=True)

# validate job outputs
output1 = responses[job.uuid][1].output
assert isinstance(output1, ForceFieldTaskDocument)
assert output1.output.energy == approx(-10.6274, rel=1e-4)
assert output1.output.ionic_steps[-1].magmoms[0] == approx(0.00303572, rel=1e-4)
assert output1.output.n_steps >= 12
if relax_cell:
assert output1.output.energy == approx(-10.62461, abs=1e-2)
assert output1.output.ionic_steps[-1].magmoms[0] == approx(0.002964, rel=1e-1)
else:
assert output1.output.energy == approx(-10.6274, rel=1e-2)
assert output1.output.ionic_steps[-1].magmoms[0] == approx(0.00303572, rel=1e-2)


def test_m3gnet_static_maker(si_structure):
Expand Down Expand Up @@ -82,7 +87,7 @@ def test_m3gnet_relax_maker(si_structure):
output1 = responses[job.uuid][1].output
assert isinstance(output1, ForceFieldTaskDocument)
assert output1.output.energy == approx(-10.8, abs=0.2)
assert output1.output.n_steps == 14
assert output1.output.n_steps == 27


mace_paths = pytest.mark.parametrize(
Expand Down Expand Up @@ -115,8 +120,9 @@ def test_mace_static_maker(si_structure, test_dir, model):
assert output1.output.n_steps == 1


@pytest.mark.parametrize("relax_cell", [True, False])
@mace_paths
def test_mace_relax_maker(si_structure, test_dir, model):
def test_mace_relax_maker(si_structure, test_dir, model, relax_cell: bool):
# translate one atom to ensure a small number of relaxation steps are taken
si_structure.translate_sites(0, [0, 0, 0.1])

Expand All @@ -126,6 +132,7 @@ def test_mace_relax_maker(si_structure, test_dir, model):
model=model,
steps=25,
optimizer_kwargs={"optimizer": "BFGSLineSearch"},
relax_cell=relax_cell,
).make(si_structure)

# run the flow or job and ensure that it finished running successfully
Expand All @@ -134,8 +141,12 @@ def test_mace_relax_maker(si_structure, test_dir, model):
# validating the outputs of the job
output1 = responses[job.uuid][1].output
assert isinstance(output1, ForceFieldTaskDocument)
assert output1.output.energy == approx(-0.051912, rel=1e-4)
assert output1.output.n_steps == 4
if relax_cell:
assert output1.output.energy == approx(-0.071052, rel=1e-1)
assert output1.output.n_steps >= 5
else:
assert output1.output.energy == approx(-0.051912, rel=1e-4)
assert output1.output.n_steps == 4


def test_gap_static_maker(si_structure, test_dir):
Expand All @@ -161,7 +172,8 @@ def test_gap_static_maker(si_structure, test_dir):
assert output1.output.n_steps == 1


def test_gap_relax_maker(si_structure, test_dir):
@pytest.mark.parametrize("relax_cell", [True, False])
def test_gap_relax_maker(si_structure, test_dir, relax_cell: bool):
importorskip("quippy")

# translate one atom to ensure a small number of relaxation steps are taken
Expand All @@ -172,6 +184,7 @@ def test_gap_relax_maker(si_structure, test_dir):
job = GAPRelaxMaker(
potential_param_file_name=test_dir / "forcefields" / "gap" / "gap_file.xml",
steps=25,
relax_cell=relax_cell,
).make(si_structure)

# run the flow or job and ensure that it finished running successfully
Expand All @@ -180,5 +193,9 @@ def test_gap_relax_maker(si_structure, test_dir):
# validating the outputs of the job
output1 = responses[job.uuid][1].output
assert isinstance(output1, ForceFieldTaskDocument)
assert output1.output.energy == approx(-10.8523, rel=1e-4)
assert output1.output.n_steps == 17
if relax_cell:
assert output1.output.energy == approx(-13.08492, rel=1e-2)
assert output1.output.n_steps == 27
else:
assert output1.output.energy == approx(-10.8523, rel=1e-4)
assert output1.output.n_steps == 17

0 comments on commit 86d70c6

Please sign in to comment.