fix(io,cpu): infer BPE for legacy tokenizer.json (#858); gather with N-D indices (#859) - #879
Merged
Merged
Conversation
…ices - TokenizerFactory.fromTokenizerJson (#858): legacy tokenizer.json files that omit model.type (e.g. openai-community/gpt2) are now supported by inferring the type from structure — a merges list is unique to BPE, so such files route to QwenByteLevelBpeTokenizer instead of throwing. - CPU gather (#859): multi-dimensional [N, L] indices threw because a flat data[i] access needs one coordinate per dimension. Now reads the indices in row-major order via the contiguous buffer, falling back to unravel. Adds tests: legacy no-model.type tokenizer.json infers BPE (and still throws without merges); gather with [2,3] indices returns [2,3,4]. Closes #858, #859
aharakal
approved these changes
Jul 24, 2026
This was referenced Jul 24, 2026
Closed
MacOS
pushed a commit
to MacOS/SKaiNET
that referenced
this pull request
Jul 27, 2026
Bump version 0.36.0 -> 0.37.0 (gradle.properties, docs/antora.yml, README quickstart). Promote CHANGELOG [Unreleased] to [0.37.0]: Lstm layer (SKaiNET-developers#824), real Dropout masking (SKaiNET-developers#867), LR schedules and mutable optimizer lr (SKaiNET-developers#866), optional-bias and open Linear (SKaiNET-developers#870, SKaiNET-developers#875), androidNative IO targets (SKaiNET-developers#836, SKaiNET-developers#842, SKaiNET-developers#845), the SDPA default-scale fix (SKaiNET-developers#880), three autograd fixes (SKaiNET-developers#877), the argMax DAG output spec (SKaiNET-developers#878), tokenizer BPE inference and N-D gather (SKaiNET-developers#879), plus the CI/docs supply-chain hardening and toolchain bumps. Refresh README "What's New" and add a Contributors (0.37.0) section. Co-Authored-By: Claude Opus 5 (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
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 the last two bugs surfaced while porting an educational GPT project.
#858— TokenizerFactory rejects legacytokenizer.jsonfromTokenizerJsonrequired amodel.typefield and threwUnsupportedTokenizerExceptionotherwise. GPT-2's officialopenai-community/gpt2tokenizer.jsonpredates that field, so it couldn't be loaded through the factory (the underlyingQwenByteLevelBpeTokenizer.fromTokenizerJsonparsed it fine).Fix: when
model.typeis absent, infer it from structure. Amodel.mergeslist is unique to BPE among the HF model types (Unigram and WordPiece have none), so its presence routes the file toQwenByteLevelBpeTokenizer. Files with neither a type nor merges still throw, with a clearer message.#859— CPUgatherfails on multi-dimensional indicesThe op documents
[N, L]index support, but read the indices with a flatindices.data[i]— the vararg element accessor needs one coordinate per dimension, so rank-2 indices threwNumber of indices (1) must match tensor dimensions (2).Fix: read the indices in row-major order — directly from the contiguous
IntArray/FloatArraybuffer when present, otherwise by unraveling the flat position into a coordinate. The output-shape logic was already correct for rank-2 indices.Tests
TokenizerFactoryDispatchTest: a legacy no-model.typetokenizer.json(with a byte-level pretokenizer and a merges list) infers BPE and encodes correctly; a file with neither type nor merges still throws.GatherRowDequantTest:gatherwith[2,3]indices over a[10,4]table returns the expected[2,3,4]result.jvmTestforskainet-io-coreandskainet-backend-cpugreen.With this, all eight issues the port surfaced have a fix (merged or in review).