Fix per-row label sets in inference(): KeyError on unequal lengths, silent mislabelling across chunks - #384
Open
jedediahg wants to merge 1 commit into
Open
Conversation
…belling `inference()` accepts `labels: List[List[str]]` (one label set per text), but two defects make that path unusable. 1. Padding class slots were never masked. With per-row label sets the class dimension is padded to the batch-wide maximum, so a row with fewer labels still carries scores in slots it never asked for. `torch.where` returned those class indices and `_build_span_tuple` raised `KeyError`, because they have no entry in that row's `id_to_class`. Masked before the candidate search in `BaseSpanDecoder._decode_batch` and `_decode_batch_item`, reusing the guard `_decode_explicit_spans` already applies. Masking rather than defaulting the lookup is deliberate: a default would emit spans labelled from padding slots -- real scores, meaningless labels, silently. 2. `id_to_classes` was never sliced to the chunk. `inference()` built the label mapping once for the whole batch and closed over it, so every DataLoader chunk received the full list while the decoder indexed it with a chunk-local index. The first chunk was correct by accident; rows in later chunks were decoded against another row's labels, with no error raised. The loaders now iterate row indices and select each chunk's label sets via `_entity_types_for_chunk`, mirroring how `_process_batches` already slices `word_input_spans`. A shared flat label list is passed through unsliced. Callers passing a single flat label list -- the documented path -- are unaffected; verified byte-identical output at batch sizes 1, 2, 4 and 8. Adds four regression tests, each of which fails without this change.
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 #383.
inference()acceptslabels: List[List[str]]— one label set per text — and the decoder has_get_id_to_class_for_samplefor exactly that. Two defects make the path unusable: unequal-length sets raiseKeyError, and equal-length sets are silently mislabelled as soon as the batch spans more than one chunk.Callers passing a single flat label list (the documented path) are unaffected.
1. Padding class slots were never masked
With per-row label sets the class dimension is padded to the batch-wide maximum, so a row with fewer labels still carries scores in slots it never asked for.
torch.wherereturned those class indices and_build_span_tupleraisedKeyError, since they have no entry in that row'sid_to_class.Fixed by masking before the candidate search in
BaseSpanDecoder._decode_batchand_decode_batch_item, reusing the guard_decode_explicit_spansalready applies:Masking rather than defaulting the lookup is deliberate. A
.get(class_idx + 1, ...)at the call site — which is what the neighbouringreturn_class_probspath does — would stop the exception and emit spans labelled from padding slots: real scores, meaningless labels, silently. That trades a loud failure for a quiet one.The mask is guarded by
if any(len(m) < num_classes ...), so the uniform-label case pays only the length check.2.
id_to_classeswas never sliced to the chunkinference()built the mapping once for the whole batch and closed over it:The
DataLoaderpassescollate_fnonly the current chunk's rows, but the closure handed the collator the fullentity_typesevery time;collator.py:110forwarded it unsliced, and the decoder then indexed that global list with a chunk-local index. The first chunk was correct by accident; every later chunk wore another row's labels, with no error.The loaders now iterate row indices and select each chunk's label sets through
_entity_types_for_chunk, mirroring how_process_batchesalready slices the other per-row input,word_input_spans. A shared flat list is passed through unsliced. Applied at all three loader sites.Verification
Before:
After: correct at every batch size, and unequal-length sets no longer raise.
urchade/gliner_multi-v2.1at batch sizes 1, 2, 4 and 8.tests/test_decoder.pycovering the batch (B > 1) and single-item (B == 1) paths — the existingtest_per_sample_id_to_classesonly uses equal-length mappings, which is the gap — and two intests/test_models.pyfor the chunk helper.tests/test_decoder.pyandtests/test_models.pypass (58 passed).ruff checkreports exactly the same set of pre-existing findings before and after. I deliberately did not runruff formaton these files: the repo is not currently format-clean, and reformatting would bury a ~120-line fix in unrelated churn.Once both are fixed, a fused call carrying a 9-label row set and a 1-label row set returns output equal to the two separate calls — max score delta 9.17e-05 over 3,827 spans on 100 real documents, with no threshold crossings.