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

add ruff formatter #266

Merged
merged 10 commits into from
May 14, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
37 changes: 23 additions & 14 deletions .ci_support/release.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,19 @@
def get_setup_version_and_pattern(setup_content):
depend_lst, version_lst = [], []
for l in setup_content:
if '==' in l:
lst = l.split('[')[-1].split(']')[0].replace(' ', '').replace('"', '').replace("'", '').split(',')
if "==" in l:
lst = (
l.split("[")[-1]
.split("]")[0]
.replace(" ", "")
.replace('"', "")
.replace("'", "")
.split(",")
)
for dep in lst:
if dep != '\n':
version_lst.append(dep.split('==')[1])
depend_lst.append(dep.split('==')[0])
if dep != "\n":
version_lst.append(dep.split("==")[1])
depend_lst.append(dep.split("==")[0])
Comment on lines 3 to +16
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace the ambiguous variable name l with a more descriptive name like line.

-    for l in setup_content:
-        if "==" in l:
-            lst = (
-                l.split("[")[-1]
-                .split("]")[0]
-                .replace(" ", "")
-                .replace('"', "")
-                .replace("'", "")
-                .split(",")
-            )
+    for line in setup_content:
+        if "==" in line:
+            lst = (
+                line.split("[")[-1]
+                .split("]")[0]
+                .replace(" ", "")
+                .replace('"', "")
+                .replace("'", "")
+                .split(",")
+            )

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
for l in setup_content:
if '==' in l:
lst = l.split('[')[-1].split(']')[0].replace(' ', '').replace('"', '').replace("'", '').split(',')
if "==" in l:
lst = (
l.split("[")[-1]
.split("]")[0]
.replace(" ", "")
.replace('"', "")
.replace("'", "")
.split(",")
)
for dep in lst:
if dep != '\n':
version_lst.append(dep.split('==')[1])
depend_lst.append(dep.split('==')[0])
if dep != "\n":
version_lst.append(dep.split("==")[1])
depend_lst.append(dep.split("==")[0])
for line in setup_content:
if "==" in line:
lst = (
line.split("[")[-1]
.split("]")[0]
.replace(" ", "")
.replace('"', "")
.replace("'", "")
.split(",")
)
for dep in lst:
if dep != "\n":
version_lst.append(dep.split("==")[1])
depend_lst.append(dep.split("==")[0])


version_high_dict = {d: v for d, v in zip(depend_lst, version_lst)}
return version_high_dict
Expand All @@ -16,14 +23,14 @@ def get_env_version(env_content):
read_flag = False
depend_lst, version_lst = [], []
for l in env_content:
if 'dependencies:' in l:
if "dependencies:" in l:
read_flag = True
elif read_flag:
lst = l.replace('-', '').replace(' ', '').replace('\n', '').split("=")
lst = l.replace("-", "").replace(" ", "").replace("\n", "").split("=")
if len(lst) == 2:
depend_lst.append(lst[0])
version_lst.append(lst[1])
return {d:v for d, v in zip(depend_lst, version_lst)}
return {d: v for d, v in zip(depend_lst, version_lst)}
Comment on lines +26 to +33
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace the ambiguous variable name l with a more descriptive name like line.

-    for l in env_content:
-        if "dependencies:" in l:
+    for line in env_content:
+        if "dependencies:" in line:
-            lst = l.replace("-", "").replace(" ", "").replace("\n", "").split("=")
+            lst = line.replace("-", "").replace(" ", "").replace("\n", "").split("=")

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if "dependencies:" in l:
read_flag = True
elif read_flag:
lst = l.replace('-', '').replace(' ', '').replace('\n', '').split("=")
lst = l.replace("-", "").replace(" ", "").replace("\n", "").split("=")
if len(lst) == 2:
depend_lst.append(lst[0])
version_lst.append(lst[1])
return {d:v for d, v in zip(depend_lst, version_lst)}
return {d: v for d, v in zip(depend_lst, version_lst)}
for line in env_content:
if "dependencies:" in line:
read_flag = True
elif read_flag:
lst = line.replace("-", "").replace(" ", "").replace("\n", "").split("=")
if len(lst) == 2:
depend_lst.append(lst[0])
version_lst.append(lst[1])
return {d: v for d, v in zip(depend_lst, version_lst)}



def update_dependencies(setup_content, version_low_dict, version_high_dict):
Expand All @@ -35,27 +42,29 @@ def update_dependencies(setup_content, version_low_dict, version_high_dict):
version_combo_dict[dep] = dep + "==" + ver

setup_content_new = ""
pattern_dict = {d:d + "==" + v for d, v in version_high_dict.items()}
pattern_dict = {d: d + "==" + v for d, v in version_high_dict.items()}
for l in setup_content:
for k, v in pattern_dict.items():
if v in l:
l = l.replace(v, version_combo_dict[k])
setup_content_new +=l
setup_content_new += l
Comment on lines +45 to +50
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Replace the ambiguous variable name l with a more descriptive name like line.

-    for l in setup_content:
+    for line in setup_content:
-            if v in l:
-                l = l.replace(v, version_combo_dict[k])
-        setup_content_new += l
+            if v in line:
+                line = line.replace(v, version_combo_dict[k])
+        setup_content_new += line

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
pattern_dict = {d: d + "==" + v for d, v in version_high_dict.items()}
for l in setup_content:
for k, v in pattern_dict.items():
if v in l:
l = l.replace(v, version_combo_dict[k])
setup_content_new +=l
setup_content_new += l
pattern_dict = {d: d + "==" + v for d, v in version_high_dict.items()}
for line in setup_content:
for k, v in pattern_dict.items():
if v in line:
line = line.replace(v, version_combo_dict[k])
setup_content_new += line

return setup_content_new


if __name__ == "__main__":
with open('pyproject.toml', "r") as f:
with open("pyproject.toml", "r") as f:
setup_content = f.readlines()

with open('environment.yml', "r") as f:
with open("environment.yml", "r") as f:
env_content = f.readlines()

setup_content_new = update_dependencies(
setup_content=setup_content[2:],
version_low_dict=get_env_version(env_content=env_content),
version_high_dict=get_setup_version_and_pattern(setup_content=setup_content[2:]),
version_high_dict=get_setup_version_and_pattern(
setup_content=setup_content[2:]
),
)

with open('pyproject.toml', "w") as f:
with open("pyproject.toml", "w") as f:
f.writelines("".join(setup_content[:2]) + setup_content_new)
10 changes: 10 additions & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
repos:
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.4.4
hooks:
- id: ruff
name: ruff lint
args: ["--fix"]
files: ^atomistics/
- id: ruff-format
name: ruff format
28 changes: 28 additions & 0 deletions atomistics/calculators/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,31 @@
)
except ImportError:
pass


__all__ = [
calc_molecular_dynamics_langevin_with_ase,
calc_molecular_dynamics_npt_with_ase,
calc_molecular_dynamics_thermal_expansion_with_ase,
calc_static_with_ase,
evaluate_with_ase,
optimize_positions_with_ase,
optimize_positions_and_volume_with_ase,
evaluate_with_hessian,
calc_static_with_qe,
evaluate_with_qe,
optimize_positions_and_volume_with_qe,
calc_molecular_dynamics_thermal_expansion_with_lammps,
calc_molecular_dynamics_nph_with_lammps,
calc_molecular_dynamics_npt_with_lammps,
calc_molecular_dynamics_nvt_with_lammps,
calc_molecular_dynamics_langevin_with_lammps,
calc_static_with_lammps,
evaluate_with_lammps,
evaluate_with_lammps_library,
get_potential_dataframe,
get_potential_by_name,
optimize_positions_and_volume_with_lammps,
optimize_positions_with_lammps,
calc_molecular_dynamics_phonons_with_lammps,
]
2 changes: 1 addition & 1 deletion atomistics/calculators/interface.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
from enum import Enum
from typing import NewType, Union, Any, TYPE_CHECKING
from typing import Union, Any, TYPE_CHECKING

# best would be StrEnum from py3.11
import sys
Expand Down
17 changes: 17 additions & 0 deletions atomistics/calculators/lammps/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,20 @@
)
except ImportError:
pass


__all__ = [
calc_molecular_dynamics_thermal_expansion_with_lammps,
calc_molecular_dynamics_nph_with_lammps,
calc_molecular_dynamics_npt_with_lammps,
calc_molecular_dynamics_nvt_with_lammps,
calc_molecular_dynamics_langevin_with_lammps,
calc_static_with_lammps,
evaluate_with_lammps,
evaluate_with_lammps_library,
optimize_positions_and_volume_with_lammps,
optimize_positions_with_lammps,
get_potential_dataframe,
get_potential_by_name,
calc_molecular_dynamics_phonons_with_lammps,
]
8 changes: 4 additions & 4 deletions atomistics/calculators/lammps/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ def lammps_run(
lmp.interactive_lib_command(c)

if input_template is not None:
for l in input_template.split("\n"):
lmp.interactive_lib_command(l)
for line in input_template.split("\n"):
lmp.interactive_lib_command(line)

return lmp

Expand Down Expand Up @@ -131,8 +131,8 @@ def lammps_thermal_expansion_loop(
Pstop=Pstop,
Pdamp=Pdamp,
)
for l in run_str_rendered.split("\n"):
lmp_instance.interactive_lib_command(l)
for line in run_str_rendered.split("\n"):
lmp_instance.interactive_lib_command(line)
volume_md_lst.append(lmp_instance.interactive_volume_getter())
temperature_md_lst.append(lmp_instance.interactive_temperatures_getter())
lammps_shutdown(lmp_instance=lmp_instance, close_instance=lmp is None)
Expand Down
10 changes: 5 additions & 5 deletions atomistics/calculators/lammps/potential.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,18 +290,18 @@ def update_potential_paths(
]
potential_dict = {os.path.basename(f): f for f in potential_file_path_lst}
potential_commands = []
for l in row.Config:
l = l.replace("\n", "")
for line in row.Config:
line = line.replace("\n", "")
for key, value in potential_dict.items():
l = l.replace(key, value)
potential_commands.append(l)
line = line.replace(key, value)
potential_commands.append(line)
config_lst.append(potential_commands)
df_pot["Config"] = config_lst
return df_pot


def get_resource_path_from_conda(
env_variables: tuple[str] = ("CONDA_PREFIX", "CONDA_DIR")
env_variables: tuple[str] = ("CONDA_PREFIX", "CONDA_DIR"),
) -> str:
env = os.environ
for conda_var in env_variables:
Expand Down
4 changes: 1 addition & 3 deletions atomistics/calculators/wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,17 +5,15 @@

from __future__ import annotations

from typing import NewType, Union, Any, TYPE_CHECKING
from typing import TYPE_CHECKING

from atomistics.calculators.interface import TaskEnum, TaskOutputEnum

if TYPE_CHECKING:
from ase import Atoms
from atomistics.calculators.interface import (
TaskName,
TaskSpec,
TaskDict,
TaskResults,
ResultsDict,
SimpleEvaluator,
)
Expand Down
12 changes: 12 additions & 0 deletions atomistics/workflows/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,15 @@
from atomistics.workflows.quasiharmonic import QuasiHarmonicWorkflow
except ImportError:
pass


__all__ = [
ElasticMatrixWorkflow,
EnergyVolumeCurveWorkflow,
LangevinWorkflow,
calc_molecular_dynamics_thermal_expansion,
optimize_positions,
optimize_positions_and_volume,
PhonopyWorkflow,
QuasiHarmonicWorkflow,
]
5 changes: 3 additions & 2 deletions atomistics/workflows/evcurve/fit.py
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ def fit_leastsq(
"""
# http://stackoverflow.com/questions/14581358/getting-standard-errors-on-fitted-parameters-using-the-optimize-leastsq-method-i

errfunc = lambda p, x, y, fittype: fitfunction(p, x, fittype) - y
def errfunc(p, x, y, fittype):
return fitfunction(p, x, fittype) - y
Comment on lines +138 to +139
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a docstring and type hints to the errfunc function for better readability and maintainability.

def errfunc(p: tuple[float], x: np.ndarray, y: np.ndarray, fittype: str) -> np.ndarray:
    """
    Calculate the error between the fitted function and the actual data.

    Args:
        p (tuple[float]): Fit parameters.
        x (np.ndarray): Volumes.
        y (np.ndarray): Energies.
        fittype (str): Type of fit.

    Returns:
        np.ndarray: Error between the fitted function and the actual data.
    """
    return fitfunction(p, x, fittype) - y


pfit, pcov, infodict, errmsg, success = scipy.optimize.leastsq(
errfunc, p0, args=(datax, datay, fittype), full_output=1, epsfcn=0.0001
Expand All @@ -153,7 +154,7 @@ def fit_leastsq(
for i in range(len(pfit)):
try:
error.append(np.absolute(pcov[i][i]) ** 0.5)
except:
except TypeError:
Comment on lines 155 to +157
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improve the error handling for the pcov calculation by specifying the exception type.

except TypeError:
    error.append(0.00)

error.append(0.00)
pfit_leastsq = pfit
perr_leastsq = np.array(error)
Expand Down
2 changes: 1 addition & 1 deletion atomistics/workflows/evcurve/thermo.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ def get_minimum_energy_path(self, pressure: np.ndarray = None) -> np.ndarray:

"""
if pressure is not None:
raise NotImplemented()
raise NotImplementedError()
v_min_lst = []
for c in self._coeff.T:
v_min = np.roots(np.polyder(c, 1))
Expand Down
1 change: 0 additions & 1 deletion atomistics/workflows/evcurve/workflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
OutputThermodynamic,
)
from atomistics.workflows.evcurve.helper import (
get_volume_lst,
generate_structures_helper,
analyse_structures_helper,
)
Expand Down
1 change: 0 additions & 1 deletion atomistics/workflows/phonons/units.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import scipy.constants
from phonopy.units import VaspToTHz, THzToEv, EvTokJmol

kJ_mol_to_eV = 1000 / scipy.constants.Avogadro / scipy.constants.electron_volt
kb = scipy.constants.physical_constants["Boltzmann constant in eV/K"][0]
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@
setup(
version=versioneer.get_version(),
cmdclass=versioneer.get_cmdclass(),
)
)
13 changes: 5 additions & 8 deletions tests/test_ase_interface/test_ase_md_mace.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,7 @@ class TestASEMD(unittest.TestCase):
def test_ase_langevin(self):
structure = bulk("Al", cubic=True).repeat([2, 2, 2])
ase_calculator = mace_mp(
model="medium",
dispersion=False,
default_dtype="float32",
device='cpu'
model="medium", dispersion=False, default_dtype="float32", device="cpu"
)
result_dict = calc_molecular_dynamics_langevin_with_ase(
structure=structure,
Expand All @@ -37,9 +34,9 @@ def test_ase_langevin(self):
self.assertEqual(result_dict["velocities"].shape, (10, 32, 3))
self.assertEqual(result_dict["cell"].shape, (10, 3, 3))
self.assertEqual(result_dict["forces"].shape, (10, 32, 3))
self.assertEqual(result_dict["temperature"].shape, (10, ))
self.assertEqual(result_dict["energy_pot"].shape, (10, ))
self.assertEqual(result_dict["energy_tot"].shape, (10, ))
self.assertEqual(result_dict["temperature"].shape, (10,))
self.assertEqual(result_dict["energy_pot"].shape, (10,))
self.assertEqual(result_dict["energy_tot"].shape, (10,))
self.assertEqual(result_dict["pressure"].shape, (10, 3, 3))
self.assertTrue(result_dict["temperature"][-1] > 25)
self.assertTrue(result_dict["temperature"][-1] < 75)
self.assertTrue(result_dict["temperature"][-1] < 75)
8 changes: 4 additions & 4 deletions tests/test_ase_interface/test_ase_md_matgl.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ def test_ase_langevin(self):
self.assertEqual(result_dict["velocities"].shape, (10, 32, 3))
self.assertEqual(result_dict["cell"].shape, (10, 3, 3))
self.assertEqual(result_dict["forces"].shape, (10, 32, 3))
self.assertEqual(result_dict["temperature"].shape, (10, ))
self.assertEqual(result_dict["energy_pot"].shape, (10, ))
self.assertEqual(result_dict["energy_tot"].shape, (10, ))
self.assertEqual(result_dict["temperature"].shape, (10,))
self.assertEqual(result_dict["energy_pot"].shape, (10,))
self.assertEqual(result_dict["energy_tot"].shape, (10,))
self.assertEqual(result_dict["pressure"].shape, (10, 3, 3))
self.assertTrue(result_dict["temperature"][-1] > 25)
self.assertTrue(result_dict["temperature"][-1] < 75)
self.assertTrue(result_dict["temperature"][-1] < 75)
20 changes: 11 additions & 9 deletions tests/test_ase_interface/test_elastic_ase_gpaw.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,20 @@ def test_calc_elastic(self):
num_of_point=5,
eps_range=0.05,
sqrt_eta=True,
fit_order=2
fit_order=2,
)
task_dict = workflow.generate_structures()
result_dict = evaluate_with_ase(
task_dict=task_dict,
ase_calculator=GPAW(
xc="PBE",
mode=PW(300),
kpts=(3, 3, 3)
)
ase_calculator=GPAW(xc="PBE", mode=PW(300), kpts=(3, 3, 3)),
)
elastic_dict = workflow.analyse_structures(output_dict=result_dict)
self.assertTrue(np.isclose(elastic_dict["elastic_matrix"][0, 0], 125.66807354, atol=1e-04))
self.assertTrue(np.isclose(elastic_dict["elastic_matrix"][0, 1], 68.41418321, atol=1e-04))
self.assertTrue(np.isclose(elastic_dict["elastic_matrix"][3, 3], 99.29916329, atol=1e-04))
self.assertTrue(
np.isclose(elastic_dict["elastic_matrix"][0, 0], 125.66807354, atol=1e-04)
)
self.assertTrue(
np.isclose(elastic_dict["elastic_matrix"][0, 1], 68.41418321, atol=1e-04)
)
self.assertTrue(
np.isclose(elastic_dict["elastic_matrix"][3, 3], 99.29916329, atol=1e-04)
)
Comment on lines +34 to +42
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding more assertions to verify the correctness of the elastic matrix calculations.

self.assertTrue(
    np.isclose(elastic_dict["elastic_matrix"][0, 0], 125.66807354, atol=1e-04)
)
self.assertTrue(
    np.isclose(elastic_dict["elastic_matrix"][0, 1], 68.41418321, atol=1e-04)
)
self.assertTrue(
    np.isclose(elastic_dict["elastic_matrix"][3, 3], 99.29916329, atol=1e-04)
)
# Additional assertions
self.assertTrue(
    np.allclose(
        elastic_dict["elastic_matrix"],
        np.array([[125.66807354, 68.41418321, 0, 0, 0, 0],
                  [68.41418321, 125.66807354, 0, 0, 0, 0],
                  [0, 0, 99.29916329, 0, 0, 0],
                  [0, 0, 0, 99.29916329, 0, 0],
                  [0, 0, 0, 0, 99.29916329, 0],
                  [0, 0, 0, 0, 0, 99.29916329]]),
        atol=1e-4
    )
)

Loading
Loading