Repository navigation
Conversation
Signed-off-by: JiaxinD <djx2048@gmail.com>
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Detect namespaces and load checkpoint tensors families/roberta/model.py, families/roberta/tests/test_checkpoint_weights.py |
The loader detects model.roberta., roberta., or no prefix. Tests compare loaded tensors across RoBERTa, XLM-RoBERTa, and CamemBERT checkpoints, including variants with and without pooler tensors. |
Add embedding end-to-end coverage families/roberta/tests/test_e2e.py, families/roberta/tests/test_embedding_e2e_contract.py, families/roberta/tests/manifests/multilingual-e5-large.json, families/roberta/tests/thresholds/multilingual-e5-large-*.json |
End-to-end helpers dispatch embedding tasks and calculate masked mean pooling with L2 normalization. The multilingual E5 manifest defines English-query and Chinese-passage cases. Contract tests check dispatch, reference outputs, parity, and case selection; threshold overrides set cosine thresholds to 0.999. |
Estimated code review effort: 3 (Moderate) | ~20 minutes
Change: Bug fix
Possibly related issues
- NVIDIA/TensorRT-Model-Connect issue 1508: The checkpoint tests cover prefixed and unprefixed checkpoint layouts, and the PR adds real-checkpoint E2E cases. The PR does not implement the requested Task SDK migration.
Merge Risk: 
Minimal · up to 473e3
-
No actionable merge-blocking risk was identified. Native GPU parity remains untested.
Pre-merge checks: 
8 · 
1
-
Failed checks: 1 warning Passed checks: 8 passed-
Full details: Family Ownership Boundary
-
Explanation
The pull request changes only
families/roberta/**. The new tests importfamilies.roberta.config,families.roberta.model, andfamilies.roberta.tests.test_e2e; they do not import another repository family.RobertaModel,XLMRobertaModel, andCamembertModelare external Transformers fixtures used to test model types already owned by the RoBERTa family, as shown byfamilies/roberta/support.pyacceptingcamembert,roberta, andxlm-roberta. The manifest and threshold files are also underfamilies/roberta/tests/. No other family or central registry, switch, source list, or strategy map is changed.
Full details: Shared Semantic Neutrality
-
Explanation
PASS. The pull request changes only
families/roberta/model.pyand RoBERTa-owned test, manifest, and threshold files. No shared code outside the model-owned Python, E2E, or family-owned test/config areas changed. The model-specific prefix handling and embedding reference behavior remain within the RoBERTa family implementation and its family tests.
Full details: Benchmark Validation Integrity
-
Explanation
PASS. The PR changes E2E validation semantics, but the compared paths remain equivalent.
families/roberta/tests/test_e2e.py:251-282dispatchesembeddingtoembedandencodingtoencode; its reference uses the same task split, manifest truncation, attention-mask mean pooling, and L2 normalization. The nativeembedcontract is independently documented inapps/perf_serving/trtmc_perf_serving/backends/reference/text.py:201-250with the same pooling and normalization meaning.test_embedding_e2e_contract.py:14-113covers both dispatch paths, masked pooling, normalization, cosine direction, and selector behavior. No benchmark timing, shard/sample accounting, serialization, or one-sided synchronization/transfer accounting changed.
Full details: Shared Change Blast Radius
-
Explanation
The pull request changes only
families/roberta/model.pyand RoBERTa-local tests, manifests, and thresholds. It does not modify shared runtime code, shared contracts, common tooling, examples, benchmarks, catalogs, or repository-wide validation infrastructure. The loader change is scoped to_RobertaModel, and the added embedding validation is implemented infamilies/roberta/tests/test_e2e.pyand family-local tests. Therefore the shared change blast-radius check is not applicable.
Full details: Description check
-
Explanation
The description covers all required sections, including motivation, exit criteria, implementation scope, change category, validation results, environment and revisions, remaining gaps, self-review, future notes, and risk rationale. It provides strong test evidence and clearly states compatibility and deployment boundaries. Some validation entries do not include exact commands, but the description is otherwise complete.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 12 functions across 4 files. (3 skipped: 3 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Comment @coderabbitai help to get the list of available commands.
|
@yifeif-nv #1597 is ready for protected CI at |
|
Internal CI trigger |
…xed-checkpoints Signed-off-by: JiaxinD <djx2048@gmail.com>
Signed-off-by: JiaxinD <djx2048@gmail.com>
|
@yifeif-nv Community CPU / Required, Stable and Dev passed for 473e328. Please trigger protected CI for this head, including the new multilingual-e5-large premerge case. |
Background
RoBERTa rejects bare encoder checkpoints saved by Hugging Face
RobertaModel,XLMRobertaModelandCamembertModel: it requests aroberta.prefix absent from these files. The pinned multilingual-e5-large checkpoint has this layout. Existing family E2E covers encoding only, without E5's mean-pooled embedding contract.Exit Criteria
Implementation
model.roberta.,roberta.or an empty root for encoder/pooler reads; fail when no root matches.embed, honor the manifest sequence limit, and retainencode/CLS for encoding.All changes are family-local. No public API, SDK ABI, bundle format, dependency or deployment-order change. No pretrained weights are distributed.
Change categories
Validation
Commands and Results
python -m pytest families/roberta/tests/test_e2e.py --e2e-model multilingual-e5-large -qThe GPU run built real engines and CLI/backend/family targets, used actual tokenizers and CUDA references, and retained the original assertions. The separate CPU launcher uses import-only TensorRT and redirects tiny HF tensor transfers to CPU.
Hardware, Environment, and Revisions
Code
473e32861f3776538dc9a84ede1ee145e3c9d2b1; E53d7cfbdacd47fdda877c5cd8a79fbcc4f2a574f3. H200 SM90, driver 595.71.05, repositoryDockerfile.dev.x86pinned TensorRT 26.07 image, TRT11.1.0.106/CUDA13.3.73, Torch2.12.0+cu130, Transformers5.2.0, Hub1.33.0. Model SHA256:020afdebf2762b29fcaf286629a96c3b3b65af241f6a08226b1cfee60a21def6. Earlier Windows CPU evidence used Transformers5.11.0.Not Run / Remaining Gaps
No TRT11.4 root-image, BF16, TP4, throughput, latency or memory comparison. Two unchanged Windows activation tests lacked TensorRT enums. Filtered-cache completeness initially failed before GPU build; the existing explicit model-directory override selected the same pinned files. Image restart emitted a compatibility warning; actual CUDA execution passed in the recorded environment only.
Contributor Self-Review
Notes For Future Readers
This fixes the existing family without another E5 owner. Review loader mapping, then task-specific reference and cases. Publisher card declares MIT. GPU evidence belongs only to the frozen head and recorded environment. Protected CI was requested once for this head after public CPU/Stable/Dev passed.
Risk level
Only key-root selection changes production behavior; graph construction and existing thresholds remain untouched.