Granite NAR current-API fix (cleaned-up #45) - #47
Merged
Conversation
…ndows The published ibm-granite/granite-speech-4.1-2b-nar custom code drifted: its inference entrypoint was renamed generate() -> transcribe() (can_generate() now returns False), and the output changed from decoded strings (output.text_preds) to raw token-id tensors (output.preds). The NAR backend still called the old API, so transcribe() raised AttributeError on the current revision under transformers 5.x -- on every platform, not just Windows. The catalog gate merely kept Linux/Mac users from discovering it; the friend's pinned Linux NAR works only because it predates the upstream rename. - asr.py: make NAR inference version-robust. Prefer model.transcribe() and fall back to model.generate(); prefer output.text_preds and else decode output.preds token ids with a tokenizer now loaded in _load(). Old and new model revisions both work, so Linux/Mac do not regress. - catalog.py / profiles.py: lift the deliberate "NAR unsupported on Windows" gate now that NAR runs on Windows CUDA. AR (granite-speech-4.1-2b) stays the Windows default; NAR becomes selectable. Incremental transcription stays gated on Windows (separate validation) with an accurate reason. Verified on an RTX 5090 (Blackwell, cu128): NAR loads on cuda:0, warm transcribe ~0.06s (vs ~0.33s for AR), correct transcript. New unit tests cover both the new (transcribe/preds) and old (generate/text_preds) code paths without needing the model downloaded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Follow-up to the nar-current-api review: - models/prefetch.py: _prefetch_granite_nar now also caches the tokenizer (AutoTokenizer) so a fresh prefetch covers everything the NAR backend loads with local_files_only=True; it previously relied on AutoProcessor co-caching the tokenizer files. - asr.py: _nar_decode uses truthiness instead of `is not None`, so a present-but-empty text_preds/preds falls through instead of raising IndexError; documented that already-decoded text_preds wins when both are present. New tests pin both behaviours. - README/CONTEXT: Granite NAR is now selectable on Windows CUDA (AR stays the default); updated the support matrix and the Windows ASR list. ROCm remains unsupported on Windows. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Upstream PR #43 added a new `windows_openvino` runtime profile that set `granite_nar_unsupported_reason=GRANITE_NAR_UNSUPPORTED_WINDOWS`. This branch deletes that constant when lifting the NAR-on-Windows gate, so git's textual auto-merge (the two changes never overlapped a line) left a dangling reference that `ty` flagged as unresolved-reference. Lift the gate on windows_openvino too, consistent with NAR being selectable on every Windows profile -- device resolution handles actual CUDA availability. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Review follow-ups on top of sumgup0's NAR-current-api work: - Load the NAR tokenizer lazily (cached) instead of eagerly in _load(). The current cached Linux gfx1151 revision returns decoded `text_preds` and never uses the tokenizer, so the eager load added an unused object to the PRIMARY user's startup. Deferring it keeps that path byte-for-byte as it was, and reverts _load() to its master 2-tuple shape. prefetch still caches the tokenizer so the lazy load works offline (local_files_only=True). - _nar_infer: raise a clear RuntimeError when a model exposes neither transcribe() nor generate(), instead of a bare AttributeError. - Trim the _nar_decode docstring. Validated: ruff clean, ty unchanged (33), 393 tests pass (incl. the version-robust NAR tests, updated for the load_tokenizer callable). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Owner-controlled, review-hardened version of #45 (original by @sumgup0). Rebased onto current master (post-#46) and carries @sumgup0's commits plus the thermonuclear-review follow-ups. Supersedes #45.
What #45 does
Makes Granite NAR inference version-robust against the upstream
generate()→transcribe()/text_preds→predsAPI drift (would otherwiseAttributeErroron the next model refresh — on Linux too), and lifts the Windows NAR gate.Review verdict
No regression to the primary Linux gfx1151 user — verified against the actual cached model revision: it falls to
generate()/text_preds, byte-identical to master. No blockers.Cleanups applied on top
_load()that the current Linux revision never uses (it returnstext_preds). Deferred to a cached_get_tokenizer()called only on thepredspath, so the primary user's startup is unchanged from master, and_load()returns to its 2-tuple shape.prefetchstill caches the tokenizer for offline use._nar_infer: clearRuntimeErrorwhen a model exposes neithertranscribe()norgenerate()._nar_decodedocstring.Kept as-is (judged correct): the ungated NAR-on-Windows code is consistent with Linux (
linux_cpualso allows NAR ungated), and the catalog/profiles formatting is ruff-compliant — reverting it would re-introduce drift.Validation
ruff clean, ty unchanged (33), 393 tests pass (incl. the version-robust NAR tests). Recommend one warm gfx1151 NAR transcription before merge (CI is GPU-less).
🤖 Generated with Claude Code