fix: reduce A5 validation false failures#901
Open
HecreReed wants to merge 2 commits into
Open
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request updates several test samples and the validation runtime. Specifically, it changes the data type from float16 to float32 in the Partmin sample, introduces 32x1 tile views and column-major layouts in the Rowexpand samples, and refactors the Sels sample to use a u8 mask instead of float/integer comparisons. Additionally, it simplifies mask packing and comparison logic in validation_runtime.py by removing is_a5_soc() checks. Feedback on scatter_golden.py suggests simplifying the idx array creation and assignment by avoiding redundant reshape and slice operations.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
29
to
33
| idx = np.arange(n_idx, dtype=np.int64).reshape(rows, cols) | ||
| out = src.reshape(rows, cols) | ||
| buffers = default_buffers(meta) | ||
| buffers[src_name] = src | ||
| buffers[idx_name] = idx.astype(meta.np_types[idx_name], copy=False).reshape(-1)[:n_idx] |
Contributor
There was a problem hiding this comment.
The idx array is reshaped to (rows, cols) only to be flattened back to a 1D array and sliced with [:n_idx] when assigned to buffers[idx_name]. Since idx is already a 1D array of size n_idx, we can simplify this by keeping idx as a 1D array and avoiding the redundant reshape and slice operations.
Suggested change
| idx = np.arange(n_idx, dtype=np.int64).reshape(rows, cols) | |
| out = src.reshape(rows, cols) | |
| buffers = default_buffers(meta) | |
| buffers[src_name] = src | |
| buffers[idx_name] = idx.astype(meta.np_types[idx_name], copy=False).reshape(-1)[:n_idx] | |
| idx = np.arange(n_idx, dtype=np.int64) | |
| out = src.reshape(rows, cols) | |
| buffers = default_buffers(meta) | |
| buffers[src_name] = src | |
| buffers[idx_name] = idx.astype(meta.np_types[idx_name], copy=False) |
Codex Review该评论由 review 机器人自动更新。
SummaryReview failed at stage Findings未生成结构化 findings，因为 review 过程提前失败。 Log Tail |
Collaborator
Author
|
/run a5 |
|
已接收
页面会自动刷新，可以直接看当前阶段、排队情况和最近结果。 |
A5 板测失败
失败用例
|
A5 板测失败详情：PR #901rope_kv_cache
rmsnorm
post_rmsnorm
plan_memory_bind_tile_alias_liveness
mscatter
mgather
|
HecreReed
force-pushed
the
codex/a5-validation-false-fail-fixes
branch
from
a61d28e to
447494e
Compare
Collaborator
Author
|
/run a5 rope_kv_cache,rmsnorm,post_rmsnorm,plan_memory_bind_tile_alias_liveness,mscatter,mgather |
|
已接收
页面会自动刷新，可以直接看当前阶段、排队情况和最近结果。 |
A5 板测失败
失败用例
|
A5 板测失败详情：PR #901mscatter
mgather
|
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
2 participants
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.
Summary
test/samples/validation_runtime.pyso mask buffers use row-strided storage instead of a dense prefixSels,Rowexpand{mul,sub,div},Scatter, andPartminsample/golden pairs so their inputs and expected outputs match the actual A5 tile-op contractsWhat this draft is trying to fix
This draft is aimed at the broad-run failures that look like precision bugs but are actually sample/harness mismatches:
cmp/cmps: packed mask layout mismatch in compare/golden handlingsel: packed mask input layout mismatchsels: sample/golden semantic mismatchrowexpandmul/rowexpandsub/rowexpanddiv: sample used an invalid full-tile broadcast operand while golden assumed per-row broadcast semanticsscatter: golden interpreted indices as row destinations instead of element indicespartmin: golden/compare usedfloat16even though the sample output isfloat32Not covered yet
This draft does not try to solve the remaining broad-run failures yet:
mgather/mscattercompile failuresrope_kv_cachermsnorm/post_rmsnormtest_tmov_col_major_16x1_align_a5segfaultValidation
python3 -m py_compile test/samples/validation_runtime.py test/samples/Sels/sels.py test/samples/Sels/sels_golden.py test/samples/Rowexpandmul/rowexpandmul.py test/samples/Rowexpandsub/rowexpandsub.py test/samples/Rowexpanddiv/rowexpanddiv.py test/samples/Scatter/scatter_golden.py test/samples/Partmin/partmin_golden.py test/samples/Partmin/partmin_compare.pygit diff --check