fix(dpmodel): remap virtual type embeddings#5856
Conversation
📝 WalkthroughWalkthroughThe 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. ChangesVirtual type embedding handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
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
1f8ca16 to
1e74ddd
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. 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. 🚀 New features to boost your workflow:
|
|
Possible reviewers based on changed lines, exact file history, and exact-file review history:
No review request was made automatically. Coding agent: Codex |
Closes #5665.
Summary
atype < 0to the explicit final zero rowse_t_tebddense and graph/node gathersSeZMTypeEmbeddingWhy 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
-1as the final row, accidentally selecting the zero padding embedding and hiding the bug. No direct descriptor regression combinedatype == -1with 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)test_type_embedding_virtual.pypass: 8 passed, includingatype_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
Bug Fixes
Tests