Skip to content

fix(io,cpu): infer BPE for legacy tokenizer.json (#858); gather with N-D indices (#859) - #879

Merged
michalharakal merged 1 commit into
developfrom
fix/tokenizer-858-gather-859
Jul 24, 2026
Merged

fix(io,cpu): infer BPE for legacy tokenizer.json (#858); gather with N-D indices (#859)#879
michalharakal merged 1 commit into
developfrom
fix/tokenizer-858-gather-859

Conversation

@michalharakal

Copy link
Copy Markdown
Contributor

Fixes the last two bugs surfaced while porting an educational GPT project.

#858 — TokenizerFactory rejects legacy tokenizer.json

fromTokenizerJson required a model.type field and threw UnsupportedTokenizerException otherwise. GPT-2's official openai-community/gpt2 tokenizer.json predates that field, so it couldn't be loaded through the factory (the underlying QwenByteLevelBpeTokenizer.fromTokenizerJson parsed it fine).

Fix: when model.type is absent, infer it from structure. A model.merges list is unique to BPE among the HF model types (Unigram and WordPiece have none), so its presence routes the file to QwenByteLevelBpeTokenizer. Files with neither a type nor merges still throw, with a clearer message.

#859 — CPU gather fails on multi-dimensional indices

The op documents [N, L] index support, but read the indices with a flat indices.data[i] — the vararg element accessor needs one coordinate per dimension, so rank-2 indices threw Number of indices (1) must match tensor dimensions (2).

Fix: read the indices in row-major order — directly from the contiguous IntArray/FloatArray buffer 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.type tokenizer.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: gather with [2,3] indices over a [10,4] table returns the expected [2,3,4] result.
  • Full jvmTest for skainet-io-core and skainet-backend-cpu green.

With this, all eight issues the port surfaced have a fix (merged or in review).

…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
@michalharakal
michalharakal requested a review from aharakal July 24, 2026 15:53
@michalharakal
michalharakal merged commit ca0f33c into develop Jul 24, 2026
13 checks passed
@michalharakal
michalharakal deleted the fix/tokenizer-858-gather-859 branch July 24, 2026 17:17
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants