add DPA-ADAPT toolkit for downstream property adaptation#5572
add DPA-ADAPT toolkit for downstream property adaptation#5572zhaiwenxi wants to merge 212 commits into
Conversation
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
feat: add DeePMD property tools
for more information, see https://pre-commit.ci
Add property tools
dpa_tools merge
…t, unify --target-key
…utput parsing - DPAFineTuner: extract _FrozenSklearnPipeline helper; keep public API unchanged - MFTFineTuner: defer _read_fitting_net_from_ckpt to first access - DPATrainer._parse_test_output: single anchored regex per metric, auto-detect format
…perty metrics - _load_labels: accept str | list[str], stack columns for multi-property - build_sklearn_head: n_outputs param, wrap RF/Ridge with MultiOutputRegressor - evaluate: per-property mae/rmse/r2 dict when target_key is a list - freeze/DPAPredictor: store and load target_key as-is (str or list) - CLI: --target-key homo,lumo parsed via _maybe_split_list - 6 new tests covering fit, evaluate, freeze/load round-trip
The old _load_descriptor_model, _validate_type_map, _remap_atom_types, _extract_features_cached, and _extract_features method bodies were left in place alongside the new thin delegators, causing CodeQL 'variable defined multiple times' warnings. Removed the old bodies; kept _extract_features_cached on DPAFineTuner directly so that test patches on DPAFineTuner._extract_features are honoured through the cache wrapper.
… method - Replace try/except ImportError in _unwrap_multioutput with direct import (sklearn is always available when dpa_tools is loaded) - Remove _FrozenSklearnPipeline.extract_features_cached (dead code; the caching wrapper lives on DPAFineTuner so test patches work)
The workflow still referenced the deleted deepmd_property_tools/ directory. Updated paths trigger to deepmd/dpa_tools/** and test command to source/tests/dpa_tools/. Added torch to lightweight dependencies.
numpy 2.3+ requires Python>=3.11, but the property_tools_tests workflow runs on Python 3.10. Pin numpy>=1.21,<2.2 to keep the lightweight dependency install working on older Python.
refactor: unify dpa_tools CLI/API and merge deepmd_property_tools
The standalone dpa-adapt CLI mixed print() with the existing _LOG logger;
ruff's T201 ("print found") flagged 26 print() calls. Route all output
through _LOG (info/warning/error) to match the handlers that already use
it and the project-wide ban on print().
Organize DPA-ADAPT docs navigation
Fix DPA adapt cache and CLI edge cases
style(dpa-adapt): route CLI output through logger to satisfy ruff T201
Fix dpa_adapt pre-commit hook failures
…k accumulator Add type annotations across the dpa_adapt library (ANN), replace print() with logging in example scripts (T201), annotate mutable class defaults with ClassVar (RUF012), replace legacy np.random.rand with Generator API (NPY002), escape regex metacharacters in pytest match patterns (RUF043), fix implicit Optional annotations (RUF013), ambiguous unicode (RUF002), zip() strict= (B905), docstring formatting (D301/D400), dict() → literal (C408), and TC003 import placement. Also fix _DescriptorExtraction._resolve_descriptor_hook_model to prefer atomic_model over dp_model. dp_model delegates set_eval_descriptor_hook and eval_descriptor to atomic_model but lacks the eval_descriptor_list attribute, so _clear_accumulator was a no-op. Descriptors from systems with different atom counts accumulated across forward passes, causing torch.concat to fail with "Expected size 5 but got size 4".
fix(dpa-adapt): resolve pre-commit ruff errors and descriptor hook accumulator
The disallow-caps pre-commit hook flags "DeepMD" as improper capitalization; the official project name is DeePMD.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| - boxes : (n_frames, 9) or None for non-periodic | ||
| - atom_types : (n_atoms,) int | ||
|
|
| np.save(cache_path, descriptors) | ||
| _LOG.info("Cached descriptors to %s", cache_path) | ||
|
|
| cache: bool = True, | ||
| type_map: list[str] | tuple[str, ...] | None = None, | ||
| ) -> np.ndarray: |
fix: address CodeQL scan findings - File not closed: wrap open() in with statement (test_trainer.py, test_mft_evaluate.py) - Unused variable: remove dead n_total, n_splits assignments (test_split_cv.py, cv.py) - Self-import: remove circular import from self (test_validate.py) - Unused import: replace import rdkit with importlib.util.find_spec (test_auto_convert.py) - Empty except: add explanatory comments where exceptions are intentionally suppressed (predictor.py, mft.py, finetuner.py, smiles.py) - Statement has no effect: replace ... with pass (test_backend_contract.py) - Mixed import styles: use consistent from-import for module (test_finetuner_strategies.py) - Cyclic import: add comments explaining lazy import pattern (finetuner.py) @
Move load_or_extract() and ensure_per_system_cache() from dpa_adapt.data.desc_cache to dpa_adapt.finetuner. Those two functions need DPAFineTuner, while finetuner imports cache helpers from desc_cache, creating a CodeQL-flagged cyclic import. desc_cache.py now contains only pure cache-path and fingerprint helpers; the extraction+backfill functions live next to DPAFineTuner in finetuner.py. Updated imports in cv.py and test_cache.py accordingly.
fix: address CodeQL scan findings
for more information, see https://pre-commit.ci
|
|
||
| def test_deterministic_folds_same_result_twice(self, tmp_path, monkeypatch): | ||
| formulas = [f"Comp{i}" for i in range(4)] | ||
| systems = _write_oer_tree(str(tmp_path), formulas, nsets=2, label_key="energy") |
test_deterministic_folds_same_result_twice unconditionally skips (needs a real DPA checkpoint), but still ran stub setup code whose `systems` local was flagged by CodeQL as unused. Reduce it to a bare skip, matching the sibling test_manifest_folds.
- input_formats: drop the manual "1./2./3." heading prefixes that doubled with Sphinx auto-numbering (e.g. "9.2.1. 1. SMILES Tables (CSV)"). - conf.py: cap auto-generated CLI reference section numbering at depth 5 via a doctree-resolved hook, so sphinx-argparse's deep subcommand nesting no longer renders numbers like "9.3.3.6.3.1.1.". Scoped to the dpa_adapt/cli page only; other pages and the global TOC are untouched.
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.
njzjz-bot
left a comment
njzjz-bot
left a comment
There was a problem hiding this comment.
Thanks for the latest round of cleanup. I re-checked the current head (6d1536952eab04be32da0d7c3d43ae5984b6ece3): the packaging metadata, docs navigation, reduced example dataset, descriptor cache identity, centralized dp lookup, type-map remapping, and CodeQL fixes are all in much better shape now. The current CodeQL and main CI checks are green.
I still see a few blockers before this is safe to merge:
-
The public
DPAFineTuner(strategy="mft")path still breaks default MFT type-map auto-detection.DPAFineTuner.__init__converts an omittedtype_mapfromNoneto[], then_ensure_mft()passes that empty list intoMFTFineTuner. However,MFTFineTuneronly auto-detects the checkpoint type map whenself.type_map is None;[]is treated as user-provided. As a result, the normal public/CLI path without--mft-type-mapcan either fail validation as “provided type_map is missing elements”, or emit an empty sharedtype_mapif data loading is deferred/failed. Please preserveNonefor the MFT delegate, or make MFT treat an empty list like omitted. -
MFT training still has inconsistent checkpoint locations.
fit()writesmft_input.jsonunderoutput_dir, but launchesdp --pt trainwithoutcwd=self.output_dir; the generated MFT config also does not settraining.save_ckpt. Later_freeze_ckpt()expectsmodel.ckpt-<max_steps>.ptunderself.output_dir. This means training can succeed while freeze/evaluate/predict fails because the checkpoint was written to the caller’s current directory/default location. Please either restore runningdp trainwithcwd=output_dir, or add an explicitsave_ckptunderoutput_dir(ideally both), with a regression test. -
There is still a stray root-level test file.
tests/test_dpa_tools.pyremains tracked at the repository root. The main Python workflow only runspytest ... source/tests, so this file is not covered by CI; the sdist excludes remove/source/tests,/doc,/examples, etc., but not/tests, so this test may also leak into source distributions. Please move it undersource/tests/dpa_adapt/if it is still needed, or delete it if it is superseded by the existing DPA-ADAPT tests.
One smaller validation gap: DPATrainer._validate_fparam() indexes shape[1] directly, so a malformed 1-D fparam.npy raises a raw IndexError instead of the intended DPADataError; it should check ndim first and preferably also preflight row count against coord.npy.
Authored by OpenClaw 2026.6.8 (844f405) (model: custom-chat-jinzhezeng-group/gpt-5.5)
njzjz-bot
left a comment
njzjz-bot
left a comment
There was a problem hiding this comment.
Adding inline versions of the current blockers so they are easier to fix at the relevant lines.
— OpenClaw 2026.6.8 (844f405), model: custom-chat-jinzhezeng-group/gpt-5.5
| self._mft = None | ||
|
|
||
| # ---- backward-compat state mirrors (delegated to pipeline) ---- | ||
| if self.type_map is None: |
There was a problem hiding this comment.
This None → [] normalization breaks the MFT default path. For strategy="mft", _ensure_mft() passes self.type_map into MFTFineTuner; [] is treated as user-provided, so _validate_and_resolve_type_map() skips checkpoint auto-detection and validates against an empty map. Please preserve None for MFT, or normalize only inside the frozen-sklearn path after dispatch.
— OpenClaw 2026.6.8 (844f405), model: custom-chat-jinzhezeng-group/gpt-5.5
| }, | ||
| }, | ||
| "numb_steps": t.max_steps, | ||
| "save_freq": t.save_freq, |
There was a problem hiding this comment.
MFT should also set an explicit checkpoint prefix, e.g. training["save_ckpt"] = os.path.join(t.output_dir, "model.ckpt") (matching DPATrainer). Right now only save_freq is emitted, so DeePMD writes model.ckpt-* relative to the process cwd while _freeze_ckpt() later looks under self.output_dir, making fit() succeed but evaluate()/predict() fail to find the checkpoint.
— OpenClaw 2026.6.8 (844f405), model: custom-chat-jinzhezeng-group/gpt-5.5
| _LOG.info("Log: %s", log_path) | ||
|
|
||
| with open(log_path, "w") as log_f: | ||
| process = subprocess.Popen( |
There was a problem hiding this comment.
This subprocess still inherits the caller's cwd. Because the MFT code writes mft_input.json/train.log under output_dir and later freezes from output_dir, training should run with a deterministic cwd (or every generated path in the JSON must be absolute, especially training.save_ckpt). Otherwise users calling from another directory get checkpoints in the wrong place.
— OpenClaw 2026.6.8 (844f405), model: custom-chat-jinzhezeng-group/gpt-5.5
| @@ -207,6 +217,7 @@ sdist.exclude = [ | |||
| ] | |||
There was a problem hiding this comment.
This exclude list still misses the repository-level /tests directory, while tests/test_dpa_tools.py is still tracked at the PR head. Since CI only runs source/tests, please either move/remove that root test or add /tests to sdist.exclude; otherwise the sdist keeps shipping an unrun, stale test tree.
— OpenClaw 2026.6.8 (844f405), model: custom-chat-jinzhezeng-group/gpt-5.5
| f"fparam.npy of shape (n_frames, {fparam_dim})." | ||
| ) | ||
| shape = np.load(fpath).shape | ||
| if shape[1] != fparam_dim: |
There was a problem hiding this comment.
Please guard the dimensionality before indexing shape[1]. A 1-D fparam.npy currently raises a bare IndexError here instead of the advertised DPADataError; this should check len(shape) == 2 and also verify shape[0] matches the frame count in the corresponding set.
— OpenClaw 2026.6.8 (844f405), model: custom-chat-jinzhezeng-group/gpt-5.5
fix(dpa-adapt): resolve pre-commit errors, desc_cache import cycle, descriptor hook accumulator
7 participants
Summary
This PR adds DPA-ADAPT, a toolkit for adapting pretrained DPA models to downstream atomistic property prediction tasks.
The new package provides a scikit-learn-style Python API and standalone CLI for fine-tuning, descriptor extraction, prediction, evaluation, cross-validation, and data preparation, without requiring users to manually write DeePMD-kit training input files.
Main changes
dpa_adaptPython package.dpa-adaptdpaadfrozen_sklearn: frozen DPA descriptors with scikit-learn regressorsfrozen_head: train a property head on top of a frozen DPA backbonefinetune: end-to-end DPA fine-tuningmft: multi-task fine-tuning with auxiliary energy/force trainingfparam.npydoc/dpa_adapt/.examples/dpa_adapt/.dpa-adaptoptional dependencies inpyproject.toml.source/tests/dpa_adapt/.Co-authored-by: zirenjin <zirenjin@umich.edu>
Summary by CodeRabbit