Skip to content

fix(api): trim PyTorch type map separators#5544

Merged
wanghan-iapcm merged 2 commits into
deepmodeling:masterfrom
njzjz-bothub:fix/pt-type-map-trailing-space
Jun 17, 2026
Merged

fix(api): trim PyTorch type map separators#5544
wanghan-iapcm merged 2 commits into
deepmodeling:masterfrom
njzjz-bothub:fix/pt-type-map-trailing-space

Conversation

@njzjz-bot

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

Copy link
Copy Markdown
Contributor

Problem

  • PyTorch and PTExpt get_type_map implementations append a trailing space to the returned type-map string.
  • Implementations that append into the output string can also retain stale caller-provided content.

Change

  • Build PyTorch/DeepSpin type-map strings by joining entries with separators only between items.
  • Clear output strings before filling them, including Paddle's character-buffer path.
  • Tighten C/C++ tests to require exact type-map strings and cover non-empty output strings.

Authored by OpenClaw (model: custom-chat-jinzhezeng-group/gpt-5.5)

Return PyTorch and PTExpt type maps with the same space-separated format as
other backends, without a trailing separator or stale caller-provided content.

Authored by OpenClaw (model: custom-chat-jinzhezeng-group/gpt-5.5)
@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Five get_type_map implementations (DeepPotPT, DeepPotPTExpt, DeepSpinPT, DeepSpinPTExpt, DeepPotPD) are fixed to clear the output string before populating it and to insert spaces only between elements, removing trailing spaces. Corresponding tests are tightened from substring checks to exact-equality assertions, and new type_map typed test cases are added for the PT, PT-expt-spin, and PD fixtures.

Changes

get_type_map trailing-space fix and test coverage

Layer / File(s) Summary
get_type_map space-join fix across backends
source/api_cc/src/DeepPotPT.cc, source/api_cc/src/DeepPotPTExpt.cc, source/api_cc/src/DeepSpinPT.cc, source/api_cc/src/DeepSpinPTExpt.cc, source/api_cc/src/DeepPotPD.cc
Each implementation now clears the output string at entry and appends a space separator only when prior content already exists, eliminating the trailing-space bug present across all five variants.
Test assertions updated to exact string equality
source/api_cc/tests/test_deeppot_pt.cc, source/api_cc/tests/test_deeppot_ptexpt.cc, source/api_cc/tests/test_deeppot_dpa_ptexpt_spin.cc, source/api_cc/tests/test_deeppot_pd.cc, source/api_c/tests/test_deeppot_a_ptexpt.cc
Existing type_map tests replace substring or tokenization checks with exact-equality assertions; new TYPED_TEST cases for TestInferDeepPotAPt, TestInferDeepSpinDpaPtExpt, and TestInferDeepPotAPd verify "O H", "Ni O H", and "O H" respectively; missing <string> includes are added to test files.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Suggested reviewers

  • wanghan-iapcm
  • njzjz
  • HydrogenSulfate
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title 'fix(api): trim PyTorch type map separators' directly and specifically describes the main change across multiple backend implementations - removing trailing spaces from type map strings.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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 and usage tips.

DeepPotPD appends characters into the caller-provided output string. Clear it
first so repeated calls and non-empty output strings do not retain stale data.

Authored by OpenClaw (model: custom-chat-jinzhezeng-group/gpt-5.5)
@codecov

codecov Bot commented Jun 16, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.75862% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.22%. Comparing base (d3834e2) to head (4c0d84c).

Files with missing lines Patch % Lines
source/api_c/tests/test_deeppot_a_ptexpt.cc 0.00% 1 Missing ⚠️
...ource/api_cc/tests/test_deeppot_dpa_ptexpt_spin.cc 75.00% 1 Missing ⚠️
source/api_cc/tests/test_deeppot_pd.cc 75.00% 1 Missing ⚠️
source/api_cc/tests/test_deeppot_pt.cc 75.00% 1 Missing ⚠️
source/api_cc/tests/test_deeppot_ptexpt.cc 0.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5544   +/-   ##
=======================================
  Coverage   82.21%   82.22%           
=======================================
  Files         892      892           
  Lines      101531   101542   +11     
  Branches     4240     4245    +5     
=======================================
+ Hits        83475    83490   +15     
+ Misses      16753    16751    -2     
+ Partials     1303     1301    -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 requested a review from wanghan-iapcm June 16, 2026 18:33
@wanghan-iapcm wanghan-iapcm added this pull request to the merge queue Jun 17, 2026
Merged via the queue into deepmodeling:master with commit f56ec2e Jun 17, 2026
70 checks passed
wccc-phys pushed a commit to wccc-phys/deepmd-kit that referenced this pull request Jun 23, 2026
Problem
- Issue deepmodeling#3905 asks for universal C/C++ API coverage instead of one-off
backend-specific DeepPot tests.
- A first pass should stay focused and avoid rewriting the existing
large API test suite.

Change
- Add shared reference data for the common DeepPot water fixture.
- Add parameterized C++ API tests for metadata and atomic inference
across TensorFlow, PyTorch, PT-Expt, JAX, and Paddle when those
backends/artifacts are available.
- Add matching C API tests using `DP_DeepPotCompute2` /
`DP_DeepPotComputef2` and link torch in the C test target so PT-Expt
guards are detected consistently.
- After deepmodeling#5544 fixed PyTorch type-map separators, require exact type-map
metadata again and tighten the remaining PyTorch DeepTensor type-map
test similarly.

Verification
- `uvx pre-commit run --files source/api_c/tests/CMakeLists.txt
source/api_c/tests/test_deeppot_universal.cc
source/api_cc/tests/test_deeppot_universal.cc
source/tests/infer/deeppot_universal_data.h
source/api_c/tests/test_deeppot_a_ptexpt.cc
source/api_cc/tests/test_deepdipole_pt.cc`
- Local full CTest was not run because this workspace does not have a
C++ backend dependency installed; the new tests skip unavailable backend
artifacts and should be exercised by the existing C++ test job after
generated models are prepared.

Refs deepmodeling#3905

Authored by OpenClaw (model: custom-chat-jinzhezeng-group/gpt-5.5)

---------

Co-authored-by: Jinzhe Zeng <jinzhe.zeng@ustc.edu.cn>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants