Repository navigation
Minicpm ocr - #983
Minicpm ocr#983michyu-amd wants to merge 21 commits into
Conversation
Wires the forked minicpm_ocr_npu engine into FastFlowLM: MiniCPM_OCR wrapper,
automodel.hpp include, the four all_models.hpp edits, the backend registration,
the CMake link, a model_list.json entry (name MiniCPM-OCR-1.0-NPU2, family
minicpm-ocr), the vendored .so, the nine xclbins and a test harness.
The xclbins are MiniCPM-V-4.7's, byte-identical: this model reuses Qwen3.5-0.8B's
text kernels and Qwen3.8-27B's vision kernels, so the port needed no new kernel
and no new MODEL_TYPE.
One real behaviour fix in modeling_minicpm_ocr_image.cpp. find_best_resize()
hardcoded `patch * 4`, which is the "16x" case:
image_processing_minicpmv4_6.py:
# "4x" merges one 2x2 block, "16x" merges twice
merge_factor = 2 if downsample_mode == "4x" else 4
MiniCPM-OCR is "4x" and merges once, so it needs `patch * 2`. Left as it was,
every image would be resized to a resolution the HF processor never picks, and
the token counts would drift with no error anywhere. `merge` is now derived from
MINICPM_OCR_DOWNSAMPLE_FACTOR and threaded through both call sites.
Verified on the NPU. The three-phase test passes end to end -- prompt, cached
follow-up, clear_context, thinking (17*23 = 391), and an image. Decode ~45-48
tok/s, prefill 131-580 tok/s. A 960x720 photo costs 1287 prompt tokens and a
783x646 text page reads back correctly, LaTeX included.
Image token counts match HF exactly (1260 for both images tested). The 4-token
prompt difference against HF is the empty <think>\n\n</think>\n\n block that
HF's template always inserts and FLM lets the model generate -- a chat-format
choice, not a mismatch.
The harness Makefile also needed common/prefix_slot_cache.cpp and -lwebp, which
automodel.cpp and image_reader.cpp reference unconditionally. No other test
harness lists either, so they share this gap.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every call site reaches fail_inference() from a `catch (...)`, so the engine's actual error -- an XRT failure, a bad shape, a throw from the vision tower -- was discarded and the user was told only "Inference failed; the current conversation was cleared". That cost us a diagnosis today: the very first run of the harness failed during decode at context length 28 with no further detail, and has not reproduced in 19 subsequent runs (10 short, 5 full four-turn, 4 ad hoc). With nothing to go on, the one occurrence is unrecoverable. std::current_exception() is still live inside a catch block, so a bare `throw;` recovers the message without changing control flow, and the guard keeps it safe if fail_inference() is ever reached outside a handler. Both paths verified in isolation: the rethrow recovers the original what(), and the no-exception case reports rather than terminating. The four-turn test still passes unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… set FLM_MINICPM_OCR_DUMP writes `tokens`, which insert() has already shortened by think_trim, so it ends at "<|im_start|>assistant\n" and is 2 tokens (thinking on) or 4 (off) shorter than the context the model actually sees -- decode() feeds the <think> tokens back so the KV checkpoint can land on a boundary the next turn reuses. Compared against HF's apply_chat_template() output, that reads as a prompt mismatch when there is none. It cost a long detour today: the rendered template is byte-identical to HF in both modes, which was only settled by rendering the same .jinja through minja directly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The harness ran four fixed turns, so prefill could only be measured at the one
length its image prompt happened to produce. --prompt-file runs a single turn on
a file's contents and exits, which is what made the GDN/conv prefill comparison
measurable across 1k to 24k tokens (models_simple/test_input/{1k,2k,8k}.txt,
concatenated for longer).
That mattered: at 1k the array looked 82% slower, at 24k only 10%. One length
would have given a badly wrong answer in either direction.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
With --prompt-file, --follow-up <file> sends a second turn the way flm serve sends one (whole conversation, restore_allowed), so the prompt cache prefills only from its saved head. That is the path where one prefill call can take a different GDN backend from the one before it, and it is what tested the host <-> array state handoff in both directions. Note the cache only splits a turn into head + tail when the head is at least min_boundary = 512 tokens; a shorter first turn is re-prefilled whole. Co-Authored-By: Claude <noreply@anthropic.com>
…gh the prefix slots Three conversations, each a first turn plus a follow-up: A a ~2k-token document (turn 1 runs the GDN on the NPU, the follow-up on the host), B a short prompt, C an image (skipped with --image ""). Phase 1 runs each one alone with the slots off (--rounds times, to see the engine repeat itself); phase 2 runs A1 B1 C1 A2 B2 C2 with the slots on and requires every follow-up to resume from its own snapshot, at the same point as the reference, with every reply byte identical; phase 3 repeats the interleaved order with the slots off to show what they save. Greedy, penalties off. First run (--Length 96): PASS. Follow-up prefill, slots on vs off when interleaved: A 336 vs 783 ms, B 149 vs 185 ms, C 313 vs 2015 ms. Co-Authored-By: Claude <noreply@anthropic.com>
With the prefix slots on, --follow-up's CLI-style turn one saves no snapshot, so turn two prefills the whole conversation in two calls (head, then tail). --resume sends turn one with restore_allowed as well, so turn two resumes from its snapshot and prefills only the reply and the new turn, like a cached follow-up in `flm serve`. The --follow-up comment said that already; it is corrected to say what --follow-up really does. Note: "Prefill time" in the statistics block accumulates across turns; turn two's own time is the difference. Co-Authored-By: Claude <noreply@anthropic.com>
The merge of main brought b4c2b90, which adds `continue_final_message` to AutoModel::apply_chat_template and updates every wrapper; MiniCPM_OCR only exists on this branch, so it no longer built ("marked override, but does not override"). Ported MiniCPM-V's changes from that commit by hand, as MiniCPM_OCR's wrapper is a fork of it: - the new apply_chat_template signature, rendering through _render_chat_template(); supports_continue_final_message() -> true - continue mode keeps every prompt token (think_trim 0) and feeds no think preamble in decode() - token_history drops a token the engine never ran (EOS not forwarded, or a turn cut short by max_tokens / cancel / the context cap) decode() and apply_chat_template() are now identical to MiniCPM-V's but for the class name. Only MiniCPM-OCR's files change. Checked on the NPU: the default 4-turn test gives the same replies as before, --multi-slot passes. Co-Authored-By: Claude <noreply@anthropic.com>
Replace the locally built libminicpm_ocr_npu.so and GateDeltaNet_prefill.xclbin with the IRON CI artifacts from AMD-ROCm-Internal/FastFlowLM_IRON PR #117 (run 40974217259, head 028476f9c), and add the missing XRT Windows dll/lib and the HRX so/dll/lib. Register minicpm_ocr_npu.dll in the inno and wix installers. The other MiniCPM-OCR xclbins (attn, conv, layer, lm_head, mm, vision_*) are byte-identical to the ones already shipped for Qwen3.5-0.8B / MiniCPM-V-4.7. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Engine (lib/xrt/libminicpm_ocr_npu.so) and new xclbins in xclbins/MiniCPM-OCR-1.0-NPU2/ from FastFlowLM_IRON branch minicpm_ocr: - vision_overlay.xclbin: vision attention + matmul A in one xclbin. - layer_with_lm_head.xclbin, layer_with_lm_head_kvb.xclbin: decode layers and lm_head in one design (no xclbin switch per token); the _kvb build reads finished K/V chunks as host-packed bfp16 and skips the y rescale when no row max moved (preferred when present). - attn_bfp16.xclbin: prefill attention on host-made bfp16 q / K / V^T tiles. - mm_bfp16_glu.xclbin: prefill matmul on host-made bfp16 x tiles, with the GLU computed on the array. - Engine: host prefill work under the array runs (row chunks, fused norms, range syncs), GDN host/array switch point 1536 -> 5120 tokens, a bfp16 KV copy for decode. Each new path has an env switch back to the old one. Speed (Strix Halo, balanced profile): prefill 124 tok 122 -> ~84 ms, 1K 383 -> ~230, 8K 2,218 -> ~1,380, 24K 7,878 -> ~4,400; decode 47.5 -> ~66.7 tok/s short, 26.7 -> ~46.4 at 24K. Golden replies changed only at exact or near ties; checked against a CPU fp32 reference. Local build, not the CI one. Co-Authored-By: Claude <noreply@anthropic.com>
Brings in the remote branch (main merge, CI-built Windows / HRX engine libs, CI GateDeltaNet_prefill.xclbin, model list, installer entries). Conflict in src/lib/xrt/libminicpm_ocr_npu.so resolved to the local engine (built from FastFlowLM_IRON minicpm_ocr, which also contains the remote IRON branch; rebuilding it from the merged IRON tree gives the same bytes). The CI GateDeltaNet_prefill.xclbin gives bit-identical prefill logits (512..24K). The Windows / HRX libs stay the CI builds of the earlier engine source. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…(CI run 42411780863) Linux/Windows libs for XRT and HRX and the 13 xclbins of minicpm-ocr:1.0, exactly as built by CI (IRON 796e61d52). conv.xclbin is unchanged. Checked on Strix Halo: 12/12 golden replies and prefill logits identical to the local build, multi-slot prompt cache passes, speed unchanged. Co-Authored-By: Claude <noreply@anthropic.com>
…ngine) Co-Authored-By: Claude <noreply@anthropic.com>
…e file The HF config.json was updated on 2026-11-04 (flm_version 1.0.12 -> 1.0.13, same size), so a fresh pull failed the hash check: "Failed to load minicpm-ocr:1.0 model!" in the PR's Windows HRX NPU tests. Co-Authored-By: Claude <noreply@anthropic.com>
… (HRX fixes) Same files as before from the IRON PR check run of 9124ee7a0, which fixes MiniCPM-OCR on HRX: no BO allocation on a second thread (HRX's single stream) and no row-chunk host/array overlap (HRX syncs whole BOs). The Linux XRT library is byte-identical to the previous one. Checked on Strix Halo: XRT golden 12/12 and prefill logits identical, multi-slot passes; HRX (CI HRX package) gives XRT's replies at 8K and 24K, npu_test chat suite 6/6. Co-Authored-By: Claude <noreply@anthropic.com>
tawei-amd
left a comment
There was a problem hiding this comment.
Automated review (not yet verified by running the code). Inline comments cover the findings; one more that can't be anchored to the diff:
- CI mapping not updated:
.github/script/ci/changes.py(changed-file -> family mapping) and the npu_testplan.pydefault-tag table have nominicpm-ocr/modeling_minicpm_ocrentries, while MiniCPM-V has entries in both. Later edits to the OCR sources or lib would trigger nominicpm-ocrNPU test job.
Separately, the only failing CI job (Test Linux XRT portable) is two chat/vision timeouts on qwen3.5-flash:2b, which this PR doesn't touch; likely a flake or runner NPU state, worth a re-run of that job.
🤖 Generated with Claude Code
| // re-prefills the whole prompt, and the engine is handed image rows with | ||
| // no pixels. | ||
| // ---------------------------------------------------------------------- | ||
| size_t prefix_skip_count = 0; |
There was a problem hiding this comment.
Prompt-cache image alignment is hand-rolled here instead of using AutoModel::_plan_reuse() (plan.skipped_media / plan.media_end), which modeling_minicpm_v.cpp uses for the same job. A picture is only dropped when the skipped prefix covers all of its pad rows. If the match ends inside a picture's pad span (partial match, or a slot resume landing mid-image), nothing is dropped from the payload, but _shared_insert/_prefix_insert still skip those rows, so the engine gets pixel views for skipped rows (or a view/row mismatch) and fails or corrupts the multimodal prefill. first_len_run below also hard-codes structural token ids instead of using the shared media spans.
| picture++; | ||
| } | ||
| if (picture != layouts.size()) { | ||
| header_print("ERROR", "the prompt has " << picture << " <|image_pad|> for " << layouts.size() << " images"); |
There was a problem hiding this comment.
A user-typed literal <|image_pad|> is encoded to the special pad id. These mismatch branches log and return false after TKOEN_ENCODE_TIME.start() without stopping the profiler, and without throwing or setting an error. The caller sees an empty/aborted generation instead of a 4xx, and the next request's timing is skewed. Too many pads reaches a generic return false with state partly mutated (input.images already cleared).
| this->fail_inference(); | ||
| } | ||
|
|
||
| checkpoint_his = token_history; |
There was a problem hiding this comment.
On the legacy path checkpoint_his = token_history and checkpoint() run even if _shared_insert returned false (cancelled / early failure). A cancelled prefill leaves the engine partial but records it as a valid checkpoint; the next restore_allowed request restores to that point and matches against a history that doesn't correspond to the KV rows.
| : std::vector<int>{ think_start_id, 271, think_end_id, 271 }; // <think>\n\n</think>\n\n | ||
| if (this->total_tokens + think_prefix.size() >= decode_cap) { | ||
| header_print("WARNING", "Max length reached, stopping generation..."); | ||
| meta_info.stop_reason = MAX_LENGTH_REACHED; |
There was a problem hiding this comment.
At the context cap decode() returns early without feeding the held-back think prefix that insert() trimmed, so those tokens never reach token_history/KV. The checkpoint taken in insert() is then misaligned with the next turn's rendered prompt (which includes the think tokens), and meta_info.generated_tokens isn't set on this path.
| this->enable_tool = true; | ||
|
|
||
| // MiniCPM-V ships no sampling defaults of its own; its text tower is | ||
| // Qwen3.5-0.8B's, so start from the Qwen3.5 non-thinking recommendation. |
There was a problem hiding this comment.
Copy-paste leftovers from MiniCPM-V: this comment says the text tower is 'Qwen3.5-0.8B's', the warning at ~l.98 says 'MiniCPM-V has no audio input', and enable_tool = true (l.24) is hard-wired for an OCR model. The Qwen non-thinking sampling defaults (top_k 20, temp 0.7, penalties) are untested for OCR, where greedy/low-temperature is usually wanted. This file is a ~470-line near-copy of modeling_minicpm_v.cpp and lacks the _plan_reuse fixes already on main; consider sharing the common code.
| "url": "https://huggingface.co/FastFlowLM/MiniCPM-OCR-1.0-NPU2/resolve/main", | ||
| "file_url": "https://huggingface.co/api/models/FastFlowLM/MiniCPM-OCR-1.0-NPU2/tree/main", | ||
| "ms_url": "https://modelscope.cn/models/amd/MiniCPM-OCR-1.0-NPU2", | ||
| "size": 1300000000, |
There was a problem hiding this comment.
size is 1300000000 (and footprint 2.4) but the listed files are model.q4nx (~1.07 GB) + vision_weight.q4nx (~0.93 GB) + tokenizer (~20 MB), about 2.0 GB. The pull/disk-space check and progress display will use the understated size.
| "details": { | ||
| "format": "NPU2", | ||
| "family": "minicpm-ocr", | ||
| "think": true, |
There was a problem hiding this comment.
think: true but no think_toggleable, while MiniCPM_OCR implements enable_think/toggle_think/reasoning_effort. The sibling minicpm-v-4.7 entry sets think_toggleable: true. As written, clients/CLI can't toggle thinking, so that path is unreachable.
| int main(int argc, char* argv[]) { | ||
| arg_utils::po::options_description desc("Allowed options"); | ||
| arg_utils::po::variables_map vm; | ||
| desc.add_options()("model,m", arg_utils::po::value<std::string>()->default_value("minicpm-ocr:1b"), "Model tag"); |
There was a problem hiding this comment.
Default model tag is minicpm-ocr:1b (also in the Makefile make test and WSL target), but the catalog entry added here is minicpm-ocr:1.0. get_model_path("minicpm-ocr:1b") fails, so the documented default test flow never runs.
|
|
||
| add_npu_test( | ||
| test_minicpm_ocr_npu | ||
| test/minicpm_ocr_npu |
There was a problem hiding this comment.
This target lists only model_backend.cpp and modeling_minicpm_ocr.cpp. The Makefile also needs modeling_minicpm_ocr_image.cpp, image_reader.cpp, lodepng, imageproc*.cpp, prefix_slot_cache.cpp, tokenizer.cpp, sampler.cpp, plus the jpeg/webp links. On the Windows/WSL path load_image, preprocess_image, image_placeholder and the PrefixSlotCache symbols are undefined at link time.
| # model folders. | ||
|
|
||
| # --- settings, override by exporting before sourcing ------------------------- | ||
| IRON_REPO="${IRON_REPO:-/scratch/michyu/Projects/FastFlowLM_IRON}" |
There was a problem hiding this comment.
Hard-codes a personal workspace (IRON_REPO=/scratch/michyu/..., FLM_MODEL_PATH=/scratch/michyu) and tells users to stage a locally built engine. This PR already ships the lib/xclbins under src/lib and src/xclbins/MiniCPM-OCR-1.0-NPU2. For anyone else source activate.sh fails with 'engine dir not found', and if it ran it would overwrite the CI-built libminicpm_ocr_npu.so, breaking the CI-built-byte-for-byte guarantee. The Makefile and header comments also carry internal paths and stage names (/scratch/michyu/minicpm-1b/CONTRACTS.md, S3/S6/S7).
CI failure root cause:
|
| symbol | kind | defined in |
|---|---|---|
op_mm, op_attn, all_channels |
STB_GNU_UNIQUE data (inline variables, same size 0x78/0x78/0x18) |
ocr, flash |
vision_overlay::gen_switch()::sw_regs, ::lock_regs |
STB_GNU_UNIQUE function statics |
ocr, flash |
vision_overlay::gen_switch, vision_overlay::prepend |
weak functions | ocr, flash |
Unique symbols are process-wide singletons, so there is one op_mm/op_attn/all_channels for both engines. ld.so initialises later-loaded libs first. OCR comes after Flash in flm-real's NEEDED list (libqwen3_5vl_flash.so ... libminicpm_ocr_npu.so), so OCR initialises first. The two overlays differ (vision_overlay.xclbin is 457,772 B in the OCR package and 399,404 B in the Flash package), so Flash's vision path appears to run with OCR's tables and the NPU hangs. That last step is inferred from the experiments below, not read from the code. Text-only Flash and stock qwen3.5:2b vision are not affected. Windows is not affected either (DLL symbols are not shared across libs), which matches the green Windows jobs.
Evidence (local NPU, CI-built portable artifacts: this PR = run 42429631495, main = run 42417942342)
One request: qwen3.5-flash:2b, a 64x64 red PNG, "What color is this? One word.", 25-90 s client timeout.
| setup | result |
|---|---|
| main CI build, clean NPU | OK 0.6 s, "red" (4 of 4) |
| PR CI build, clean NPU | timeout (3 of 3) |
PR tree + main's flm-real |
OK 0.6 s |
main tree + PR's flm-real (+ OCR lib present) |
timeout |
PR build, unmodified, LD_PRELOAD=libminicpm_ocr_npu.so (OCR loads first, so initialises last) |
OK 0.6 s, twice |
stock qwen3.5:2b image request |
OK 0.9 s |
Flash's engine lib and xclbins are byte-identical in the PR and main builds. Only flm-real, the catalog and the OCR files differ, and the flm-real swap moves the failure with it. Side effect: a hung Flash vision run leaves the NPU wedged for the next Flash vision attempt even after the process is killed. A stock qwen3.5:2b request clears it.
Fix plan
- Isolate the OCR engine in IRON (IRON#173). Move
op_mm,op_attn,all_channelsand thevision_overlaycode, including thegen_switchstatics, into an OCR-only namespace (for exampleocr_overlay_cfg::). hy-mem already does this withhy_overlay_cfg::. Only the OCR side needs to change; Flash is merged and stays as is. - Rebuild and re-vendor the OCR lib, and the xclbins if they change, from IRON CI into this PR. Re-run the full PR CI.
- Add a CI guard: scan the vendored engine libs and fail if two libs define the same GNU-unique or weak data symbol, minus an allow-list of known-benign ones (
cfg_sub,std::*,Dequant::Impl::IT,Gemm::Impl::shim_tiles,LMHead::Impl::lm_head_tiles). Next tod3-contractwould fit. - Verify beyond CI: Flash vision, then OCR vision, then Flash vision again in one server process; repeat with
LD_PRELOADforcing both library orders; then the full chat sweep on XRT and HRX. - Do not merge until the above is green. Reordering the link list is not a fix: it only swaps which engine breaks.
Reordering is also order-fragile in the other direction: in CI OCR works because it initialises first, so with the flipped order OCR vision would likely break instead (not tested).
Impact on other models
- Confirmed impact: only
qwen3.5-flash:2bvision. Every other model's tests in this PR's sweep passed (includingminicpm-v-4.7,minicpm-v-4.7-moe,qwen3vl,gemma4). - Latent, low risk: OCR also joins
Gemm::Impl::shim_tilesandLMHead::Impl::lm_head_tiles, generic unique statics that about 20 libs on main already define. This is an existing pattern; the tests pass, but I have not verified that those tables are identical across engines. - OCR shares weak helper code (
LM_Config,npu_app,buffer<>) with 34 libs. Some function sizes differ, but 48 same-named functions already differ among the existing libs, which looks like compiler variance on shared sources rather than a real fork. - Any future engine that copies the Flash vision overlay would hit the same clash without the guard in step 3.
Repro: unpack the CI portable artifact, FLM_MODEL_PATH=<models> x/flm serve qwen3.5-flash:2b, then send one chat completion with an image. The LD_PRELOAD variant is exec env LD_PRELOAD=$SCRIPT_DIR/lib/libminicpm_ocr_npu.so flm-real "$@" in the wrapper script.
🤖 Generated with Claude Code
FastFlowLM PR — MiniCPM-OCR
Settings: repo ROCm/FastFlowLM, base
main, headminicpm_ocr, ready for review (not draft).Title
feat: minicpm-ocr:1.0 (MiniCPM-OCR-1.0) engine, libs and xclbins from IRON#173
Body
Adds MiniCPM-OCR-1.0 (
minicpm-ocr:1.0, weightsFastFlowLM/MiniCPM-OCR-1.0-NPU2), a 1B OCR / vision-language modelwith the MiniCPM-V-4.7 text tower (hybrid GatedDeltaNet + attention).
App (
src/)modeling_minicpm_ocr{,_image}.cpp+ header: chat template, image preprocessing for OCR's merge factor, prompt cache(prefix slots), thinking on/off.
all_models.hpp,automodel.hpp,builtin_backends.cpp(familyminicpm-ocr),CMakeLists.txt,inno/flm.iss,wix/flm.wxs.model_list.json+model_info.jsonentries.src/test/minicpm_ocr_npu(--follow-up,--resume,--multi-slot,--prompt-file).Binaries, all from the IRON PR check run 42411780863 (FastFlowLM_IRON#173, commit 796e61d52), byte for byte:
libminicpm_ocr_npu.so,minicpm_ocr_npu.dll/.lib, for XRT (src/lib/xrt) and HRX (src/lib/hrx).src/xclbins/MiniCPM-OCR-1.0-NPU2/: the 13 xclbins of theminicpm-ocr:1.0kernel map.conv.xclbinis unused bythe engine and left as it was.
Measured on Strix Halo with these files (prompt tokens → prefill / decode):
Checked: 12 fixed text / multi-turn / image cases (incl. a German OCR page and a 960x720 photo) give the same
replies as the reference build, prefill logits bit-identical; cached follow-ups resume from the prompt cache;
interleaved conversations through the prefix slots pass. Windows
.dll/.libbuilt by CI, not run on Windows here.Other models are not touched: only MiniCPM-OCR files plus one line per wiring list.
🤖 Generated with Claude Code