Skip to content

fix(tf2): register se_t, se_t_tebd, and se_atten_v2 descriptors#5721

Merged
njzjz merged 1 commit into
deepmodeling:masterfrom
wanghan-iapcm:fix-tf2-descriptor-register
Jul 3, 2026
Merged

fix(tf2): register se_t, se_t_tebd, and se_atten_v2 descriptors#5721
njzjz merged 1 commit into
deepmodeling:masterfrom
wanghan-iapcm:fix-tf2-descriptor-register

Conversation

@wanghan-iapcm

@wanghan-iapcm wanghan-iapcm commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Problem

Fixes #5677. TF2 standard-model construction resolves descriptor classes through BaseDescriptor.get_class_by_type(<type>), and the TF2 descriptor registry is a distinct registry from the dpmodel and JAX ones (each backend builds its own via make_base_descriptor). The se_t, se_t_tebd, and se_atten_v2 wrappers were defined without the @BaseDescriptor.register(...) decorators their JAX counterparts carry, so their config type names could not be resolved and TF2 model construction failed with an unknown-descriptor error even though the wrapper classes exist and are exported by deepmd.tf2.descriptor. The affected type names are se_e3, se_at, se_a_3be (all se_t), se_e3_tebd (se_t_tebd), and se_atten_v2.

Fix

Add the missing @BaseDescriptor.register(...) decorators, matching the JAX registrations for the same descriptor names. The se_e3_tebd registration goes on the outer DescrptSeTTebd only (not the block), mirroring JAX.

Test

Adds source/tests/consistent/test_tf2_descriptor_registration.py, which asserts that every TF2 descriptor config type name resolves via get_class_by_type. It fails on master for the five previously-unregistered names and passes with the fix. The test is gated on INSTALLED_TF2 (run with DEEPMD_TEST_TF2=1). Existing TF2 consistency tests never caught this because they instantiate the wrapper classes directly rather than through the registry string lookup.

Summary by CodeRabbit

  • New Features
    • Expanded TF2 descriptor availability so more descriptor names are recognized and usable.
  • Bug Fixes
    • Improved descriptor lookup reliability, helping ensure the correct TF2 descriptor is resolved at runtime.
  • Tests
    • Added coverage to verify that all expected TF2 descriptor types are registered and callable when the TF2 backend is available.

TF2 standard-model construction resolves descriptor classes through
BaseDescriptor.get_class_by_type(<type>) on the TF2 descriptor registry, which
is a separate registry from the dpmodel and JAX ones. The se_t, se_t_tebd, and
se_atten_v2 wrappers were defined without the @BaseDescriptor.register(...)
decorators their JAX counterparts carry, so their config type names (se_e3,
se_at, se_a_3be, se_e3_tebd, se_atten_v2) could not be resolved and TF2 model
construction failed with an unknown-descriptor error, even though the wrapper
classes exist and are exported by deepmd.tf2.descriptor.

Add the missing decorators, matching the JAX registrations for the same
descriptor names.

Adds a registration test asserting every TF2 descriptor type name resolves via
get_class_by_type; it fails on master for the five unregistered names and
passes with the fix. The test is gated on INSTALLED_TF2 and run with
DEEPMD_TEST_TF2=1. Existing TF2 consistency tests never caught this because they
instantiate the wrapper classes directly rather than through the registry.

Fix deepmodeling#5677
@dosubot dosubot Bot added the bug label Jul 3, 2026
@github-actions github-actions Bot added the Python label Jul 3, 2026
@wanghan-iapcm wanghan-iapcm requested a review from njzjz July 3, 2026 07:06
@coderabbitai

coderabbitai Bot commented Jul 3, 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: 3dd9bc53-c89a-4467-91aa-e8cfc159969b

📥 Commits

Reviewing files that changed from the base of the PR and between dd38b35 and c95c79e.

📒 Files selected for processing (4)
  • deepmd/tf2/descriptor/se_atten_v2.py
  • deepmd/tf2/descriptor/se_t.py
  • deepmd/tf2/descriptor/se_t_tebd.py
  • source/tests/consistent/test_tf2_descriptor_registration.py

📝 Walkthrough

Walkthrough

Adds @BaseDescriptor.register(...) decorators to three TF2 descriptor wrapper classes (DescrptSeAttenV2, DescrptSeT, DescrptSeTTebd) so they register under their respective config names, and adds a new unit test verifying these descriptor types resolve correctly.

Changes

TF2 Descriptor Registration

Layer / File(s) Summary
Register descriptor wrapper classes
deepmd/tf2/descriptor/se_atten_v2.py, deepmd/tf2/descriptor/se_t.py, deepmd/tf2/descriptor/se_t_tebd.py
Imports BaseDescriptor and adds @BaseDescriptor.register(...) decorators: "se_atten_v2" on DescrptSeAttenV2; "se_e3", "se_at", "se_a_3be" on DescrptSeT; and "se_e3_tebd" on DescrptSeTTebd.
Registration verification test
source/tests/consistent/test_tf2_descriptor_registration.py
Adds TestTF2DescriptorRegistration, skipped unless TF2 is installed, which loops over TF2_DESCRIPTOR_TYPES and asserts each resolves to a callable via BaseDescriptor.get_class_by_type.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Related issues: #5677 — Registers missing TF2 descriptor wrappers (se_atten_v2, se_e3, se_at, se_a_3be, se_e3_tebd) before model construction, matching the corresponding JAX registrations.

Suggested labels: TF2, bug, tests

Suggested reviewers: njzjz, wanghan-iapcm

Poem:

A rabbit hopped through TF2's code,
Found wrappers stranded down the road,
Decorators added, one by one,
"se_e3", "se_at", now they're done!
A test confirms each name resolves —
No more descriptor mysteries unsolved. 🐇✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes the main change: registering TF2 descriptor wrappers.
Linked Issues check ✅ Passed The PR adds the missing TF2 BaseDescriptor registrations for se_atten_v2, se_e3, se_at, se_a_3be, and se_e3_tebd as requested.
Out of Scope Changes check ✅ Passed The added TF2 registration test is aligned with the issue and no unrelated code changes are indicated.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ 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.

@codecov

codecov Bot commented Jul 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 81.14%. Comparing base (dd38b35) to head (c95c79e).

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5721      +/-   ##
==========================================
- Coverage   81.26%   81.14%   -0.13%     
==========================================
  Files         988      988              
  Lines      110877   110884       +7     
  Branches     4234     4232       -2     
==========================================
- Hits        90103    89973     -130     
- Misses      19249    19387     +138     
+ Partials     1525     1524       -1     

☔ 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 added this pull request to the merge queue Jul 3, 2026
Merged via the queue into deepmodeling:master with commit bbc3908 Jul 3, 2026
60 checks passed
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] Register TF2 descriptor wrappers before model construction

2 participants