test: add .pte and .pt2 tests for dp convert-backend#5384
Open
wanghan-iapcm wants to merge 1 commit intodeepmodeling:masterfrom
Open
test: add .pte and .pt2 tests for dp convert-backend#5384wanghan-iapcm wants to merge 1 commit intodeepmodeling:masterfrom
wanghan-iapcm wants to merge 1 commit intodeepmodeling:masterfrom
Conversation
Add pt_expt backend (.pte/.pt2) to the parameterized extensions in test_models.py to verify convert-backend works for the new exportable formats. The fparam_aparam model (1 atom type) is switched from type_one_side=False to type_one_side=True (with ndim 2→1), which is equivalent for single-type models but enables pt_expt export. Models with type_one_side=False and multiple types (se_e2_a, se_e2_r) are skipped for .pte/.pt2 as make_fx cannot trace data-dependent indexing in NetworkCollection(ndim=2).
Contributor
📝 WalkthroughWalkthroughThis change updates test configurations and test suite to support pt-expt model backends (.pte and .pt2 extensions). YAML test case files are modified to adjust embedding dimensionality and type-one-side handling flags. The test suite reorganizes skip logic to handle different model backends appropriately. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
source/tests/infer/test_models.py (1)
42-45: Prefer deriving the pt_expt skip from fixture metadata.The hard-coded
("se_e2_a", "se_e2_r")list will stay stale the next time a fixture is made exportable. The fixture already carriesmodel_def_script.descriptor.type_one_side, so reading the skip condition fromcasewould keep this gate aligned with the actual model data instead of the testcase key.♻️ Suggested refactor
`@classmethod` def setUpClass(cls) -> None: key, extension = cls.param if extension in (".pte", ".pt2") and not INSTALLED_PT_EXPT: raise unittest.SkipTest("pt_expt backend not installed") - if key in ("se_e2_a", "se_e2_r") and extension in (".pte", ".pt2"): + case = get_cases()[key] + descriptor = (case.model_def_script or {}).get("descriptor", {}) + if ( + extension in (".pte", ".pt2") + and descriptor.get("type") in {"se_e2_a", "se_e2_r"} + and not descriptor.get("type_one_side", False) + ): raise unittest.SkipTest( "type_one_side=False is not supported for pt_expt export" ) if key == "se_e2_r" and extension == ".pth": raise unittest.SkipTest( "se_e2_r type_one_side is not supported for PyTorch models" ) - cls.case = get_cases()[key] + cls.case = case🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@source/tests/infer/test_models.py` around lines 42 - 45, Replace the hard-coded key check with a metadata-driven condition: instead of testing if key in ("se_e2_a", "se_e2_r"), read the fixture's descriptor via the test case (e.g., case.model_def_script.descriptor.type_one_side) and if that value is False and extension is in (".pte", ".pt2") raise unittest.SkipTest; update the conditional that currently uses variables key and extension to use case.model_def_script.descriptor.type_one_side for the gate so the skip stays in sync with fixture metadata.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@source/tests/infer/test_models.py`:
- Around line 42-45: Replace the hard-coded key check with a metadata-driven
condition: instead of testing if key in ("se_e2_a", "se_e2_r"), read the
fixture's descriptor via the test case (e.g.,
case.model_def_script.descriptor.type_one_side) and if that value is False and
extension is in (".pte", ".pt2") raise unittest.SkipTest; update the conditional
that currently uses variables key and extension to use
case.model_def_script.descriptor.type_one_side for the gate so the skip stays in
sync with fixture metadata.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 75ef918e-5c9d-4fdd-a70c-1ed85d0f074f
📒 Files selected for processing (4)
source/tests/infer/fparam_aparam-testcase.yamlsource/tests/infer/fparam_aparam.yamlsource/tests/infer/fparam_aparam_default.yamlsource/tests/infer/test_models.py
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5384 +/- ##
==========================================
- Coverage 82.38% 79.74% -2.65%
==========================================
Files 812 812
Lines 83611 83476 -135
Branches 4091 4051 -40
==========================================
- Hits 68882 66567 -2315
- Misses 13508 15682 +2174
- Partials 1221 1227 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
njzjz
approved these changes
Apr 7, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
.pteand.pt2to parameterized extensions intest_models.pyto verifyconvert-backendworks for the pt_expt exportable formatsfparam_aparammodel (1 atom type) fromtype_one_side=FalsetoTrue(withndim2→1), which is equivalent for single-type models but enables pt_expt exportse_e2_a/se_e2_rfor.pte/.pt2—type_one_side=Falsewith multiple types triggersGuardOnDataDependentSymNodeinmake_fx(data-dependent indexing inNetworkCollection(ndim=2))test_descriptorandtest_fitting_last_layerfor.pte/.pt2(not implemented in pt_exptDeepEval)Test plan
python -m pytest source/tests/infer/test_models.py -v— 55 passed, 67 skippedpython -m pytest source/tests/infer/test_models.py -v -k "pte or pt2"— 12 passed (fparam_aparam), rest skipped.pb/.pthtests unaffectedSummary by CodeRabbit