Skip to content

[Fix][Relax] Validate non-concat axes before fusing parallel matmul biases - #20243

Open
OmarAzizi wants to merge 2 commits into
apache:mainfrom
OmarAzizi:fix-combine-parallel-matmul
Open

[Fix][Relax] Validate non-concat axes before fusing parallel matmul biases#20243
OmarAzizi wants to merge 2 commits into
apache:mainfrom
OmarAzizi:fix-combine-parallel-matmul

Conversation

@OmarAzizi

Copy link
Copy Markdown
Contributor

CombineParallelMatmul fused parallel matmul biases by checking rank alone, not whether their non-concat axes actually matched, so incompatible shapes like [1, 4] and [2, 5] slipped through and crashed in concat.

Added a check that validates the non-concat axes and skips fusing the bias when they don't match.

Fixes #20205.

if (bias_shape_unknown) {
return ffi::Map<Var, Expr>{};
}
if (!shapes_compatible_excluding_trailing_axes(bias_shapes, 1)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also require each bias’s last dimension to match the corresponding splits[i].split_size. For weights [3,4]/[3,5] and biases [2,1]/[2,1], both original adds are valid via broadcasting, but this check passes and produces a [2,2] concatenated bias that cannot broadcast to the combined [2,9] output. This case should skip fusion, with a regression test added.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, I added the check and a new regression test in a c67ceca.

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.

[Bug][Relax] CombineParallelMatmul fails valid branches with broadcast-compatible biases

2 participants