Skip to content

fix(pt): fail early when compress lacks custom ops#5610

Merged
wanghan-iapcm merged 2 commits into
deepmodeling:masterfrom
njzjz-bot:fix-pt-compress-custom-op-check
Jun 30, 2026
Merged

fix(pt): fail early when compress lacks custom ops#5610
wanghan-iapcm merged 2 commits into
deepmodeling:masterfrom
njzjz-bot:fix-pt-compress-custom-op-check

Conversation

@njzjz-bot

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

Copy link
Copy Markdown
Contributor

Summary

  • fail early in dp --pt compress when libdeepmd_op_pt is not loaded
  • explain why saving a compressed model without the custom tabulation ops produces an unusable artifact
  • add a regression test for the compression custom-op guard

Addresses #5368.

Tests

  • python3 -m py_compile deepmd/pt/entrypoints/compress.py source/tests/pt/test_compress_entrypoint.py
  • uvx ruff check deepmd/pt/entrypoints/compress.py source/tests/pt/test_compress_entrypoint.py
  • uvx ruff format --check deepmd/pt/entrypoints/compress.py source/tests/pt/test_compress_entrypoint.py
  • git diff --check

Could not run the pytest case locally because this checkout does not have the project test dependencies installed, and uv run --extra test ... failed dependency resolution before creating a usable test environment.

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

Summary by CodeRabbit

  • Bug Fixes

    • Added a pre-flight guard for dp --pt compress to confirm the required PyTorch customized operations are loaded before compression starts.
    • If support is missing, compression now stops immediately with a clearer error (before any model loading or serialization happens).
  • Tests

    • Added unit tests covering the guard behavior (raises when missing, proceeds when available) and verifying the check occurs before model loading.

Authored by OpenClaw (model: custom-chat-jinzhezeng-group/gpt-5.5)
@njzjz-bot njzjz-bot force-pushed the fix-pt-compress-custom-op-check branch from f0ddb9d to 3112b13 Compare June 29, 2026 21:26
@coderabbitai

coderabbitai Bot commented Jun 29, 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: 32529f8c-c43d-452c-9b4e-e7a67a8ed056

📥 Commits

Reviewing files that changed from the base of the PR and between 3112b13 and 76e27fe.

📒 Files selected for processing (1)
  • source/tests/pt/test_compress_entrypoint.py

📝 Walkthrough

Walkthrough

Adds a pre-flight check to dp --pt compress: a new helper raises RuntimeError when ENABLE_CUSTOMIZED_OP is false, and enable_compression() calls it before loading the model. New tests cover the guard and the entrypoint behavior.

Changes

Customized OP guard for compress entrypoint

Layer / File(s) Summary
Guard constant, helper, and enable_compression() call
deepmd/pt/entrypoints/compress.py
Imports ENABLE_CUSTOMIZED_OP, defines CUSTOMIZED_OP_COMPRESSION_ERROR, adds assert_customized_op_available_for_compression(), and calls it at the start of enable_compression().
Unit tests for the guard
source/tests/pt/test_compress_entrypoint.py
Adds the test module, verifies the guard raises or passes based on ENABLE_CUSTOMIZED_OP, checks enable_compression() raises before torch.jit.load(), and includes the test runner entry point.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

Suggested reviewers

  • njzjz
  • iProzd
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% 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 states the main change: pt compression now fails early when custom ops are missing.
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.
✨ 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 `@source/tests/pt/test_compress_entrypoint.py`:
- Around line 13-27: The current regression tests only cover
assert_customized_op_available_for_compression in isolation, so they do not
protect the actual enable_compression flow. Add a new test in
test_compress_entrypoint.py that patches compress.ENABLE_CUSTOMIZED_OP to False,
calls compress.enable_compression(), and asserts it raises before
torch.jit.load() is reached; use the existing helpers and symbols like
enable_compression() and assert_customized_op_available_for_compression() to
locate the right path.
🪄 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: 1d5de2ea-a0ca-4700-b402-5b165e09db31

📥 Commits

Reviewing files that changed from the base of the PR and between 1550599 and f0ddb9d.

📒 Files selected for processing (2)
  • deepmd/pt/entrypoints/compress.py
  • source/tests/pt/test_compress_entrypoint.py

Comment thread source/tests/pt/test_compress_entrypoint.py

@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.

🧹 Nitpick comments (1)
deepmd/pt/entrypoints/compress.py (1)

64-65: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Cover the enable_compression() call site in tests.

The current regression tests only exercise assert_customized_op_available_for_compression() directly. If Line 64 is removed later, the suite would still stay green, so the "dp --pt compress fails early" contract is not fully pinned down yet. Please add one entrypoint-level test that asserts enable_compression() raises before torch.jit.load() when customized ops are unavailable.

🤖 Prompt for 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.

In `@deepmd/pt/entrypoints/compress.py` around lines 64 - 65, Add an
entrypoint-level regression test around enable_compression() in compress.py so
the early-failure contract is covered end to end. The current tests only call
assert_customized_op_available_for_compression() directly, so they would miss a
removed guard in enable_compression(); add a test that exercises
enable_compression() itself, verifies it raises when customized ops are
unavailable, and confirms the failure happens before any torch.jit.load() call.
Use the enable_compression() and
assert_customized_op_available_for_compression() symbols to locate the code
path.
🤖 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.

Nitpick comments:
In `@deepmd/pt/entrypoints/compress.py`:
- Around line 64-65: Add an entrypoint-level regression test around
enable_compression() in compress.py so the early-failure contract is covered end
to end. The current tests only call
assert_customized_op_available_for_compression() directly, so they would miss a
removed guard in enable_compression(); add a test that exercises
enable_compression() itself, verifies it raises when customized ops are
unavailable, and confirms the failure happens before any torch.jit.load() call.
Use the enable_compression() and
assert_customized_op_available_for_compression() symbols to locate the code
path.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: b426579c-3496-4bde-8f98-5ff3d42bad87

📥 Commits

Reviewing files that changed from the base of the PR and between f0ddb9d and 3112b13.

📒 Files selected for processing (2)
  • deepmd/pt/entrypoints/compress.py
  • source/tests/pt/test_compress_entrypoint.py
🚧 Files skipped from review as they are similar to previous changes (1)
  • source/tests/pt/test_compress_entrypoint.py

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

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.41%. Comparing base (1550599) to head (76e27fe).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5610   +/-   ##
=======================================
  Coverage   82.41%   82.41%           
=======================================
  Files         903      903           
  Lines      101846   101850    +4     
  Branches     4071     4072    +1     
=======================================
+ Hits        83940    83944    +4     
+ Misses      16439    16437    -2     
- Partials     1467     1469    +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.

@wanghan-iapcm wanghan-iapcm added this pull request to the merge queue Jun 30, 2026
Merged via the queue into deepmodeling:master with commit 92c6a9a Jun 30, 2026
70 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.

[Feature Request]Warn on missing custom OP during dp --pt convert/dp --pt compress and allow overriding SHARED_LIB_DIR

2 participants