Skip to content

fix(dpmodel): remap virtual type embeddings#5856

Open
njzjz-bot wants to merge 1 commit into
deepmodeling:masterfrom
njzjz-bot:fix/dpmodel-negative-type-embedding-5665
Open

fix(dpmodel): remap virtual type embeddings#5856
njzjz-bot wants to merge 1 commit into
deepmodeling:masterfrom
njzjz-bot:fix/dpmodel-negative-type-embedding-5665

Conversation

@njzjz-bot

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

Copy link
Copy Markdown
Contributor

Closes #5665.

Summary

  • add a shared padded type-embedding gather that maps atype < 0 to the explicit final zero row
  • apply it to DPA1, DPA2, DPA3, and se_t_tebd dense and graph/node gathers
  • remap each center/neighbor type before stripped one-sided and two-sided pair-table indexing
  • apply the same padding-row contract to SeZMTypeEmbedding
  • add NumPy and Array API strict unit tests plus direct DPA1 concat/strip and SeTTebd strip descriptor regressions

Why existing tests missed this

Normal model calls often clamp negative types before descriptor execution and zero virtual outputs afterward, while graph builders exclude virtual edges. Descriptor consistency fixtures themselves use nonnegative atom types. NumPy also silently interprets -1 as the final row, accidentally selecting the zero padding embedding and hiding the bug. No direct descriptor regression combined atype == -1 with an array backend that rejects negative gather indices, and stripped pair-index paths were not checked against the explicit padding type.

Validation

  • pytest source/tests/common/dpmodel/test_type_embedding_virtual.py source/tests/common/dpmodel/test_dpa1_call_graph_descriptor.py source/tests/common/dpmodel/test_descriptor_dpa2.py -q (29 passed after adding the SeTTebd strip case)
  • focused test_type_embedding_virtual.py pass: 8 passed, including atype_ext=[0, 1, -1] versus explicit padding [0, 1, 2]
  • ruff format . (1 new test file reformatted; 1663 files unchanged)
  • ruff check . (passed)
  • git diff --check (passed)

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

Summary by CodeRabbit

  • New Features

    • Improved handling of virtual and empty atom types across descriptor calculations.
    • Added consistent padding-aware type-embedding lookups for dense and graph execution.
    • Invalid or negative type indices now safely use a zero-padded embedding row.
  • Bug Fixes

    • Prevented backend-dependent errors when processing virtual atom types.
    • Ensured equivalent results for virtual types and explicit padding types across supported descriptor modes.
  • Tests

    • Added regression coverage for padding behavior across supported array backends and descriptor configurations.

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

coderabbitai Bot commented Jul 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The PR adds shared padding-aware type-embedding helpers and applies them across DPA1, DPA2, DPA3, SeTTebd, and SeZM embedding paths. New regression tests cover NumPy, strict array APIs, padded embeddings, and DPA1 virtual types.

Changes

Virtual type embedding handling

Layer / File(s) Summary
Embedding helper and boundary contracts
deepmd/dpmodel/utils/type_embed.py, deepmd/dpmodel/descriptor/dpa4_nn/embedding.py, source/tests/common/dpmodel/test_type_embedding_virtual.py
Adds negative-index remapping and backend-aware embedding gathers, updates padded SeZM lookups, and tests explicit padding-row behavior.
Descriptor embedding gather integration
deepmd/dpmodel/descriptor/dpa1.py, deepmd/dpmodel/descriptor/dpa2.py, deepmd/dpmodel/descriptor/dpa3.py, deepmd/dpmodel/descriptor/se_t_tebd.py
Routes dense and graph embedding lookups through take_type_embedding across the updated descriptor paths.
Strip-path type remapping and regression coverage
deepmd/dpmodel/descriptor/dpa1.py, deepmd/dpmodel/descriptor/se_t_tebd.py, source/tests/common/dpmodel/test_type_embedding_virtual.py
Remaps center and neighbor types for strip-mode indexing and verifies DPA1 virtual types match explicit padding types.

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

Possibly related PRs

Suggested labels: bug, enhancement, new feature, Python, Docs

Suggested reviewers: wanghan-iapcm

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Negative-type remapping is added across the affected dpmodel gather paths, with padding-aware tests covering virtual types.
Out of Scope Changes check ✅ Passed The changes stay focused on type-embedding remapping, helper utilities, and regression tests.
Docstring Coverage ✅ Passed Docstring coverage is 81.25% which is sufficient. The required threshold is 80.00%.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: remapping virtual type embeddings in dpmodel.
✨ 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.

Map negative virtual atom types to the explicit padding embedding row before array-api gathers and type-pair indexing. Apply the shared boundary across DPA1 through DPA4, SeZM, and SeTTebd descriptor paths.

Cover NumPy and array_api_strict embedding gathers, DPA1 concat and strip modes, SeZM, and an independent SeTTebd strip pair-index regression comparing -1 with the explicit padding type.

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/dpmodel-negative-type-embedding-5665 branch from 1f8ca16 to 1e74ddd Compare July 17, 2026 02:02
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.32%. Comparing base (6c3b985) to head (1e74ddd).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5856      +/-   ##
==========================================
- Coverage   78.58%   78.32%   -0.26%     
==========================================
  Files        1050     1050              
  Lines      120637   120650      +13     
  Branches     4356     4363       +7     
==========================================
- Hits        94801    94502     -299     
- Misses      24278    24588     +310     
- Partials     1558     1560       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@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 — 34 commits on changed files; 45 reviews on exact changed files (deepmd/dpmodel/descriptor/dpa1.py, deepmd/dpmodel/descriptor/dpa2.py, deepmd/dpmodel/descriptor/dpa3.py, deepmd/dpmodel/descriptor/dpa4_nn/embedding.py, deepmd/dpmodel/descriptor/se_t_tebd.py, deepmd/dpmodel/utils/type_embed.py).
  • @iProzd — 43 commits on changed files (deepmd/dpmodel/descriptor/dpa1.py, deepmd/dpmodel/descriptor/dpa2.py, deepmd/dpmodel/descriptor/dpa3.py, deepmd/dpmodel/descriptor/se_t_tebd.py, deepmd/dpmodel/utils/type_embed.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] Remap negative atom types before dpmodel type-embedding gathers

2 participants