Skip to content

fix(infer): normalize parameter shorthand before batching#5857

Open
njzjz-bot wants to merge 2 commits into
deepmodeling:masterfrom
njzjz-bot:fix/deepeval-param-shorthand-backends-5666
Open

fix(infer): normalize parameter shorthand before batching#5857
njzjz-bot wants to merge 2 commits into
deepmodeling:masterfrom
njzjz-bot:fix/deepeval-param-shorthand-backends-5666

Conversation

@njzjz-bot

@njzjz-bot njzjz-bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Closes #5666.

Summary

  • centralize the documented fparam and aparam size checks and tiling rules
  • normalize inputs before AutoBatchSize in the PyTorch, JAX, TF2, and Paddle backend adapters
  • share the same helper with the public input wrapper while preserving its flattened atomic-parameter ABI
  • cover both normal and spin backend routes because PyTorch/Paddle normalize before selecting their route
  • add direct backend regressions with two frames and a forced one-frame batch size
  • normalize PyTorch eval_embedding before batching, which also fixes its eval_descriptor and eval_fitting_last_layer wrappers

Why existing tests missed this

Existing multi-frame inference tests call the high-level DeepPot.eval wrapper, whose _standard_input already expands shorthand before backend dispatch. Backend-direct consistency tests used canonical full shapes, while many backend-specific fixtures use only one frame, where shorthand sizes can coincide with full sizes. Spin inference fixtures also use zero-dimensional fparam/aparam. TF2/JAX coverage runs only in the TF2-only job, and Paddle was absent from the cross-backend IO matrix.

The regressions deliberately call .deep_eval.eval and the PyTorch backend's .eval_embedding to bypass public normalization, supplies (dim_fparam,), (natoms, dim_aparam), and (dim_aparam,), and forces auto_batch_size=natoms so each frame is evaluated separately. This catches both direct reshape failures and accidental slicing of the atom axis.

Validation

  • pytest source/tests/common/test_deep_eval_parameter_shorthand.py -q (7 passed)
  • TF2/JAX TestDeepPotFparamAparam.test_deep_eval with backend-direct forced batching (passed)
  • Paddle is now included in the same consistency path when its dependency is available
  • PyTorch TestEmbeddingDeepEvalAPI (6 passed), including shared fparam, per-atom aparam, and scalar aparam through forced one-frame batching
  • ruff format . (1664 files unchanged on final pass)
  • ruff check . (passed)
  • git diff --check (passed)

The pre-existing optional PyTorch C++ extension still has an ABI mismatch with the current Torch package, so local tests disabled loading that unrelated .so; the pure PyTorch backend-direct embedding suite ran successfully.

Coding agent: Codex
Codex version: codex-cli 0.144.4
Model: gpt-5.6-sol
Reasoning effort: xhigh

Summary by CodeRabbit

  • New Features

    • Added consistent support for shorthand frame and atomic parameter inputs across evaluation backends, including embedding evaluation.
    • Parameter inputs are now normalized to the evaluation-ready shape automatically.
  • Bug Fixes

    • Improved handling of parameter shapes across frames and atoms.
    • Invalid shorthand parameter sizes are now rejected with clear validation errors.
    • Shorthand inputs are guaranteed to match results from explicitly expanded inputs.
  • Tests

    • Added new unit tests for frame/atomic shorthand normalization and invalid-shape rejection.
    • Expanded IO and embedding tests to verify cross-backend consistency and shorthand-vs-expanded equivalence.

@dosubot dosubot Bot added the bug label Jul 17, 2026
@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 1989cefa-5157-457a-9fa6-12686a54ecc3

📥 Commits

Reviewing files that changed from the base of the PR and between 2149d24 and bbfc000.

📒 Files selected for processing (8)
  • deepmd/infer/deep_eval.py
  • deepmd/jax/infer/deep_eval.py
  • deepmd/pd/infer/deep_eval.py
  • deepmd/pt/infer/deep_eval.py
  • deepmd/tf2/infer/deep_eval.py
  • source/tests/common/test_deep_eval_parameter_shorthand.py
  • source/tests/consistent/io/test_io.py
  • source/tests/pt/model/test_embedding.py
🚧 Files skipped from review as they are similar to previous changes (6)
  • deepmd/jax/infer/deep_eval.py
  • deepmd/pd/infer/deep_eval.py
  • source/tests/common/test_deep_eval_parameter_shorthand.py
  • deepmd/tf2/infer/deep_eval.py
  • source/tests/consistent/io/test_io.py
  • deepmd/infer/deep_eval.py

📝 Walkthrough

Walkthrough

DeepEval gains shared validation and expansion for fparam and aparam shorthand inputs. JAX, Paddle, PyTorch, and TensorFlow 2 use the helper, with unit, cross-backend, and embedding tests covering canonicalization, invalid sizes, and output parity.

Changes

Parameter shorthand normalization

Layer / File(s) Summary
Shared normalization contract
deepmd/infer/deep_eval.py
Adds shared validation and expansion for frame and atomic parameters while preserving the existing flattened atomic-parameter ABI.
Backend evaluation integration
deepmd/jax/infer/deep_eval.py, deepmd/pd/infer/deep_eval.py, deepmd/pt/infer/deep_eval.py, deepmd/tf2/infer/deep_eval.py
All four backends standardize fparam and aparam using frame, atom, and model dimension metadata before evaluation; PyTorch also normalizes before embedding auto-batching.
Normalization and backend parity tests
source/tests/common/test_deep_eval_parameter_shorthand.py, source/tests/consistent/io/test_io.py, source/tests/pt/model/test_embedding.py
Tests cover shorthand expansion, invalid sizes, conditional backend coverage, cross-backend parity, and embedding outputs.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related issues

  • #5662: Covers the related dpmodel implementation of fparam/aparam shorthand normalization.

Suggested labels: Python

Suggested reviewers: outisli

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 63.64% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly reflects the main change: normalizing parameter shorthand before batching.
Linked Issues check ✅ Passed The PR adds shared shorthand normalization in the affected backends and regression tests, matching #5666's requirements.
Out of Scope Changes check ✅ Passed The added tests and embedding normalization support the stated shorthand-handling fix and do not appear unrelated.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@deepmd/pt/infer/deep_eval.py`:
- Around line 548-555: Update eval_embedding to call _standardize_fparam_aparam
on fparam and aparam before dispatching to self._eval_func, using the embedding
evaluation’s frame count, natoms, and model parameter dimensions. Pass the
standardized values to the backend so shorthand inputs are batched by frames
correctly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e2d24567-276c-4ec1-a33e-25ee33258076

📥 Commits

Reviewing files that changed from the base of the PR and between 6c3b985 and 2149d24.

📒 Files selected for processing (7)
  • deepmd/infer/deep_eval.py
  • deepmd/jax/infer/deep_eval.py
  • deepmd/pd/infer/deep_eval.py
  • deepmd/pt/infer/deep_eval.py
  • deepmd/tf2/infer/deep_eval.py
  • source/tests/common/test_deep_eval_parameter_shorthand.py
  • source/tests/consistent/io/test_io.py

Comment thread deepmd/pt/infer/deep_eval.py
Standardize frame and atomic parameter shorthand in the common wrapper and backend entry points before automatic batching. Preserve full per-frame and per-atom arrays while broadcasting documented shared forms consistently across TensorFlow 2, PyTorch, JAX, and Paddle.

Normalize PyTorch embedding extraction before split execution so eval_embedding, eval_descriptor, and eval_fitting_last_layer accept shared fparam, per-atom aparam, and scalar aparam forms. Cover two frames forced through one-frame backend batches.

Coding-Agent: Codex
Codex-Version: codex-cli 0.144.4
Model: gpt-5.6-sol
Reasoning-Effort: xhigh
@njzjz-bot
njzjz-bot force-pushed the fix/deepeval-param-shorthand-backends-5666 branch from 2149d24 to 9227907 Compare July 17, 2026 01:57
@njzjz

njzjz commented Jul 18, 2026

Copy link
Copy Markdown
Member

Possible reviewers based on changed lines, exact file history, and exact-file review history:

  • @wanghan-iapcm — 15 commits on changed files; 105 reviews on exact changed files (deepmd/infer/deep_eval.py, deepmd/jax/infer/deep_eval.py, deepmd/pd/infer/deep_eval.py, deepmd/pt/infer/deep_eval.py, deepmd/tf2/infer/deep_eval.py, source/tests/consistent/io/test_io.py).
  • @iProzd — 19 commits on changed files (deepmd/infer/deep_eval.py, deepmd/pd/infer/deep_eval.py, deepmd/pt/infer/deep_eval.py).

No review request was made automatically.

Coding agent: Codex
Codex version: codex-cli 0.144.4
Model: gpt-5.6-sol
Reasoning effort: xhigh

@njzjz
njzjz requested review from iProzd and wanghan-iapcm July 18, 2026 07:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Code scan] Honor documented fparam/aparam shorthand in non-TF1 DeepEval backends

2 participants