diff --git a/jmlx-models/src/main/java/se/alipsa/jmlx/models/Qwen3VlPromptExpander.java b/jmlx-models/src/main/java/se/alipsa/jmlx/models/Qwen3VlPromptExpander.java index dc213a2d..d9c3b8ef 100644 --- a/jmlx-models/src/main/java/se/alipsa/jmlx/models/Qwen3VlPromptExpander.java +++ b/jmlx-models/src/main/java/se/alipsa/jmlx/models/Qwen3VlPromptExpander.java @@ -118,13 +118,18 @@ private static int scanGrammar(int[] ids, Qwen3VlPromptTokens tokens, boolean[] int end = tokens.visionEndTokenId(); int image = tokens.imageTokenId(); int video = tokens.videoTokenId(); + // A declared video token is rejected anywhere in the prompt, before the grammar scan: the + // video error is reported instead of whichever wrapper error its position would trigger. + if (video >= 0) { + for (int id : ids) { + if (id == video) { + throw videoError(id); + } + } + } int wrappers = 0; for (int i = 0; i < ids.length; i++) { int id = ids[i]; - if (video >= 0 && id == video) { - throw new IllegalArgumentException( - "prompt contains the video token (ID " + id + "); video is not supported"); - } if (id == start) { if (i + 2 >= ids.length || ids[i + 1] != image || ids[i + 2] != end) { throw new IllegalArgumentException( @@ -162,6 +167,11 @@ private static int scanGrammar(int[] ids, Qwen3VlPromptTokens tokens, boolean[] return wrappers; } + private static IllegalArgumentException videoError(int id) { + return new IllegalArgumentException( + "prompt contains the video token (ID " + id + "); video is not supported"); + } + private static int mergedTokens(int[] grid) { return Math.multiplyExact(grid[0], Math.multiplyExact(grid[1], grid[2])); } diff --git a/jmlx-models/src/main/java/se/alipsa/jmlx/models/Qwen3VlPromptPlan.java b/jmlx-models/src/main/java/se/alipsa/jmlx/models/Qwen3VlPromptPlan.java index 6dc69bd4..f615409f 100644 --- a/jmlx-models/src/main/java/se/alipsa/jmlx/models/Qwen3VlPromptPlan.java +++ b/jmlx-models/src/main/java/se/alipsa/jmlx/models/Qwen3VlPromptPlan.java @@ -160,29 +160,25 @@ public void requireCacheCapacity(KVCachePolicy policy, int maxNewTokens) { /** * The shared RoPE coordinate all three axes take at one decode step: the logical cache position - * plus this plan's delta. A negative result is rejected: decoding before the expanded prompt is - * fully prefilled has no valid coordinate. + * plus this plan's delta. Decoding before the expanded prompt is fully prefilled has no valid + * coordinate, so a cache position inside the expanded prompt is rejected. * - * @param cachePosition the logical cache position of the decode step; must be non-negative + * @param cachePosition the logical cache position of the decode step; must be at least {@link + * #expandedLength()} * @return the coordinate to use on all three axes */ public int ropePositionAt(int cachePosition) { if (cachePosition < 0) { throw new IllegalArgumentException("cachePosition must be non-negative: " + cachePosition); } - int position = Math.addExact(cachePosition, positionDelta); - if (position < 0) { + if (cachePosition < expandedLength()) { throw new IllegalArgumentException( - "RoPE position at cache position " + "cachePosition " + cachePosition - + " is negative: " - + position - + " (delta " - + positionDelta - + "); decode starts at cache position " + + " is inside the expanded prompt: decode starts at cache position " + expandedLength()); } - return position; + return Math.addExact(cachePosition, positionDelta); } private static int[] requireAxis(int[] axis, String name, int expandedLength) { diff --git a/jmlx-models/src/main/java/se/alipsa/jmlx/models/Qwen3VlPromptTokens.java b/jmlx-models/src/main/java/se/alipsa/jmlx/models/Qwen3VlPromptTokens.java index 204c5cdb..500dbc2a 100644 --- a/jmlx-models/src/main/java/se/alipsa/jmlx/models/Qwen3VlPromptTokens.java +++ b/jmlx-models/src/main/java/se/alipsa/jmlx/models/Qwen3VlPromptTokens.java @@ -84,11 +84,13 @@ record Qwen3VlPromptTokens( * tokenizer's name-to-ID direction, in the order image, video, vision start, vision end. A * sentinel that is not a vocabulary token fails the load (the tokenizer's own exception is left * to propagate), and a sentinel whose resolved ID differs from the config's declaration for that - * role is rejected rather than silently overridden. + * role is rejected rather than silently overridden. The video sentinel is the one exception: a + * config video token ID of -1 (the checkpoint declares no video token) resolves to the same -1 + * without ever looking the video spelling up in the vocabulary. * * @param tokenizer the model's tokenizer; null is rejected * @param configImageTokenId the outer config's image token ID - * @param configVideoTokenId the outer config's video token ID + * @param configVideoTokenId the outer config's video token ID, or -1 when it declares none * @param configVisionStartTokenId the outer config's vision start token ID * @param configVisionEndTokenId the outer config's vision end token ID * @return the resolved, cross-checked token set @@ -103,7 +105,12 @@ static Qwen3VlPromptTokens resolve( int configVisionEndTokenId) { Objects.requireNonNull(tokenizer, "tokenizer"); int image = resolveRole(tokenizer, IMAGE_TOKEN_CONTENT, configImageTokenId, "image"); - int video = resolveRole(tokenizer, VIDEO_TOKEN_CONTENT, configVideoTokenId, "video"); + // -1 is the record's "no video token" convention: a config that declares none must not be + // required to have a video sentinel in its vocabulary. + int video = + configVideoTokenId == -1 + ? -1 + : resolveRole(tokenizer, VIDEO_TOKEN_CONTENT, configVideoTokenId, "video"); int start = resolveRole( tokenizer, VISION_START_TOKEN_CONTENT, configVisionStartTokenId, "vision start"); @@ -114,7 +121,8 @@ static Qwen3VlPromptTokens resolve( private static int resolveRole( HfTokenizer tokenizer, String sentinel, int configId, String role) { - int resolved = tokenizer.eosTokenId(sentinel).getAsInt(); + // The sentinels are ordinary vocabulary entries, not EOS role tokens. + int resolved = tokenizer.tokenId(sentinel).getAsInt(); if (resolved != configId) { throw new IllegalArgumentException( "the " diff --git a/jmlx-models/src/main/java/se/alipsa/jmlx/models/SmolVlmModel.java b/jmlx-models/src/main/java/se/alipsa/jmlx/models/SmolVlmModel.java index 9aa35812..785fd190 100644 --- a/jmlx-models/src/main/java/se/alipsa/jmlx/models/SmolVlmModel.java +++ b/jmlx-models/src/main/java/se/alipsa/jmlx/models/SmolVlmModel.java @@ -198,9 +198,10 @@ static SmolVlmPromptTokens resolveTokens(HfTokenizer tokenizer) { } private static int special(HfTokenizer tokenizer, String name) { - // The tokenizer's configured-token lookup is exposed through eosTokenId(String); it accepts - // any vocabulary token. The checks below independently require a declared special marker. - int id = tokenizer.eosTokenId(name).orElseThrow(); + // The sentinels are ordinary vocabulary entries, not role tokens configured in + // tokenizer_config.json, so this is a neutral name-to-ID lookup; the checks below independently + // require a declared special marker. + int id = tokenizer.tokenId(name).orElseThrow(); if (!tokenizer.decode(List.of(id), true).isEmpty() || !tokenizer.encode(name, false).equals(List.of(id))) { throw new IllegalArgumentException("image marker must be a declared special token: " + name); diff --git a/jmlx-models/src/test/java/se/alipsa/jmlx/models/Qwen3VlPromptExpanderTest.java b/jmlx-models/src/test/java/se/alipsa/jmlx/models/Qwen3VlPromptExpanderTest.java index 797dd206..dc04688c 100644 --- a/jmlx-models/src/test/java/se/alipsa/jmlx/models/Qwen3VlPromptExpanderTest.java +++ b/jmlx-models/src/test/java/se/alipsa/jmlx/models/Qwen3VlPromptExpanderTest.java @@ -10,6 +10,7 @@ import java.nio.file.Path; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; @@ -86,6 +87,7 @@ private static Qwen3VlPromptTokens goldenTokens() throws Exception { @Test void goldenExpansionsAndPositionsMatchThePinnedReference() throws Exception { Qwen3VlPromptTokens tokens = goldenTokens(); + int checked = 0; for (JsonNode caseNode : golden().required("cases")) { String caseName = caseNode.required("name").asString(); for (var variant : caseNode.required("variants").properties()) { @@ -116,13 +118,21 @@ void goldenExpansionsAndPositionsMatchThePinnedReference() throws Exception { assertArrayEquals(ints(positions.required("h")), plan.heightPositions(), name); assertArrayEquals(ints(positions.required("w")), plan.widthPositions(), name); assertEquals(value.required("delta").asInt(), plan.positionDelta(), name); + checked++; } } + assertEquals( + 16, + checked, + "golden must contain exactly 16 case variants (8 cases x 2 add_generation_prompt " + + "variants): " + + goldenPath()); } @Test void singleImageCasesReproduceTheWp1ProbeNumbers() throws Exception { Qwen3VlPromptTokens tokens = goldenTokens(); + int checked = 0; for (String caseName : List.of("single_image_256x256", "single_image_64x32")) { JsonNode caseNode = caseByName(caseName); for (var variant : caseNode.required("variants").properties()) { @@ -141,8 +151,14 @@ void singleImageCasesReproduceTheWp1ProbeNumbers() throws Exception { assertEquals(90, plan.expandedLength(), name); assertEquals(-60, plan.positionDelta(), name); } + checked++; } } + assertEquals( + 2, + checked, + "golden must contain exactly 2 single-image add_generation_prompt_true variants: " + + goldenPath()); } private static JsonNode caseByName(String name) throws Exception { @@ -192,7 +208,7 @@ void wrapperExpandsToTheMergedFeatureCount() { assertArrayEquals(new int[] {0, 1, 2, 3, 3, 3, 3, 3, 3, 6, 7, 8}, plan.temporalPositions()); assertArrayEquals(new int[] {0, 1, 2, 3, 3, 3, 4, 4, 4, 6, 7, 8}, plan.heightPositions()); assertArrayEquals(new int[] {0, 1, 2, 3, 4, 5, 3, 4, 5, 6, 7, 8}, plan.widthPositions()); - // The block's largest coordinate (8) plus one minus the 12-token length: -3. + // The prompt's largest coordinate (8) plus one minus the 12-token length: -3. assertEquals(-3, plan.positionDelta()); } @@ -212,9 +228,12 @@ void twoImagesWithDifferentGridsPlanTextBetweenThemAndANegativeDelta() { assertArrayEquals(new int[] {0, 1, 1, 1, 2, 2, 2, 4, 5, 6, 7, 7, 9}, plan.heightPositions()); assertArrayEquals(new int[] {0, 1, 2, 3, 1, 2, 3, 4, 5, 6, 7, 8, 9}, plan.widthPositions()); assertEquals(-3, plan.positionDelta()); - // Decode continues from the logical cache position plus the delta, never the KV length. + // Decode continues from the logical cache position plus the delta, never the KV length, + // and only at or after the expanded prompt's finished position. assertEquals(10, plan.ropePositionAt(13)); - assertEquals(9, plan.ropePositionAt(12)); + assertEquals( + "cachePosition 12 is inside the expanded prompt: decode starts at cache position 13", + assertThrows(IllegalArgumentException.class, () -> plan.ropePositionAt(12)).getMessage()); } @Test @@ -228,7 +247,10 @@ void textOnlyPromptPlansSequentialPositionsAndZeroDelta() { assertEquals(List.of(), plan.images()); assertEquals(0, plan.totalFeatureRows()); assertArrayEquals(new int[0], plan.featureDestinations()); - assertEquals(0, plan.ropePositionAt(0)); + assertEquals(3, plan.ropePositionAt(3)); + assertEquals( + "cachePosition 0 is inside the expanded prompt: decode starts at cache position 3", + assertThrows(IllegalArgumentException.class, () -> plan.ropePositionAt(0)).getMessage()); } @Test @@ -255,6 +277,32 @@ void videoTokensAreRejectedWhenDeclared() { "prompt contains the video token (ID 11); video is not supported", failure.getMessage()); } + @Test + void completeVideoWrapperReceivesTheVideoError() { + // START/VIDEO/END is a complete video wrapper: the start-token check must not hide the + // video error behind the less helpful malformed-wrapper message. + IllegalArgumentException failure = + assertThrows( + IllegalArgumentException.class, + () -> Qwen3VlPromptExpander.expand(new int[] {START, VIDEO, END}, tokens(), List.of())); + assertEquals( + "prompt contains the video token (ID 11); video is not supported", failure.getMessage()); + } + + @Test + void videoTokenInsideImageWrapperReceivesTheVideoError() { + // A video token in the third (or fourth, with a trailing end) position of an image wrapper + // must report the video error, not the malformed-wrapper error the grammar check would emit. + for (int[] prompt : new int[][] {{START, IMAGE, VIDEO}, {START, IMAGE, VIDEO, END}}) { + IllegalArgumentException failure = + assertThrows( + IllegalArgumentException.class, + () -> Qwen3VlPromptExpander.expand(prompt, tokens(), List.of())); + assertEquals( + "prompt contains the video token (ID 11); video is not supported", failure.getMessage()); + } + } + @Test void anUndeclaredVideoIdIsAnOrdinaryToken() { Qwen3VlPromptPlan plan = @@ -458,6 +506,70 @@ void thePlannerHandlesTextOnlyAndEmptyPrompts() { assertEquals(new Qwen3VlRopePlan(new int[0], new int[0], new int[0], 0), empty); } + @Test + void multiFrameImagePlanTakesPerFrameTemporalCoordinates() { + // Production preprocessing currently rejects gridT != 1 (Qwen3VlGeometryPlan), so this + // test calls the planner directly to pin its multi-frame contract ahead of that. + // The reference's single_image_t2 case: vision grid (2, 4, 4) with merge 2 (the LLM grid + // (2, 2, 2)), wrapped in three text tokens, the vision start sentinel, the eight image + // sentinels, the vision end sentinel and two text tokens: 15 tokens in total. + Qwen3VlRopePlan plan = + Qwen3VlPositionPlanner.plan( + new int[] { + 1000, 1001, 1002, START, IMAGE, IMAGE, IMAGE, IMAGE, IMAGE, IMAGE, IMAGE, IMAGE, END, + 1003, 1004 + }, + tokens(), + List.of(new int[] {2, 2, 2})); + // The text run is 0..3 (the vision start sentinel counts as text); the image block at 4..11 + // takes base 4: T is 4,4,4,4,5,5,5,5 by frame while H and W restart per frame, and the + // trailing text resumes at the block maximum (5) plus one. + assertArrayEquals( + new int[] {0, 1, 2, 3, 4, 4, 4, 4, 5, 5, 5, 5, 6, 7, 8}, plan.temporalPositions()); + assertArrayEquals( + new int[] {0, 1, 2, 3, 4, 4, 5, 5, 4, 4, 5, 5, 6, 7, 8}, plan.heightPositions()); + assertArrayEquals( + new int[] {0, 1, 2, 3, 4, 5, 4, 5, 4, 5, 4, 5, 6, 7, 8}, plan.widthPositions()); + // The prompt's largest coordinate (8) plus one minus the 15-token length: -6. + assertEquals(-6, plan.delta()); + + // The reference's single_image_t5 case: vision grid (5, 4, 4) with merge 2 (the LLM grid + // (5, 2, 2)), the same wrapper shape with the twenty image sentinels: 27 tokens in total. + // Here the frame count (5) is the block's largest dimension, so a block maximum computed + // without it would shift the trailing text to 6..8 and the delta to -18. + int[] t5Ids = new int[27]; + t5Ids[0] = 1000; + t5Ids[1] = 1001; + t5Ids[2] = 1002; + t5Ids[3] = START; + Arrays.fill(t5Ids, 4, 24, IMAGE); + t5Ids[24] = END; + t5Ids[25] = 1003; + t5Ids[26] = 1004; + Qwen3VlRopePlan t5Plan = + Qwen3VlPositionPlanner.plan(t5Ids, tokens(), List.of(new int[] {5, 2, 2})); + // The text run is again 0..3; the image block at 4..23 takes base 4: T is 4, 5, 6, 7, 8 + // by frame (five frames of four tokens) while H and W restart per frame, and the trailing + // text resumes at the block maximum (8) plus one. + assertArrayEquals( + new int[] { + 0, 1, 2, 3, 4, 4, 4, 4, 5, 5, 5, 5, 6, 6, 6, 6, 7, 7, 7, 7, 8, 8, 8, 8, 9, 10, 11 + }, + t5Plan.temporalPositions()); + assertArrayEquals( + new int[] { + 0, 1, 2, 3, 4, 4, 5, 5, 4, 4, 5, 5, 4, 4, 5, 5, 4, 4, 5, 5, 4, 4, 5, 5, 9, 10, 11 + }, + t5Plan.heightPositions()); + assertArrayEquals( + new int[] { + 0, 1, 2, 3, 4, 5, 4, 5, 4, 5, 4, 5, 4, 5, 4, 5, 4, 5, 4, 5, 4, 5, 4, 5, 9, 10, 11 + }, + t5Plan.widthPositions()); + // The prompt's largest coordinate (11) plus one minus the 27-token length: -15. + assertEquals(-15, t5Plan.delta()); + } + @Test void theTokensRecordValidatesItsIds() { assertEquals(new Qwen3VlPromptTokens(IMAGE, VIDEO, START, END), tokens()); @@ -562,6 +674,37 @@ void resolvePropagatesTheTokenizerExceptionForAMissingSentinel(@TempDir Path tem () -> Qwen3VlPromptTokens.resolve(tokenizer, IMAGE, VIDEO, START, END)); } + @Test + void resolveSkipsTheVideoLookupWhenTheConfigDeclaresNoVideoToken(@TempDir Path tempDir) + throws Exception { + // The config's -1 must not require the video sentinel in the vocabulary at all: it must + // resolve through a fixture variant that has no video token added. + HfTokenizer noVideoTokenizer = fixtureTokenizerMissingAddedToken(VIDEO, tempDir); + Qwen3VlPromptTokens resolved = + Qwen3VlPromptTokens.resolve(noVideoTokenizer, IMAGE, -1, START, END); + assertEquals(new Qwen3VlPromptTokens(IMAGE, -1, START, END), resolved); + // The -1 also resolves against the full fixture, where the video sentinel is present: the + // config's declaration wins over the vocabulary. + assertEquals( + new Qwen3VlPromptTokens(IMAGE, -1, START, END), + Qwen3VlPromptTokens.resolve(fixtureTokenizer(), IMAGE, -1, START, END)); + // The other three roles still resolve and cross-check with the config while the video + // role is absent from it. + for (String role : List.of("image", "vision start", "vision end")) { + int configImage = role.equals("image") ? 99 : IMAGE; + int configStart = role.equals("vision start") ? 99 : START; + int configEnd = role.equals("vision end") ? 99 : END; + IllegalArgumentException failure = + assertThrows( + IllegalArgumentException.class, + () -> + Qwen3VlPromptTokens.resolve( + noVideoTokenizer, configImage, -1, configStart, configEnd)); + assertTrue(failure.getMessage().contains(role), role + ": " + failure.getMessage()); + assertTrue(failure.getMessage().contains("99"), role + ": " + failure.getMessage()); + } + } + @Test void theFixtureDecodesEachSentinelIdBackToItsContractConstant() throws Exception { HfTokenizer tokenizer = fixtureTokenizer(); @@ -764,7 +907,7 @@ void thePlanBudgetArithmeticMatchesTheSevenPointThreeFamily() { } @Test - void theDecodeContinuationRejectsNegativePositions() { + void theDecodeContinuationRejectsCachePositionsBeforePrefill() { Qwen3VlPromptPlan plan = Qwen3VlPromptExpander.expand( new int[] {START, IMAGE, END, 7, START, IMAGE, END}, @@ -774,8 +917,7 @@ void theDecodeContinuationRejectsNegativePositions() { "cachePosition must be non-negative: -1", assertThrows(IllegalArgumentException.class, () -> plan.ropePositionAt(-1)).getMessage()); assertEquals( - "RoPE position at cache position 2 is negative: -1 (delta -3); decode starts at cache " - + "position 13", + "cachePosition 2 is inside the expanded prompt: decode starts at cache position 13", assertThrows(IllegalArgumentException.class, () -> plan.ropePositionAt(2)).getMessage()); } diff --git a/jmlx-tokenizer/src/main/java/se/alipsa/jmlx/tokenizer/HfTokenizer.java b/jmlx-tokenizer/src/main/java/se/alipsa/jmlx/tokenizer/HfTokenizer.java index 94ecaba2..2096f327 100644 --- a/jmlx-tokenizer/src/main/java/se/alipsa/jmlx/tokenizer/HfTokenizer.java +++ b/jmlx-tokenizer/src/main/java/se/alipsa/jmlx/tokenizer/HfTokenizer.java @@ -273,6 +273,20 @@ public List eosTokenIds(List eosTokens) { return List.copyOf(ids); } + /** + * Performs a plain exact name-to-ID lookup over this tokenizer's vocabulary, with no BOS/EOS or + * other special-token role semantics and no {@code tokenizer_config.json} consultation. Use this + * for ordinary vocabulary entries (for example added tokens such as vision sentinels); prefer the + * role-specific overloads when the token's meaning comes from configuration. {@code null} returns + * empty; an unknown non-null token throws. + * + * @param token vocabulary token name + * @return empty if {@code token} is null; otherwise its resolved id + */ + public OptionalInt tokenId(String token) { + return vocabularyTokenId(token, "HfTokenizer: no vocabulary entry for token"); + } + /** * Returns the tokenizer's contiguous id range. This is not the model's output-head size; obtain * that from the checkpoint's {@code config.json}. @@ -296,12 +310,16 @@ private OptionalInt fallbackSpecialTokenId(List names) { } private OptionalInt configuredTokenId(String token) { + return vocabularyTokenId( + token, "HfTokenizer: no vocabulary entry for configured special token"); + } + + private OptionalInt vocabularyTokenId(String token, String missingEntryMessage) { if (token == null) { return OptionalInt.empty(); } if (!vocabulary.hasToken(token)) { - throw new TokenizerException( - "HfTokenizer: no vocabulary entry for configured special token '" + token + "'"); + throw new TokenizerException(missingEntryMessage + " '" + token + "'"); } return OptionalInt.of(vocabulary.idOf(token)); } diff --git a/jmlx-tokenizer/src/test/java/se/alipsa/jmlx/tokenizer/HfTokenizerTest.java b/jmlx-tokenizer/src/test/java/se/alipsa/jmlx/tokenizer/HfTokenizerTest.java index 4d0640fb..d27b94fc 100644 --- a/jmlx-tokenizer/src/test/java/se/alipsa/jmlx/tokenizer/HfTokenizerTest.java +++ b/jmlx-tokenizer/src/test/java/se/alipsa/jmlx/tokenizer/HfTokenizerTest.java @@ -91,6 +91,15 @@ void exposesModelServingTokenIdsAndVocabularySize() { assertEquals(128001, llama.vocabSize()); } + @Test + void tokenIdIsANeutralVocabularyLookupWithoutSpecialTokenSemantics() { + HfTokenizer tokenizer = HfTokenizer.fromFile(fixture("qwen-style.tokenizer.json")); + assertEquals(OptionalInt.of(13), tokenizer.tokenId("low")); + assertEquals(OptionalInt.of(15), tokenizer.tokenId("the")); + assertThrows(TokenizerException.class, () -> tokenizer.tokenId("not-a-token")); + assertEquals(OptionalInt.empty(), tokenizer.tokenId(null)); + } + @Test void decodeSkipsAnAboveVocabIdInsteadOfAbortingTheWholeSequence() { // e.g. Qwen2.5's config.json vocab_size (152064) exceeds its tokenizer vocab (151665) -- a diff --git a/req/plans/phase7-3b-plan.md b/req/plans/phase7-3b-plan.md index 8871fc58..e327f12a 100644 --- a/req/plans/phase7-3b-plan.md +++ b/req/plans/phase7-3b-plan.md @@ -347,9 +347,9 @@ resolving each sentinel name to its ID through the tokenizer and cross-checking outer `config.json` ID (no hard-coded numeric IDs in production code) — the golden `tools/hf-reference/goldens/qwen3-vl-wp3-prompt.json` (8 cases * 2 `add_generation_prompt` variants, schema `jmlx-phase7-3b-wp3-qwen3vl-prompt-v1`) digest-pinned in -`tools/hf-reference/provenance.json`, the 33 pure-Java tests (no native bootstrap), the -accepted prompt grammar record, and the correction of the earlier sentinel assumption -(terminal display stripping; findings §6.8). Non-goals held: no native-pin change, no +`tools/hf-reference/provenance.json`, the pure-Java `Qwen3VlPromptExpanderTest` suite (no +native bootstrap), the accepted prompt grammar record, and the correction of the earlier sentinel +assumption (terminal display stripping; findings §6.8). Non-goals held: no native-pin change, no generated-binding edit, no loader and no RoPE kernel — those are later WPs. ## 6. Core RoPE and decoder integration (WP4) diff --git a/req/plans/phase7-3b-reference-findings.md b/req/plans/phase7-3b-reference-findings.md index ce1984aa..7369d731 100644 --- a/req/plans/phase7-3b-reference-findings.md +++ b/req/plans/phase7-3b-reference-findings.md @@ -655,7 +655,14 @@ no extra tokens. checked: an image run, cursor advance or coordinate that would overflow a signed 32-bit integer is rejected, and a grid that extends past the expanded prompt is rejected as a geometry/prompt mismatch — both before any array is written (the bounds-check ordering was - verified and fixed this session). + verified and fixed this session). The t > 1 (multi-frame / video frame) coordinate path — + T = base + frame index while H/W restart per frame — is implemented in both the Python + re-derivation (`qwen3vl_wp3_prompt.rederive_rope_all`) and the planner; since the pinned + WP3 golden is t = 1 only, the opt-in script + `tools/hf-reference/qwen3vl_rope_multiframe_check.py` (pinned venv, never Gradle-invoked, + no golden output) cross-checks the re-derivation against the real `get_rope_index` on + synthetic grids with t = 1, 2, 3 and 5 plus a two-image prompt with a text run between the + images, requiring exact equality of all three position axes and the delta. - **Plan record** (`Qwen3VlPromptPlan`, mirroring 7.3a's `SmolVlmPromptPlan`): owned copies of the original and expanded IDs, one contiguous `Qwen3VlImagePlacement` (start + count) per image in prompt order, the three position axes and the delta. The constructor rejects axes of @@ -694,7 +701,7 @@ no extra tokens. spans and deltas against the pinned WP1 probe golden, plus a string-level re-expansion and re-tokenization round-trip and a model `get_rope_index` comparison, exiting non-zero on any drift. - - `Qwen3VlPromptExpanderTest` (`jmlx-models`): 33 pure-Java tests (no native bootstrap, no + - `Qwen3VlPromptExpanderTest` (`jmlx-models`): pure-Java tests (no native bootstrap, no Tier-B download) over the golden's cases, the `resolve` match/mismatch/missing-token paths against the synthetic fixture `jmlx-models/src/test/resources/qwen3vl-tokens/tokenizer.json` (the four sentinel spellings at local IDs 10/11/12/13, `PROVENANCE.md` alongside) and diff --git a/tools/hf-reference/qwen3vl_rope_multiframe_check.py b/tools/hf-reference/qwen3vl_rope_multiframe_check.py new file mode 100644 index 00000000..1d8ba364 --- /dev/null +++ b/tools/hf-reference/qwen3vl_rope_multiframe_check.py @@ -0,0 +1,150 @@ +#!/usr/bin/env python3 +"""Phase 7.3b WP3 opt-in cross-check: multi-frame (t > 1) image RoPE. + +Runs under tools/hf-reference/.venv (transformers 4.57.6, torch 2.6.0, +Pillow 12.3.0, NumPy 2.5.3, Python 3.12), CPU, single-threaded, +deterministic. It is never invoked by Gradle and writes no golden: it +cross-checks qwen3vl_wp3_prompt.rederive_rope_all -- the independent +re-derivation of the 4.57.6 Qwen3VLModel.get_rope_index image path -- +against the real pinned reference method on synthetic image grids with +t = 1, 2, 3 and 5 plus a two-image prompt with a text run between the +images. The pinned WP3 golden is t = 1 only, so its cases never exercise +the per-frame T coordinate (T = base + frame index while H/W restart per +frame); this check covers that path, and the text run between the two +images of the last case exercises per-block starts as well. + +The synthetic prompts carry the pinned snapshot's real four vision token +IDs (read from its config.json) plus arbitrary non-colliding text IDs: +get_rope_index reads only config fields and token IDs, so no tokenizer or +images are needed. Each image block has the real chat-template shape -- +vision start, the image sentinel run, vision end -- because the reference +counts images as vision-start tokens whose next token is the image +sentinel, and the sentinel run has exactly t * (h/merge) * (w/merge) +tokens, as a real expansion produces. Every case requires exact integer +equality of all three position axes and the mrope delta; one line is +printed per case and the exit code is non-zero if any case mismatches. +""" +from __future__ import annotations + +import argparse +import json +import sys +from pathlib import Path +from types import SimpleNamespace + +import qwen3vl_probe as probe +from qwen3vl_wp3_prompt import CONFIG_FIELDS, rederive_rope_all + +AXIS_NAMES = ("t", "h", "w") + +# (name, segments): a segment is ("text", n) -- n text tokens -- or +# ("image", t, h, w) -- one vision-wrapped image block with grid (t, h, w). +CASES: list[tuple[str, list[tuple]]] = [ + ("single_image_t1", [("text", 3), ("image", 1, 4, 4), ("text", 2)]), + ("single_image_t2", [("text", 3), ("image", 2, 4, 4), ("text", 2)]), + ("single_image_t3", [("text", 3), ("image", 3, 4, 6), ("text", 2)]), + ("single_image_t5", [("text", 3), ("image", 5, 4, 4), ("text", 2)]), + ("two_images_text_between", + [("text", 3), ("image", 2, 4, 4), ("text", 4), ("image", 3, 6, 4), ("text", 2)]), +] + + +def build_ids(segments: list[tuple], tokens: dict, merge: int) -> tuple[list[int], list[list[int]]]: + """Expand the segments into a synthetic prompt plus its image grids. + + Text token IDs are fresh integers (1000 and up) that can never collide + with the four vision IDs; the image block is the vision start sentinel, + the run of t * (h/merge) * (w/merge) image sentinels and the vision end + sentinel. + """ + ids: list[int] = [] + grids: list[list[int]] = [] + next_text = 1000 + for segment in segments: + if segment[0] == "text": + for _ in range(segment[1]): + ids.append(next_text) + next_text += 1 + else: + _, t, h, w = segment + grids.append([t, h, w]) + ids.append(tokens["vision_start"]) + ids.extend([tokens["image"]] * (t * (h // merge) * (w // merge))) + ids.append(tokens["vision_end"]) + return ids, grids + + +def first_difference(expected: list[int], actual: list[int]) -> str | None: + for i, (e, a) in enumerate(zip(expected, actual)): + if e != a: + return f"first difference at index {i}: expected {e}, got {a}" + if len(expected) != len(actual): + return f"length {len(actual)} != expected {len(expected)}" + return None + + +def main() -> None: + parser = argparse.ArgumentParser(description=__doc__) + here = Path(__file__).resolve().parent + parser.add_argument("--dir", type=Path, + default=here.parent.parent / ".tier-b" / "qwen3-vl-2b-instruct") + args = parser.parse_args() + directory: Path = args.dir + if not (directory / "config.json").is_file(): + sys.exit(f"no config.json under {directory}; pass --dir to the pinned snapshot") + + probe.gate_versions() + import torch + from transformers.models.qwen3_vl.configuration_qwen3_vl import Qwen3VLConfig + from transformers.models.qwen3_vl.modeling_qwen3_vl import Qwen3VLModel + + torch.set_num_threads(1) + torch.use_deterministic_algorithms(True) + + config = json.loads((directory / "config.json").read_text()) + tokens = {role: int(config[field]) for role, field in CONFIG_FIELDS.items()} + merge = int(config["vision_config"]["spatial_merge_size"]) + + # get_rope_index reads only config fields, so a namespace carrying the + # real config exercises the actual method without loading weights (the + # same construction as qwen3vl_wp3_prompt.main). + rope_config = Qwen3VLConfig.from_pretrained(str(directory)) + for role, field in CONFIG_FIELDS.items(): + probe.check_facts("config", f"{role}_id", int(getattr(rope_config, field)), + tokens[role], "pinned snapshot config drifted from config.json") + probe.check_facts("config", "spatial_merge_size", + int(rope_config.vision_config.spatial_merge_size), merge) + rope_self = SimpleNamespace(config=rope_config) + + failures = 0 + for name, segments in CASES: + ids, grids = build_ids(segments, tokens, merge) + ids_tensor = torch.as_tensor([ids], dtype=torch.long) + grid_tensor = torch.as_tensor(grids, dtype=torch.long) + attention_mask = torch.as_tensor([[1] * len(ids)], dtype=torch.long) + position_ids, rope_deltas = Qwen3VLModel.get_rope_index( + rope_self, ids_tensor, grid_tensor, None, attention_mask=attention_mask) + actual = [[int(v) for v in row] for row in position_ids[:, 0].tolist()] + delta = int(rope_deltas[0][0].item()) + expected = rederive_rope_all(ids, grids, merge, tokens["image"]) + + diffs = [] + for axis_name, axis, expected_axis in zip(AXIS_NAMES, actual, expected["position_ids"]): + difference = first_difference(expected_axis, axis) + if difference is not None: + diffs.append(f"axis {axis_name}: {difference}") + if delta != expected["rope_delta"]: + diffs.append(f"rope_delta {delta} != expected {expected['rope_delta']}") + if diffs: + failures += 1 + print(f"{name}: grids={grids} length={len(ids)} MISMATCH {'; '.join(diffs)}") + else: + print(f"{name}: grids={grids} length={len(ids)} delta={delta} OK") + + if failures: + sys.exit(f"{failures}/{len(CASES)} cases mismatched get_rope_index") + print(f"all {len(CASES)} cases match get_rope_index exactly") + + +if __name__ == "__main__": + main() diff --git a/tools/hf-reference/qwen3vl_wp3_prompt.py b/tools/hf-reference/qwen3vl_wp3_prompt.py index 56569d02..d8a7a1de 100644 --- a/tools/hf-reference/qwen3vl_wp3_prompt.py +++ b/tools/hf-reference/qwen3vl_wp3_prompt.py @@ -80,16 +80,20 @@ def rederive_rope_all( """Independent re-derivation of the 4.57.6 get_rope_index image path. Encodes the exact algorithm for N images in prompt order: text runs get - all three axes equal (arange + st_idx); image runs get T = base, H = - base + h_idx (rows of llm_w), W = base + w_idx with base = st_idx + - text_len and llm grid = (t, h/merge, w/merge); st advances past the run; - every block starts at the previous block's maximum plus one; the delta is - the largest coordinate plus one minus the sequence length. + all three axes equal (arange + st_idx); an image's llm grid is + (t, h/merge, w/merge) and its t * lh * lw tokens get T = base + f for + every token of frame f, while H = base + h_idx (rows of llm_w) and + W = base + w_idx restart per frame, with base = st_idx + text_len -- the + reference's (t, h, w) index stack flattened t-major (see the arange/expand + construction at modular_qwen3_vl.py:939-943, shared with the video path); + st advances past the whole run (t * lh * lw); every block starts at the + previous block's maximum plus one; the delta is the largest coordinate + plus one minus the sequence length. """ positions: list[list[int]] = [[], [], []] st = 0 for t, h, w in grids: - lh, lw = h // merge, w // merge + lt, lh, lw = t, h // merge, w // merge ed = input_ids.index(image_token_id, st) text_len = ed - st st_idx = max(max(axis) for axis in positions) + 1 if positions[0] else 0 @@ -97,12 +101,13 @@ def rederive_rope_all( for axis in positions: axis.append(st_idx + i) base = st_idx + text_len - for hi in range(lh): - for wi in range(lw): - positions[0].append(base + 0) - positions[1].append(base + hi) - positions[2].append(base + wi) - st = ed + lh * lw + for frame in range(lt): + for hi in range(lh): + for wi in range(lw): + positions[0].append(base + frame) + positions[1].append(base + hi) + positions[2].append(base + wi) + st = ed + lt * lh * lw if st < len(input_ids): st_idx = max(max(axis) for axis in positions) + 1 if positions[0] else 0 for i in range(len(input_ids) - st): @@ -341,6 +346,10 @@ def run_variant(name: str, messages: list[dict], image_names: list[str], rope_self, ids_tensor, grid_tensor, None, attention_mask=attention_mask) actual = [[int(v) for v in row] for row in position_ids[:, 0].tolist()] delta = int(rope_deltas[0][0].item()) + # The golden contains only t = 1 (single-frame) image grids, so + # rederive_rope_all's multi-frame (t > 1) coordinates are verified + # by logic against the pinned reference's get_rope_index, not by + # golden coverage. expected = rederive_rope_all(expanded, grids, img_proc.merge_size, tokens["image"]) probe.check_facts(name, "position_ids", actual, expected["position_ids"], "get_rope_index != independent re-derivation")