Repository navigation
Conversation
Replace old IEmbedding/IEncoding/IReranking interfaces with the new semantic Task SDK contracts: encoding -> text_to_token_features (new primary/default task) embedding -> text_to_embedding reranking -> text_pair_to_relevance Changes within families/albert/** only: - support.py: update declared tasks and default to new semantic IDs - model.py: accept new task strings; reject old ones with a clear message - runtime/plugin.cpp: require_task() checks ITextToTokenFeatures::kTask, ITextToEmbedding::kTask, ITextPairToRelevance::kTask; includes trtmc/internal/features.h for the kTask constants - runtime/pipeline.h: EncoderPipeline inherits trtmc::internal::IModel + ITextToTokenFeatures + ITextToEmbedding + ITextPairToRelevance; removes old IEmbedding/IEncoding/IReranking base classes - runtime/pipeline.cpp: implements task_bindings() with bind<> for all three interfaces; run() overloads port the existing postprocessing (CLS extraction, mean-pool+L2-norm, rerank template) unchanged to the new result types (TokenFeaturesResult, SemanticEmbeddingResult, RelevanceResult); encode_ids() helper is preserved as-is - tests/test_support.py: add CPU-CI identity contracts (new file); covers positive and negative model_type cases - tests/test_e2e.py: update TASKS frozenset to text_to_token_features; oracle (cosine >= 0.8 vs HF last_hidden_state[0,0]) unchanged - tests/manifests/albert-base.json: task -> text_to_token_features - tests/manifests/albert-base-tp4.json: task -> text_to_token_features - tests/sdk_consumer.c: public C API consumer for all three tasks - tests/sdk_consumer.cpp: public C++ API consumer for all three tasks Existing bundles built with task=encoding/embedding/reranking must be rebuilt; the primary task header in the bundle must match the new IDs. Signed-off-by: Darshan3690 <darshan.rajput123091@marwadiuniversity.ac.in>
Split SPDX legal header directives into standalone 4-line block comments at the preamble location in tests/sdk_consumer.c and tests/sdk_consumer.cpp, resolving the legal_headers.py check. Signed-off-by: Darshan3690 <darshan.rajput123091@marwadiuniversity.ac.in>
Extract std::string_view and Span<const int32_t> via std::get_if in EncoderPipeline::run, and use TRTMC_TEXT_UTF8 with as.text in sdk_consumer.c. Signed-off-by: Darshan3690 <darshan.rajput123091@marwadiuniversity.ac.in>
Require a tokenizer only for text input and format the Albert SDK migration sources for the repository source-quality check. Signed-off-by: Darshan3690 <darshan.rajput123091@marwadiuniversity.ac.in>
Implement text_to_pooled_features returning CLS representations to preserve legacy encoding results and align the E2E evaluation contract. Normalize builder legacy 'encoding' task to text_to_pooled_features for qualification profile compatibility, fix C and C++ SDK consumers, and integrate consumer builds into CMake. Signed-off-by: Darshan3690 <darshan.rajput123091@marwadiuniversity.ac.in>
… features per row Signed-off-by: Darshan3690 <darshan.rajput123091@marwadiuniversity.ac.in>
Report invalid encoder output through the task error channel, correct the public C++ matrix-view access, and exercise token features in the E2E manifest. Signed-off-by: Darshan3690 <darshan.rajput123091@marwadiuniversity.ac.in>
… SDK consumers The trtmc encode CLI returns token-feature values as a flat 1D array with a separate shape field. The E2E test expected a pre-shaped 2D array, causing an AssertionError in _assert_parity. - Read the reported shape, validate dimensions and element count, reshape the flat array, then perform per-row cosine parity. - Add the legacy encoding task to support.py (required by the benchmark qualification profile) and the corresponding assertion in test_support.py. - Integrate _assert_sdk_consumers into the E2E workflow to exercise the C and C++ SDK consumer executables when TRTMC_NATIVE_BUILD_DIR is set. Signed-off-by: Darshan3690 <darshan.rajput123091@marwadiuniversity.ac.in>
Include both family SDK consumers in selective model builds when tests are enabled. Initialize the C load-options size before calling the public model loader. Signed-off-by: yifeif <277870278+yifeif-nv@users.noreply.github.com>
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Comment |
This branch has not been deployed
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.
Background
The Community GPU run for #1564 fails because the selected model target does not build the SDK consumers required by its new E2E checks. Building those consumers exposes a second failure: the C consumer leaves
trtmc_load_options_v1.struct_sizezero.Exit Criteria
The existing selective model build produces both SDK consumers, and both unchanged ALBERT E2E cases pass with valid C API load options.
Implementation
This is a stacked follow-up to #1564. The fix itself is commit
3232d48dde31d216abc0a00126ef02d2c6215ee4, containing two additions:opts.struct_sizein the C consumer.Change categories
Validation
Commands and Results
The fix was integrated locally with main
587840c6362440bea6ae83080bb8ec11a450a2d5:albert-baseandalbert-base-token-features). It tested published head3232d48dde31d216abc0a00126ef02d2c6215ee4through merge1ec62d751f15712b79d703b15674f3ea0eee5ba4, based on main9b083a7fdac56f9d0f14084e61e2aa9dc0dc8816, with CI6e9cc60beafd1b38d0492144c6aa471837a33135. The selective build produced the required SDK consumers, original test criteria were retained, and deletion of the single functionally ready VM was confirmed by both cleanup paths. The later main addition of BiRefNet was not part of the earlier local GPU run.Hardware, Environment, and Revisions
Published head
3232d48d, additive on ALBERTe092cfb6. The local integration used main587840c6, Linux ARM64, GB300, TensorRT 11.1.0.106, Torch 2.12.0+cu130, andalbert/albert-base-v2@8e2f239c5f8a2c0f253781ca60135db913e5c80c. The completed Dev run used Linux x86_64, AWSg6.4xlarge/ L4, TensorRT 11.1.0.106, FP16 native bundles, and the existing FP32 reference cases.Not Run / Remaining Gaps
Both original selected TP1 ALBERT cases are qualified on AWS L4 and Nebius L40S at the published head. Latest Dev 37800278424, CI
13947f20e6014351c23c8a72a813c0423796bd00, passed the new 64 GiB pre-allocation profile and both original E2Es on Nebius, then confirmed deletion through owner and backstop. One create and one probe were observed; no setup retry, reconnect or image cache was used. TP4, other checkpoints, performance, and Internal/Nightly qualification were not run. Parent PR #1564 is still open; this follow-up must not merge ahead of it.Contributor Self-Review
Notes For Future Readers
Depends on #1564. Until that parent lands, GitHub's main comparison also includes its ALBERT feature; review the two-line fix commit separately. The contributor's original fork branch is unchanged. This follow-up provides a source snapshot for Dev qualification and must not be merged ahead of its parent.
Risk level
The change affects GPU validation or artifact contracts and requires the recorded target-platform qualification; existing numerical criteria and explicit failure gates are retained.