Fix token_lengths kwarg leaking into the labels encoder#372
Open
yuriihavrylko wants to merge 1 commit into
Open
Fix token_lengths kwarg leaking into the labels encoder#372yuriihavrylko wants to merge 1 commit into
yuriihavrylko wants to merge 1 commit into
Conversation
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
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.
Fixes #370
Problem
Since 3a38d45 ("make gliner serving more efficient") the collator emits a
token_lengthskwarg — precomputed CPU-side text lengths that avoid anattention_mask.sum().tolist()GPU→CPU sync. The uni-encoder text path pops it, butBiEncoder.encode_labelsonly removedpacking_configandpair_attention_maskbefore forwarding kwargs to the labels backbone. The text-sidetoken_lengthstherefore reaches the labels encoder and crashes any plain HF backbone:This breaks every bi-encoder checkpoint (e.g.
knowledgator/modern-gliner-bi-base-v1.0,knowledgator/modern-gliner-bi-large-v1.0) on transformers 4.x. Bisected to 3a38d45: every commit before it predicts correctly, every commit after crashes.Fix
Pop
token_lengthsinencode_labelsalongside the other text-side kwargs — the value describes text sequences and is meaningless for label sequences.Verification
knowledgator/modern-gliner-bi-base-v1.0+ transformers 4.48.3:TypeErrorabove[('Angela Merkel', 'person', 0.85), ('Kyiv', 'location', 0.75), ('CDU', 'organization', 0.93)]— identical to gliner 0.2.16 outputNote: on transformers 5.x this crash does not reproduce (the kwarg is silently swallowed), but bi-encoders fail there for an unrelated reason — see the companion PR for #324.