Skip to content

Commit 89d353e

Browse files
committed
Merge zhaiwenxi/master into master
Incorporate zhaiwenxi's CodeQL-fix PR (#49). Conflict resolution: - finetuner.py: keep our structural fix for the desc_cache <-> finetuner import cycle (load_or_extract / ensure_per_system_cache now live in finetuner.py) rather than their lazy `from dpa_adapt.data.desc_cache import load_or_extract`, which would ImportError since that symbol moved. Their swallowed-exception comments on the cache read/write paths are kept (auto-merged). - test_split_cv.py: keep our bare-skip stub, which fully removes the unused `systems` flagged by CodeQL; their variant deleted only the rng/n_total lines and left `systems` assigned-but-unused. cv.py merged cleanly: their unused-`n_splits` removal plus our redirect of the ensure_per_system_cache import to dpa_adapt.finetuner.
2 parents bf81301 + 6d15369 commit 89d353e

11 files changed

Lines changed: 25 additions & 16 deletions

File tree

dpa_adapt/cv.py

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -458,8 +458,6 @@ def cross_validate(
458458
train_groups -= test_formulas
459459
if val_groups and train_groups:
460460
fold_assignments.append((train_groups, val_groups))
461-
462-
n_splits = len(fold_assignments)
463461
else:
464462
# Deterministic GroupKFold: sort groups, split by index (no shuffle).
465463
# Reproducible given the same set of systems and groups.

dpa_adapt/data/smiles.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -362,9 +362,11 @@ def smiles_to_3d_coords(
362362
else:
363363
AllChem.UFFOptimizeMolecule(mol, maxIters=500)
364364
except Exception:
365+
# MMFF optimization failed; fall back to UFF.
365366
try:
366367
AllChem.UFFOptimizeMolecule(mol, maxIters=500)
367368
except Exception:
369+
# Even UFF failed — proceed with unoptimized conformer.
368370
pass
369371

370372
conf = mol.GetConformer()

dpa_adapt/finetuner.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,6 +1022,7 @@ def _extract_features_cached(self, systems: list[dpdata.System]) -> np.ndarray:
10221022
``self._extract_features()`` call below.
10231023
"""
10241024
try:
1025+
# Lazy import to avoid circular dependency: finetuner → desc_cache → finetuner.
10251026
from dpa_adapt.data.desc_cache import (
10261027
_cache_dir,
10271028
_cache_key,
@@ -1038,13 +1039,17 @@ def _extract_features_cached(self, systems: list[dpdata.System]) -> np.ndarray:
10381039
if cache_path.is_file():
10391040
return np.load(cache_path)
10401041
except Exception:
1042+
# Cache read failed (e.g. corrupted file, permissions) —
1043+
# fall through and recompute features from scratch.
10411044
pass
10421045

10431046
features = self._extract_features(systems)
10441047
try:
10451048
cache_path.parent.mkdir(parents=True, exist_ok=True)
10461049
np.save(cache_path, features)
10471050
except Exception:
1051+
# Cache write is best-effort — silently skip on permission errors
1052+
# or disk-full conditions; the features are already in memory.
10481053
pass
10491054
return features
10501055

dpa_adapt/mft.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,9 @@ def _validate_and_resolve_type_map(
261261
)
262262

263263
# Read elements from both datasets.
264+
# If data cannot be loaded (e.g. glob hasn't resolved yet, or the
265+
# data directory does not exist), fall back to empty lists — the
266+
# type_map will still be resolved from the checkpoint below.
264267
try:
265268
train_systems = load_data(train_data)
266269
except Exception:

dpa_adapt/predictor.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,8 @@ def fit(
194194
try:
195195
est[-1].set_params(random_state=seed)
196196
except ValueError:
197+
# Estimator does not support random_state (e.g. KNeighborsRegressor);
198+
# training with the default clone is fine — no ensemble diversity needed.
197199
pass
198200
est.fit(features, y_flat)
199201
self.estimators_.append(est)

source/tests/dpa_adapt/test_auto_convert.py

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,16 @@
55
annotations,
66
)
77

8+
from importlib.util import (
9+
find_spec,
10+
)
811
from pathlib import (
912
Path,
1013
)
1114

1215
import pytest
1316

14-
try:
15-
import rdkit # noqa: F401
16-
17-
_HAS_RDKIT = True
18-
except ImportError:
19-
_HAS_RDKIT = False
17+
_HAS_RDKIT = find_spec("rdkit") is not None
2018

2119
from dpa_adapt.data.convert import (
2220
_is_smiles_input,

source/tests/dpa_adapt/test_backend_contract.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,8 @@ class _HeavyContract:
120120

121121
def test_real_checkpoint_descriptor_shape(
122122
self,
123-
): ... # placeholder for future Bohrium-only tests
123+
): # placeholder for future Bohrium-only tests
124+
pass
124125

125126

126127
class _HookOwner:

source/tests/dpa_adapt/test_finetuner_strategies.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ def test_extract_features_detaches_grad_tensors_before_numpy(monkeypatch):
454454
import numpy as np
455455
import torch
456456

457-
import dpa_adapt.finetuner as finetuner_mod
457+
from dpa_adapt import finetuner as finetuner_mod
458458

459459
class FakeExtractor:
460460
def __init__(self, model):

source/tests/dpa_adapt/test_mft_evaluate.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,8 @@ def _fake_run(cmd, *args, **kwargs):
404404
cmd = captured["cmd"]
405405
f_idx = cmd.index("-f")
406406
datafile = cmd[f_idx + 1]
407-
lines = [l for l in open(datafile).read().split("\n") if l.strip()]
407+
with open(datafile) as f:
408+
lines = [l for l in f.read().split("\n") if l.strip()]
408409
assert lines == [test_data]
409410
assert out["mae"] == pytest.approx(7.0e-03)
410411
assert out["n_systems"] == 1
@@ -444,7 +445,8 @@ def _fake_run(cmd, *args, **kwargs):
444445

445446
cmd = captured["cmd"]
446447
datafile = cmd[cmd.index("-f") + 1]
447-
lines = [l for l in open(datafile).read().split("\n") if l.strip()]
448+
with open(datafile) as f:
449+
lines = [l for l in f.read().split("\n") if l.strip()]
448450
assert lines == paths
449451
assert out["n_systems"] == 4
450452

source/tests/dpa_adapt/test_trainer.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -525,7 +525,8 @@ def _capture(cmd, *args, **kwargs):
525525
f_idx = captured_cmd.index("-f")
526526
datafile = captured_cmd[f_idx + 1]
527527
assert os.path.isfile(datafile), f"datafile not written: {datafile}"
528-
lines = [l for l in open(datafile).read().split("\n") if l.strip()]
528+
with open(datafile) as f:
529+
lines = [l for l in f.read().split("\n") if l.strip()]
529530
assert len(lines) == 5, f"Expected 5 systems in datafile, got {len(lines)}"
530531

531532
assert out["mae"] == pytest.approx(0.01)

0 commit comments

Comments
 (0)