Skip to content

refactor(minicpm5): move model-contract tests under tests/models - #706

Open
Aharrypotter wants to merge 4 commits into
UbiquitousLearning:mainfrom
Aharrypotter:refactor/minicpm5-test-layout
Open

refactor(minicpm5): move model-contract tests under tests/models#706
Aharrypotter wants to merge 4 commits into
UbiquitousLearning:mainfrom
Aharrypotter:refactor/minicpm5-test-layout

Conversation

@Aharrypotter

@Aharrypotter Aharrypotter commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Summary

Moves the MiniCPM5-1B model-contract tests out of tests/cpu into tests/models/minicpm5, following the test placement introduced by #704 for Qwen3.5.

This PR:

  • relocates MiniCPM5ConfigTest, MiniCPM5TokenizerTest, and MiniCPM5ModelTest to tests/models/minicpm5 with their own CMake targets and CTest registration (label minicpm5);
  • removes the three model-specific targets from tests/cpu/CMakeLists.txt, which now holds only kernel and backend-op coverage;
  • lets the config and model tests take an MLLM_MINICPM5_EXAMPLE_DIR override (same convention as Qwen35ConfigTest) so the same binaries can find the example config when executed on a device;
  • adds GqaDecodeKernelTest to the unified CPU kernel suite: the native KV-head grouped-query-attention decode kernel shipped with MiniCPM5 previously had only public-op coverage in tests/nn, no kernel-level oracle in tests/cpu;
  • registers the existing Mllm-Test-Nn-GroupedQueryAttention and Mllm-Test-Nn-KVHeadStaticCache executables with CTest (label nn-op); they were built but never run by ctest;
  • changes nothing under mllm/.

Reviewer focus

  1. Every MiniCPM5 test keeps its assertions; only its directory, CMake target location, and the example-directory lookup change.
  2. tests/cpu no longer registers any model-named target; the retained CPU targets (Mllm-Test-CPUKernel, Mllm-Test-KaiW4A32Pack, Mllm-Test-CPUContiguousOp) are untouched.
  3. The new kernel test enters through mllm::cpu::gqa_decode::fwdBhsdFp32 directly, with an independent double-accumulating scalar reference; it does not reconstruct the model or the public op.
  4. This PR adds no operation, kernel, or model capability and makes no performance claim.

Test placement

Test Protected contract Old location New location
MiniCPM5ConfigTest official 1B runtime contract, config/checkpoint match tests/cpu tests/models/minicpm5
MiniCPM5TokenizerTest digit pre-tokenizer, chat template, UTF-8 streaming, pinned official token IDs (env-gated) tests/cpu tests/models/minicpm5
MiniCPM5ModelTest native KV-head cache geometry, logical slots, reset tests/cpu tests/models/minicpm5

Kernel coverage added to tests/cpu

Case Contract proven
MatchesScalarReferenceAcrossFocusedMatrix scalar path, exact NEON blocks, qk/value-dim tails, single KV head, group sizes 1/2/8, 128-dim heads at several cache fills (tolerance 1e-4 against the reference)
NativeCacheViewAndTransposedQueryMatchContiguousBitwise a KV view inside a larger [B, Hkv, max_len, D] static cache plus a transposed [B, 1, Hq, D] query/output is addressed through the strides and is bitwise identical to the contiguous computation
GroupedSlicesMatchSingleHeadCallsBitwise every (batch, kv-head) group reproduces the per-head single-KV call bitwise, so the grouped scratch rows do not leak across heads or batches
RepeatedCallsAreBitwiseStable fixed accumulation order across repeats
RejectsInvalidGeometryAndStrides head-count mismatch, zero extents, null buffers, non-unit dimension strides, and non-positive strides return false without touching the output

The cases are registered in tests/cpu/KernelTest.cpp and added to the CPUKernelFocused CTest filter. The fixture follows the neighbouring GatedDeltaRuleKernelTest.hpp shape: a gqa_decode_kernel_test namespace with the reference, a StridedView (storage plus the [B, H, S, D] strides under test) built by purpose-named helpers for the contiguous, static-cache, and transposed layouts, explicit fwdBhsdFp32 calls in every case, and a thin forwarding fixture class. Public-op behaviour (trace, serialization, fallback, product geometries) stays covered by the existing tests/nn/GroupedQueryAttentionTest.cpp.

The MiniCPM5 model graph in mllm/models/minicpm5/modeling_minicpm5.hpp already composes registered nn layers only (GroupedQueryAttention, KVHeadStaticCache, RoPE, RMSNorm, Linear) and includes no backend kernel header, so the operation-registration half of #704 does not apply here.

Review map

  1. CMake ownership: tests/cpu/CMakeLists.txt (removals), tests/models/CMakeLists.txt, tests/models/minicpm5/CMakeLists.txt.
  2. Example-directory override: exampleDir() in tests/models/minicpm5/MiniCPM5ConfigTest.cpp and MiniCPM5ModelTest.cpp.
  3. Unchanged assertions: the three model-test bodies are byte-identical apart from the override helper.
  4. Kernel oracle: tests/cpu/GqaDecodeKernelTest.hpp (reference, strided run builders, case functions) and the TEST_F entries in tests/cpu/KernelTest.cpp.
  5. Public-op registration: the two add_test lines in tests/nn/CMakeLists.txt.

Validation

Validation is bound to commit 7c4223ce1debfacb0c1acee760352d3aecb43b17 (four commits on top of UbiquitousLearning/mllm:main@eef7dc2be28f54d5ee5d8079a8d3d1989878d31d). The model-test rows ran on the first commit d1a1a280, whose tests/models files are unchanged since; the ctest -L nn-op row ran on df85620d, whose registration lines are unchanged since; the kernel-test rows (host, NDK cross-build, device) were rerun on the exact 7c4223ce tree after the fixture restructure. All gates are ARM.

Validation (PASS) — macOS Apple Silicon host, Android NDK r28b cross-builds, and OnePlus 13T device runs for both the model tests and the kernel suite
Evidence class Result What it proves
Abstraction-boundary audit, git diff --check, clang-format --dry-run PASS 0 errors, 0 warnings; the diff is formatted and touches only tests/
macOS Apple Silicon build (Release, ARM backend, GCD threads) 3/3 CTest MiniCPM5{Config,Tokenizer,Model}Focused are registered from tests/models/minicpm5 and pass; ctest -N shows no MiniCPM5 target left under tests/cpu
macOS official-tokenizer oracle 4/4 with MLLM_MINICPM5_TOKENIZER_JSON set, pinned token IDs and the 200-token demo prompt match
Android NDK r28b arm64-v8a cross-build (API 28, -march=armv8.2-a+fp16+fp16fml+dotprod+i8mm, OpenMP) PASS the three test executables and mllm-minicpm5-runner configure, compile, and link; 8/8 artifacts are AArch64 ELF, 4/4 executables request /system/bin/linker64, NEEDED entries audited
OnePlus 13T (PKX110, Android 16, Snapdragon 8 Elite) focused device run Config 3/3, Tokenizer 4/4, Model 1/1 the arm64 bundle passes on-device with MLLM_MINICPM5_EXAMPLE_DIR pointing at the shipped example config and the official tokenizer; an invalid override directory makes the config test fail, proving the override is honored
macOS Apple Silicon Mllm-Test-CPUKernel GqaDecodeKernelTest.* 5/5; CPUKernelFocused CTest PASS the new oracle passes on the host NEON path and is executed by CTest
Android NDK r28b arm64-v8a cross-build of Mllm-Test-CPUKernel PASS the unified kernel test plus libMllmRT.so/libMllmCPUBackend.so compile and link; 5/5 artifacts AArch64, executable requests /system/bin/linker64, ISA flags present in the compile database
OnePlus 13T Mllm-Test-CPUKernel GqaDecodeKernelTest.* 5/5; focused filter 18/18 across 4 suites the kernel oracle and the existing causal-conv/GDN kernel cases pass on the device NEON path with the exact H20-built bundle
macOS Apple Silicon ctest -L nn-op 2/2 (GroupedQueryAttention 12 cases, KVHeadStaticCache 4 cases) the newly registered public-op tests are executed by CTest and pass; not rerun on Android, since the registration changes no binary

Dependencies for the cross-build came from a verified offline mirror (cache-assisted); product source, flags, and targets were unchanged.

Supported scope and limits

Changed by this PR: test file locations, CMake targets and CTest labels, the example-directory override in two tests, one new kernel-test fixture plus its registry entries, and CTest registration for two existing public-op tests.

Not changed: MiniCPM5 model graph, configuration, tokenizer, runner, conversion, CPU kernels, or any other model's tests.

Not claimed: new model capability, full-model generation on this commit (runtime bytes are unchanged from main), model-quality or performance results, or upstream CI (not run on this commit).

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests
    • Added focused validation for grouped-query attention decoding, including native cache layouts, strided inputs, grouped heads, repeatability, and invalid inputs.
    • Added dedicated MiniCPM5 coverage for configuration loading, tokenization, chat templates, UTF-8 streaming, and model behavior.
    • Added environment-based overrides for MiniCPM5 configuration and tokenizer test data.
    • Registered grouped-query attention and KV-cache neural-network tests.
    • Reorganized MiniCPM5 test targets into dedicated model test coverage.

Follow the test placement introduced by UbiquitousLearning#704: MiniCPM5 configuration,
tokenizer, and model-graph tests protect the model contract, not a CPU
kernel, so they now live in tests/models/minicpm5 with their own CMake
targets and CTest registration (label `minicpm5`). tests/cpu keeps only
kernel and backend-op coverage.

The config and model tests accept an MLLM_MINICPM5_EXAMPLE_DIR override
(same convention as Qwen35ConfigTest) so the binaries can locate the
example config when run outside the build host.

No change under mllm/; the MiniCPM5 model graph already composes
registered nn layers only.
@coderabbitai

coderabbitai Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Team

Run ID: 58633bad-2948-4422-a8b4-a7baf651704e

📥 Commits

Reviewing files that changed from the base of the PR and between df85620 and 7c4223c.

📒 Files selected for processing (1)
  • tests/cpu/GqaDecodeKernelTest.hpp

Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review.


📝 Walkthrough

Walkthrough

The changes add grouped-query attention decode coverage, register MiniCPM5 configuration, tokenizer, and model tests, add environment-based configuration paths, and register two NN-focused CTest cases.

Changes

Grouped-query attention decode tests

Layer / File(s) Summary
GQA decode oracle and validation cases
tests/cpu/GqaDecodeKernelTest.hpp
Adds scalar-reference validation, contiguous and strided layout checks, grouped-slice comparisons, repeat-stability checks, and invalid-input checks.
GQA test registration and focused execution
tests/cpu/KernelTest.cpp, tests/cpu/CMakeLists.txt
Adds five GoogleTest cases and includes them in the focused CPU test filter.

MiniCPM5 test suite

Layer / File(s) Summary
MiniCPM5 build wiring and configuration paths
tests/models/CMakeLists.txt, tests/models/minicpm5/CMakeLists.txt, tests/models/minicpm5/MiniCPM5ConfigTest.cpp, tests/models/minicpm5/MiniCPM5ModelTest.cpp
Registers three MiniCPM5 test targets and resolves example paths from MLLM_MINICPM5_EXAMPLE_DIR with a compile-time fallback.
MiniCPM5 tokenizer behavior cases
tests/models/minicpm5/MiniCPM5TokenizerTest.cpp
Adds tests for digit chunking, chat templates, incremental UTF-8 decoding, and optional exact tokenizer outputs.

NN focused test registration

Layer / File(s) Summary
NN CTest registrations
tests/nn/CMakeLists.txt
Registers grouped-query attention and static KV-head cache tests with the nn-op label.

Estimated code review effort: 4 (Complex) | ~45 minutes

Merge Risk: ⚪ Minimal · up to 7c422

This change reorganizes and registers MiniCPM5 and NN test coverage while adding focused GQA decode validation; no current merge-blocking risk remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 39 functions across 5 files. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the primary change: moving MiniCPM5 model-contract tests into tests/models. It is related to the main refactor, although it does not mention the additional ke…
Description check ✅ Passed The description is complete and directly related to the changes. It explains the test relocation, new kernel coverage, CMake and CTest updates, environment overrides, scope limits, and validation resu…
  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

The native KV-head grouped-query-attention decode kernel
(gqa_decode/fwd_bhsd.hpp) shipped with MiniCPM5 without kernel-level
coverage; only the public nn::GroupedQueryAttention tests exercised it.
Add GqaDecodeKernelTest.hpp following the neighbouring fixture shape:

- independent double-accumulating scalar reference over a focused
  geometry matrix (scalar path, exact NEON blocks, qk/value tails,
  single KV head, 128-dim heads at several cache fills);
- native static-cache view plus transposed [B, 1, H, D] query strides
  must match the contiguous computation bitwise;
- grouped (batch, kv-head) slices must match per-head single-KV calls
  bitwise, proving scratch rows do not leak across heads or batches;
- repeat stability and rejection of invalid geometry, null buffers, and
  unsupported strides without touching the output.

Register the cases in KernelTest.cpp and extend the CPUKernelFocused
filter so CTest runs them.
…Test

Both public-operation test executables introduced with MiniCPM5 were
built but never registered, so `ctest` skipped them. Register them the
same way as GatedDeltaRuleFocused, under the `nn-op` label.
@Aharrypotter
Aharrypotter marked this pull request as ready for review September 5, 2026 15:10
Replace the Run state bag and invoke() wrapper with a StridedView
(storage plus the [B, H, S, D] strides under test), purpose-named view
builders for the contiguous, static-cache, and transposed layouts, and
explicit fwdBhsdFp32 calls in every case so the kernel contract is
visible at the call site. Make the fixture class forward through
non-static members with namespace-qualified types, matching
GatedDeltaRuleKernelTest and CausalDepthwiseConvCurrentFirstKernelTest.
Case names and assertions are unchanged.
Aharrypotter added a commit to Aharrypotter/mllm that referenced this pull request Sep 6, 2026
Split the branch-local Ling3KDATest into the layers that the repository
now maintains separately (UbiquitousLearning#704/UbiquitousLearning#706):

- tests/cpu/KimiDeltaAttentionKernelTest.hpp: scalar-reference fixture
  for the KDA kernel (both gate variants, NEON lane blocks and tails,
  bitwise prefill-vs-tokenwise and serial-vs-parallel checks, argument
  validation), registered in KernelTest.cpp and the CPUKernelFocused
  ctest filter.
- tests/nn/KimiDeltaAttentionTest.cpp: public nn::KimiDeltaAttention
  contract through a Module (eager reference match including the
  16x128 production head geometry, in-place vs copied state, chunked
  prefill/decode equivalence, invalid geometry/options, trace plus
  option serialization round trip), registered with add_test.
- tests/models/ling3: config, tokenizer and RoPE tests with add_test
  registration, the `ling3` label, and an MLLM_LING3_EXAMPLE_DIR
  override for on-device runs.

The causal-convolution contract is covered by the upstream
tests/nn/CausalDepthwiseConv1DTest.cpp and the CausalDepthwiseConv
kernel suites, so the branch-local copies are removed.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Aharrypotter added a commit to Aharrypotter/mllm that referenced this pull request Sep 6, 2026
Split the branch-local Ling3KDATest into the layers that the repository
now maintains separately (UbiquitousLearning#704/UbiquitousLearning#706):

- tests/cpu/KimiDeltaAttentionKernelTest.hpp: scalar-reference fixture
  for the KDA kernel (both gate variants, NEON lane blocks and tails,
  bitwise prefill-vs-tokenwise and serial-vs-parallel checks, argument
  validation), registered in KernelTest.cpp and the CPUKernelFocused
  ctest filter.
- tests/nn/KimiDeltaAttentionTest.cpp: public nn::KimiDeltaAttention
  contract through a Module (eager reference match including the
  16x128 production head geometry, in-place vs copied state, chunked
  prefill/decode equivalence, invalid geometry/options, trace plus
  option serialization round trip), registered with add_test.
- tests/models/ling3: config, tokenizer and RoPE tests with add_test
  registration, the `ling3` label, and an MLLM_LING3_EXAMPLE_DIR
  override for on-device runs.

The causal-convolution contract is covered by the upstream
tests/nn/CausalDepthwiseConv1DTest.cpp and the CausalDepthwiseConv
kernel suites, so the branch-local copies are removed.
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.

1 participant