Skip to content

Commit 5767f84

Browse files
authored
Fix WalltimeHandler crash on incomplete OUTCAR + new test (#412)
* Fix WalltimeHandler crash on incomplete OUTCAR + new test * fix lints * update legacy ruff pre-commit hook name
1 parent eb3e60b commit 5767f84

9 files changed

Lines changed: 94 additions & 69 deletions

File tree

.pre-commit-config.yaml

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ repos:
1010
- repo: https://github.com/astral-sh/ruff-pre-commit
1111
rev: v0.14.7
1212
hooks:
13-
- id: ruff
13+
- id: ruff-check
1414
args: [--fix, --unsafe-fixes]
1515
- id: ruff-format
1616

@@ -30,15 +30,15 @@ repos:
3030
rev: v2.4.1
3131
hooks:
3232
- id: codespell
33-
stages: [ commit-msg ]
34-
exclude_types: [ html ]
35-
additional_dependencies: [ tomli ] # needed to read pyproject.toml below py3.11
33+
stages: [commit-msg]
34+
exclude_types: [html]
35+
additional_dependencies: [tomli] # needed to read pyproject.toml below py3.11
3636

3737
- repo: https://github.com/MarcoGorelli/cython-lint
3838
rev: v0.18.1
3939
hooks:
4040
- id: cython-lint
41-
args: [ --no-pycodestyle ]
41+
args: [--no-pycodestyle]
4242
- id: double-quote-cython-strings
4343

4444
- repo: https://github.com/adamchainz/blacken-docs
@@ -55,13 +55,13 @@ repos:
5555
# MD033: no inline HTML
5656
# MD041: first line in a file should be a top-level heading
5757
# MD025: single title
58-
args: [ --disable, MD013, MD024, MD025, MD033, MD041, "--" ]
58+
args: [--disable, MD013, MD024, MD025, MD033, MD041, '--']
5959

6060
- repo: https://github.com/kynan/nbstripout
6161
rev: 0.8.2
6262
hooks:
6363
- id: nbstripout
64-
args: [ --drop-empty-cells, --keep-output ]
64+
args: [--drop-empty-cells, --keep-output]
6565

6666
- repo: https://github.com/RobertCraigie/pyright-python
6767
rev: v1.1.407

pyproject.toml

Lines changed: 60 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,15 @@ authors = [
1818
]
1919
maintainers = [{ name = "Shyue Ping Ong" }]
2020
readme = "README.md"
21-
keywords = ["jit", "job", "just-in-time", "management", "vasp", "nwchem", "qchem"]
21+
keywords = [
22+
"jit",
23+
"job",
24+
"just-in-time",
25+
"management",
26+
"nwchem",
27+
"qchem",
28+
"vasp",
29+
]
2230
classifiers = [
2331
"Development Status :: 5 - Production/Stable",
2432
"Intended Audience :: Science/Research",
@@ -38,8 +46,10 @@ requires-python = ">=3.10"
3846
dependencies = ["monty>=2.0.6", "psutil", "ruamel.yaml>=0.15.6"]
3947

4048
[project.optional-dependencies]
41-
matsci = ["pymatgen"] # Error handlers and jobs for materials simulations, e.g., VASP, Nwchem, qchem, etc.
42-
gaussian = ["pymatgen", "matplotlib"]
49+
matsci = [
50+
"pymatgen",
51+
] # Error handlers and jobs for materials simulations, e.g., VASP, Nwchem, qchem, etc.
52+
gaussian = ["matplotlib", "pymatgen"]
4353
error-statistics = ["sentry-sdk>=0.8.0"]
4454

4555
[project.scripts]
@@ -63,55 +73,56 @@ line-length = 120
6373

6474
[tool.ruff.lint]
6575
select = [
66-
"B", # flake8-bugbear
67-
"C4", # flake8-comprehensions
68-
"D", # pydocstyle
69-
"E", # pycodestyle error
70-
"EXE", # flake8-executable
71-
"F", # pyflakes
72-
"FA", # flake8-future-annotations
73-
"FLY", # flynt
74-
"I", # isort
75-
"ICN", # flake8-import-conventions
76-
"ISC", # flake8-implicit-str-concat
77-
"PD", # pandas-vet
76+
"B", # flake8-bugbear
77+
"C4", # flake8-comprehensions
78+
"D", # pydocstyle
79+
"E", # pycodestyle error
80+
"EXE", # flake8-executable
81+
"F", # pyflakes
82+
"FA", # flake8-future-annotations
83+
"FLY", # flynt
84+
"I", # isort
85+
"ICN", # flake8-import-conventions
86+
"ISC", # flake8-implicit-str-concat
87+
"PD", # pandas-vet
7888
"PERF", # perflint
79-
"PIE", # flake8-pie
80-
"PL", # pylint
81-
"PT", # flake8-pytest-style
82-
"PYI", # flakes8-pyi
83-
"Q", # flake8-quotes
84-
"RET", # flake8-return
85-
"RSE", # flake8-raise
86-
"RUF", # Ruff-specific rules
87-
"SIM", # flake8-simplify
89+
"PIE", # flake8-pie
90+
"PL", # pylint
91+
"PT", # flake8-pytest-style
92+
"PYI", # flakes8-pyi
93+
"Q", # flake8-quotes
94+
"RET", # flake8-return
95+
"RSE", # flake8-raise
96+
"RUF", # Ruff-specific rules
97+
"SIM", # flake8-simplify
8898
"SLOT", # flake8-slots
89-
"TCH", # flake8-type-checking
90-
"TID", # tidy imports
91-
"TID", # flake8-tidy-imports
92-
"UP", # pyupgrade
93-
"W", # pycodestyle warning
94-
"YTT", # flake8-2020
99+
"TCH", # flake8-type-checking
100+
"TID", # tidy imports
101+
"TID", # flake8-tidy-imports
102+
"UP", # pyupgrade
103+
"W", # pycodestyle warning
104+
"YTT", # flake8-2020
95105
]
96106
ignore = [
97-
"B023", # Function definition does not bind loop variable
98-
"B028", # No explicit stacklevel keyword argument found
99-
"B904", # Within an except clause, raise exceptions with ...
100-
"C408", # unnecessary-collection-call
107+
"B023", # Function definition does not bind loop variable
108+
"B028", # No explicit stacklevel keyword argument found
109+
"B904", # Within an except clause, raise exceptions with ...
110+
"C408", # unnecessary-collection-call
101111
"COM812",
102-
"D105", # Missing docstring in magic method
103-
"D205", # 1 blank line required between summary line and description
104-
"D212", # Multi-line docstring summary should start at the first line
112+
"D105", # Missing docstring in magic method
113+
"D205", # 1 blank line required between summary line and description
114+
"D212", # Multi-line docstring summary should start at the first line
105115
"ISC001",
106-
"PD011", # pandas-use-of-dot-values
107-
"PD901", # pandas-df-variable-name
116+
"PD011", # pandas-use-of-dot-values
117+
"PD901", # pandas-df-variable-name
108118
"PERF203", # try-except-in-loop
109-
"PLR", # pylint refactor
119+
"PLC0415", # import-outside-top-level (used for performance/optional deps)
120+
"PLR", # pylint refactor
110121
"PLW2901", # Outer for loop variable overwritten by inner assignment target
111-
"PT013", # pytest-incorrect-pytest-import
122+
"PT013", # pytest-incorrect-pytest-import
112123
"PTH",
113-
"RUF012", # Disable checks for mutable class args
114-
"SIM105", # Use contextlib.suppress(OSError) instead of try-except-pass
124+
"RUF012", # Disable checks for mutable class args
125+
"SIM105", # Use contextlib.suppress(OSError) instead of try-except-pass
115126
]
116127
pydocstyle.convention = "google"
117128
isort.split-on-trailing-comma = false
@@ -163,22 +174,18 @@ exclude = ["**/tests"]
163174

164175
[dependency-groups]
165176
dev = [
177+
"invoke>=2.2.0",
178+
"mypy>=1.15.0",
179+
"myst-parser>=4.0.1",
166180
"pre-commit>=4.2.0",
167181
"pymatgen>=2025.5.16",
168-
"pytest>=8.3.5",
169182
"pytest-cov>=6.0.0",
170-
"mypy>=1.15.0",
183+
"pytest>=8.3.5",
171184
"ruff>=0.11.2",
172-
"invoke>=2.2.0",
173-
"sphinx>=8.1.3",
174-
"myst-parser>=4.0.1",
175185
"sphinx-markdown-builder>=0.6.8",
186+
"sphinx>=8.1.3",
176187
]
177-
lint = [
178-
"pre-commit>=4.2.0",
179-
"mypy>=1.15.0",
180-
"ruff>=0.11.2",
181-
]
188+
lint = ["mypy>=1.15.0", "pre-commit>=4.2.0", "ruff>=0.11.2"]
182189

183190
[tool.setuptools.package-data]
184191
custodian = ["py.typed"]

src/custodian/utils.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ class tracked_lru_cache:
7272
Allows Custodian to clear the cache after all the checks have been performed.
7373
"""
7474

75-
cached_functions: ClassVar = set()
75+
cached_functions: ClassVar[set] = set()
7676

7777
def __init__(self, func) -> None:
7878
"""

src/custodian/vasp/handlers.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1800,7 +1800,10 @@ def check(self, directory="./") -> bool:
18001800
if self.wall_time:
18011801
run_time = datetime.datetime.now() - self.start_time
18021802
total_secs = run_time.total_seconds()
1803-
outcar = load_outcar(os.path.join(directory, "OUTCAR"))
1803+
try:
1804+
outcar = load_outcar(os.path.join(directory, "OUTCAR"))
1805+
except Exception: # Can't perform check if Outcar not valid (e.g. file being written)
1806+
return False
18041807
if not self.electronic_step_stop:
18051808
# Determine max time per ionic step.
18061809
outcar.read_pattern({"timings": r"LOOP\+.+real time(.+)"}, postprocess=float)

tests/qchem/test_jobs.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -778,7 +778,7 @@ def test_OptFF(self) -> None:
778778
QCInput.from_file(f"{TEST_DIR}/5690_frag18/mol.qin.freq_2").as_dict()
779779
== QCInput.from_file(os.path.join(SCR_DIR, "mol.qin")).as_dict()
780780
)
781-
with pytest.raises(ValueError, match="ERROR: Can't deal with multiple neg frequencies yet! Exiting..."):
781+
with pytest.raises(ValueError, match=r"ERROR: Can't deal with multiple neg frequencies yet! Exiting\.\.\."):
782782
next(job)
783783

784784

tests/vasp/test_handlers.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1016,6 +1016,24 @@ def test_check_and_correct(self) -> None:
10161016
assert content == "LABORT = .TRUE."
10171017
os.remove("STOPCAR")
10181018

1019+
def test_check_with_malformed_outcar(self, tmp_path: Path) -> None:
1020+
"""Test that WalltimeHandler.check() returns False on malformed OUTCAR.
1021+
1022+
This can happen when VASP is mid-write and the file contains incomplete
1023+
data like a standalone '-' instead of a float. See
1024+
https://github.com/materialsproject/pymatgen/issues/2251
1025+
"""
1026+
os.chdir(tmp_path)
1027+
1028+
# Create a malformed OUTCAR that would cause parsing to fail
1029+
with open("OUTCAR", "w") as file:
1030+
file.write("Free energy of the ion-electron system (eV)\n")
1031+
file.write(" alpha Z PSCENC = -\n") # incomplete value
1032+
1033+
handler = WalltimeHandler(wall_time=3600, buffer_time=120)
1034+
# Should return False (not crash) when OUTCAR parsing fails
1035+
assert handler.check() is False
1036+
10191037
@classmethod
10201038
def tearDown(cls) -> None:
10211039
os.environ.pop("CUSTODIAN_WALLTIME_START", None)

tests/vasp/test_io.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@
99
@pytest.fixture(autouse=True)
1010
def _clear_tracked_cache() -> None:
1111
"""Clear the cache of the stored functions between the tests."""
12-
from custodian.utils import tracked_lru_cache
13-
1412
tracked_lru_cache.tracked_cache_clear()
1513

1614

tests/vasp/test_jobs.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ def test_terminate_exception_during_graceful_termination(self):
305305
self.mock_process.terminate.side_effect = OSError("Permission denied")
306306

307307
# Act & Assert
308-
with pytest.raises(OSError):
308+
with pytest.raises(OSError, match="Permission denied"):
309309
self.vasp_job.terminate()
310310

311311
self.mock_process.terminate.assert_called_once()
@@ -323,7 +323,7 @@ def test_terminate_exception_during_force_kill(self):
323323
self.mock_process.kill.return_value = None
324324

325325
# Act & Assert
326-
with pytest.raises(OSError):
326+
with pytest.raises(OSError, match="Process not found"):
327327
self.vasp_job.terminate()
328328

329329
self.mock_process.terminate.assert_called_once()

tests/vasp/test_validators.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,14 @@
33

44
import pytest
55

6+
from custodian.utils import tracked_lru_cache
67
from custodian.vasp.validators import VaspAECCARValidator, VaspFilesValidator, VaspNpTMDValidator, VasprunXMLValidator
78
from tests.conftest import TEST_FILES
89

910

1011
@pytest.fixture(autouse=True)
1112
def _clear_tracked_cache() -> None:
1213
"""Clear the cache of the stored functions between the tests."""
13-
from custodian.utils import tracked_lru_cache
14-
1514
tracked_lru_cache.tracked_cache_clear()
1615

1716

0 commit comments

Comments
 (0)