Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix release CI #161

Merged
merged 5 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ jobs:
uses: actions/checkout@v4

- name: Build wheels
uses: pypa/cibuildwheel@v2.16.5
uses: pypa/cibuildwheel@v2.18.1
env:
CIBW_BUILD: cp${{ matrix.python-version }}-*
CIBW_ARCHS_MACOS: universal2
Expand All @@ -74,10 +74,8 @@ jobs:
- name: Download build artifacts
uses: actions/download-artifact@v4
with:
name: artifact
path: dist
merge-multiple: true
pattern: dist-*

- name: Publish to PyPi or TestPyPI
uses: pypa/gh-action-pypi-publish@release/v1
Expand Down
6 changes: 3 additions & 3 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ default_install_hook_types: [pre-commit, commit-msg]

repos:
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.4.4
rev: v0.4.7
hooks:
- id: ruff
args: [--fix]
Expand All @@ -23,7 +23,7 @@ repos:
- id: trailing-whitespace

- repo: https://github.com/codespell-project/codespell
rev: v2.2.6
rev: v2.3.0
hooks:
- id: codespell
stages: [commit, commit-msg]
Expand All @@ -46,7 +46,7 @@ repos:
- svelte

- repo: https://github.com/pre-commit/mirrors-eslint
rev: v9.3.0
rev: v9.4.0
hooks:
- id: eslint
types: [file]
Expand Down
10 changes: 7 additions & 3 deletions chgnet/graph/graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,14 +127,18 @@ def __init__(self, nodes: list[Node]) -> None:
self.undirected_edges: dict[frozenset[int], list[UndirectedEdge]] = {}
self.undirected_edges_list: list[UndirectedEdge] = []

def add_edge(self, center_index, neighbor_index, image, distance) -> None:
def add_edge(
self, center_index, neighbor_index, image, distance, dist_tol: float = 1e-6
) -> None:
"""Add an directed edge to the graph.

Args:
center_index (int): center node index
neighbor_index (int): neighbor node index
image (np.array): the periodic cell image the neighbor is from
distance (float): distance between center and neighbor.
dist_tol (float): tolerance for distance comparison between edges.
Default = 1e-6
"""
# Create directed_edge (DE) index using the length of added DEs
directed_edge_index = len(self.directed_edges_list)
Expand Down Expand Up @@ -173,7 +177,7 @@ def add_edge(self, center_index, neighbor_index, image, distance) -> None:
# different image and distance (this is possible consider periodicity)
for undirected_edge in self.undirected_edges[tmp]:
if (
abs(undirected_edge.info["distance"] - distance) < 1e-6
abs(undirected_edge.info["distance"] - distance) < dist_tol
and len(undirected_edge.info["directed_edge_index"]) == 1
):
# There is an undirected edge with similar length and only one of
Expand Down Expand Up @@ -286,7 +290,7 @@ def line_graph_adjacency_list(self, cutoff) -> tuple[list[list[int]], list[int]]
# if encountered exception,
# it means after Atom_Graph creation, the UDE has only 1 DE associated
# This exception is not encountered from the develop team's experience
if len(u_edge.info["directed_edge_index"]) != 2:
if len(u_edge.info["directed_edge_index"]) != 2: # noqa: PLR2004
raise ValueError(
"Did not find 2 Directed_edges !!!"
f"undirected edge {u_edge} has:"
Expand Down
2 changes: 1 addition & 1 deletion chgnet/model/dynamics.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,7 @@ def relax(
dict[str, Structure | TrajectoryObserver]:
A dictionary with 'final_structure' and 'trajectory'.
"""
import ase.filters as filters
from ase import filters
from ase.filters import Filter

valid_filter_names = [
Expand Down
20 changes: 14 additions & 6 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ readme = "README.md"
license = { text = "Modified BSD" }
dependencies = [
"ase>=3.23.0",
"cython>=0.29.26",
"cython>=3",
"numpy>=1.26",
"nvidia-ml-py3>=7.352.0",
"pymatgen>=2023.10.11",
Expand Down Expand Up @@ -73,12 +73,11 @@ ignore = [
"ERA001", # found commented out code
"ISC001",
"NPY002", # TODO replace legacy np.random.seed
"PLR", # pylint refactor
"PLR0912", # too many branches
"PLR0913", # too many args in function def
"PLR0915", # too many statements
"PLW2901", # Outer for loop variable overwritten by inner assignment target
"PT006", # pytest-parametrize-names-wrong-type
"PT011", # pytest-raises-too-broad
"PT013", # pytest-incorrect-pytest-import
"PT019", # pytest-fixture-param-without-value
"PTH", # prefer Path to os.path
"S108",
"S301", # pickle can be unsafe
Expand All @@ -96,7 +95,16 @@ docstring-code-format = true

[tool.ruff.lint.per-file-ignores]
"site/*" = ["INP001", "S602"]
"tests/*" = ["ANN201", "D100", "D103", "FBT001", "FBT002", "INP001", "S101"]
"tests/*" = [
"ANN201",
"D100",
"D103",
"FBT001",
"FBT002",
"INP001",
"PLR2004",
"S101",
]
# E402 Module level import not at top of file
"examples/*" = ["E402", "I002", "INP001", "N816", "S101", "T201"]
"chgnet/**/*" = ["T201"]
Expand Down
3 changes: 2 additions & 1 deletion site/make_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@

# remove all files with less than 20 lines
# these correspond to mostly empty __init__.py files
if markdown.count("\n") < 20:
min_line_cnt = 20
if markdown.count("\n") < min_line_cnt:
os.remove(path)
continue

Expand Down
16 changes: 10 additions & 6 deletions tests/test_converter.py
Original file line number Diff line number Diff line change
@@ -1,22 +1,24 @@
from __future__ import annotations

from typing import Literal
from typing import TYPE_CHECKING, Literal

import pytest
from pymatgen.core import Lattice, Structure
from pytest import CaptureFixture

from chgnet.graph import CrystalGraph
from chgnet.graph.converter import CrystalGraphConverter

if TYPE_CHECKING:
from collections.abc import Generator

lattice = Lattice.cubic(4)
species = ["Na", "Cl"]
coords = [[0, 0, 0], [0.5, 0.5, 0.5]]
NaCl = Structure(lattice, species, coords)


@pytest.fixture()
def _set_make_graph() -> None:
def _set_make_graph() -> Generator[None, None, None]:
# fixture to force make_graph to be None and then restore it after test
from chgnet.graph import converter

Expand Down Expand Up @@ -63,7 +65,7 @@ def test_crystal_graph_converter_warns():

@pytest.mark.parametrize("on_isolated_atoms", ["ignore", "warn", "error"])
def test_crystal_graph_converter_forward(
on_isolated_atoms, capsys: CaptureFixture[str]
on_isolated_atoms, capsys: pytest.CaptureFixture[str]
):
atom_graph_cutoff = 5
converter = CrystalGraphConverter(
Expand All @@ -75,13 +77,15 @@ def test_crystal_graph_converter_forward(
strained.apply_strain(5)
graph_id = "strained"
err_msg = (
f"Structure {graph_id=} has 2 isolated atom(s) with "
f"Structure {graph_id=} has {len(NaCl)} isolated atom(s) with "
f"{atom_graph_cutoff=}. "
f"CHGNet calculation will likely go wrong"
)

if on_isolated_atoms == "error":
with pytest.raises(ValueError) as exc_info:
with pytest.raises(
ValueError, match=f"Structure {graph_id=} has {len(NaCl)} isolated atom"
) as exc_info:
converter.forward(strained, graph_id=graph_id)
assert err_msg in str(exc_info.value)
else:
Expand Down
4 changes: 1 addition & 3 deletions tests/test_encoders.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,11 +58,9 @@ def test_angle_encoder(num_angular: int, learnable: bool) -> None:

@pytest.mark.parametrize("num_angular", [-2, 8])
def test_angle_encoder_num_angular(num_angular: int) -> None:
with pytest.raises(ValueError) as exc_info:
with pytest.raises(ValueError, match=f"{num_angular=} must be an odd integer"):
AngleEncoder(num_angular=num_angular)

assert f"{num_angular=} must be an odd integer" in str(exc_info.value)


@pytest.mark.parametrize("learnable", [True, False])
def test_bond_encoder_learnable(learnable: bool) -> None:
Expand Down
2 changes: 1 addition & 1 deletion tests/test_graph.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def test_directed_edge() -> None:
info = {"image": np.zeros(3), "distance": 1}
edge = DirectedEdge([0, 1], index=0, info=info)
undirected = edge.make_undirected(index=0, info=info)
assert edge == edge
assert edge == edge # noqa: PLR0124
assert edge == undirected
assert edge.nodes == [0, 1]
assert edge.index == 0
Expand Down
2 changes: 1 addition & 1 deletion tests/test_md.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from pymatgen.analysis.structure_matcher import StructureMatcher
from pymatgen.core import Structure
from pymatgen.io.ase import AseAtomsAdaptor
from pytest import MonkeyPatch, approx
from pytest import MonkeyPatch, approx # noqa: PT013

from chgnet import ROOT
from chgnet.graph import CrystalGraphConverter
Expand Down
19 changes: 9 additions & 10 deletions tests/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
import numpy as np
import pytest
from pymatgen.core import Structure
from pytest import mark

from chgnet import ROOT
from chgnet.graph import CrystalGraphConverter
Expand All @@ -14,13 +13,13 @@
model = CHGNet.load()


@mark.parametrize("atom_fea_dim", [1, 64])
@mark.parametrize("bond_fea_dim", [1, 64])
@mark.parametrize("angle_fea_dim", [1, 64])
@mark.parametrize("num_radial", [1, 9])
@mark.parametrize("num_angular", [1, 9])
@mark.parametrize("n_conv", [1, 4])
@mark.parametrize("composition_model", ["MPtrj", "MPtrj_e", "MPF"])
@pytest.mark.parametrize("atom_fea_dim", [1, 64])
@pytest.mark.parametrize("bond_fea_dim", [1, 64])
@pytest.mark.parametrize("angle_fea_dim", [1, 64])
@pytest.mark.parametrize("num_radial", [1, 9])
@pytest.mark.parametrize("num_angular", [1, 9])
@pytest.mark.parametrize("n_conv", [1, 4])
@pytest.mark.parametrize("composition_model", ["MPtrj", "MPtrj_e", "MPF"])
def test_model(
atom_fea_dim: int,
bond_fea_dim: int,
Expand Down Expand Up @@ -118,8 +117,8 @@ def test_predict_structure() -> None:
assert out["atom_fea"].shape == (8, 64)


@mark.parametrize("axis", [[0, 0, 1], [1, 1, 0], [-2, 3, 1]])
@mark.parametrize("rotation_angle", [5, 30, 45, 120])
@pytest.mark.parametrize("axis", [[0, 0, 1], [1, 1, 0], [-2, 3, 1]])
@pytest.mark.parametrize("rotation_angle", [5, 30, 45, 120])
def test_predict_structure_rotated(rotation_angle: float, axis: list) -> None:
from pymatgen.transformations.standard_transformations import RotationTransformation

Expand Down
12 changes: 6 additions & 6 deletions tests/test_relaxation.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import torch
from ase.filters import ExpCellFilter, Filter, FrechetCellFilter
from pymatgen.core import Structure
from pytest import approx, mark, param

from chgnet.graph import CrystalGraphConverter
from chgnet.model import CHGNet, StructOptimizer
Expand Down Expand Up @@ -54,19 +53,20 @@ def test_relaxation(
assert len(traj) == 2 if algorithm == "legacy" else 4

# make sure final structure is more relaxed than initial one
assert traj.energies[-1] == approx(-58.94209, rel=1e-4)
assert traj.energies[-1] == pytest.approx(-58.94209, rel=1e-4)


no_cuda = mark.skipif(not torch.cuda.is_available(), reason="No CUDA device")
no_cuda = pytest.mark.skipif(not torch.cuda.is_available(), reason="No CUDA device")
# skip in macos-14 M1 CI due to OOM error (TODO investigate if
# PYTORCH_MPS_HIGH_WATERMARK_RATIO can fix)
no_mps = mark.skipif(
no_mps = pytest.mark.skipif(
not torch.backends.mps.is_available() or "CI" in os.environ, reason="No MPS device"
)


@mark.parametrize(
"use_device", ["cpu", param("cuda", marks=no_cuda), param("mps", marks=no_mps)]
@pytest.mark.parametrize(
"use_device",
["cpu", pytest.param("cuda", marks=no_cuda), pytest.param("mps", marks=no_mps)],
)
def test_structure_optimizer_passes_kwargs_to_model(use_device: str) -> None:
relaxer = StructOptimizer(use_device=use_device)
Expand Down
Loading