Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

[Kernels] Fix static conv width-padding boundaries - #6788

#6788
Open
blinding-pixels wants to merge 2 commits into
modular:mainmodular/modular:mainfrom
blinding-pixels:agent/fix-static-conv-width-paddingblinding-pixels/modular:agent/fix-static-conv-width-paddingCopy head branch name to clipboard
Open

[Kernels] Fix static conv width-padding boundaries#6788
blinding-pixels wants to merge 2 commits into
modular:mainmodular/modular:mainfrom
blinding-pixels:agent/fix-static-conv-width-paddingblinding-pixels/modular:agent/fix-static-conv-width-paddingCopy head branch name to clipboard

Conversation

@blinding-pixels

Copy link
Copy Markdown

Linked issue

Fixes #6712

(#6712 is currently unassigned and Needs Triage — no maintainer review yet. Opening this to surface the concrete fix; happy to hold for direction on the approach.)

Type of change

  • Bug fix (non-breaking change that fixes an issue)

Motivation

The static conv padding path miscomputes outputs near the padding boundary. At wo=16 (K=8) it returns 23 where the correct value is 21, because the static path doesn't derive per-tile padding adjustments the way the non-static path does. (I picked this up as an entry point into the conv kernels — small and self-contained.)

What changed

Compute the left and right padding-impact boundaries from the convolution's padding, stride, dilation, input width, and kernel width — the same geometry the non-static path already uses. The static path keeps its fixed-width micro-kernel tiles, but now: marks every tile that has a true padding boundary, passes the tile's output-width offset into _inner_loops_static, and computes each tap's padding adjustment relative to that tile offset. Net −71 lines in conv.mojo (37 added, 108 removed).

Testing

Added a deterministic 16-case regression sweep. All-ones inputs with packed filters give an analytical oracle: each output must equal the exact number of valid taps times the channel count, which checks every output element and inherently rejects NaNs. The fixed kernel passes all 16 cases exactly. As a negative control, the same test fails against the original implementation in 3/3 uncached runs at the K=8 boundary (wo=16, expected 21, actual 23).

Checklist

  • The linked issue above has been reviewed by a maintainer and is agreed-upon, or this is a trivial fix that does not need prior approval
  • PR is small and focused
  • I ran ./bazelw run format to format my changes
  • I added or updated tests to cover my changes
  • If AI tools assisted with this contribution, I have included an Assisted-by: trailer

Assisted-by: AI


BEGIN_PUBLIC
[Kernels] Fix static conv width-padding boundaries

Derive tile padding boundaries from convolution geometry.

Assisted-by: AI
END_PUBLIC

Signed-off-by: blinding-pixels <281499151+blinding-pixels@users.noreply.github.com>
@blinding-pixels
blinding-pixels marked this pull request as ready for review July 18, 2026 08:12
@blinding-pixels
blinding-pixels requested a review from a team as a code owner July 18, 2026 08:12
Signed-off-by: blinding-pixels <281499151+blinding-pixels@users.noreply.github.com>
@github-actions

github-actions Bot commented Jul 18, 2026

Copy link
Copy Markdown

All contributors have signed the CLA ✍️ ✅
Posted by the CLA Assistant Lite bot.

@blinding-pixels

Copy link
Copy Markdown
Author

I have read the CLA Document and I hereby sign the CLA

modular-cla-bot Bot added a commit to modular/cla that referenced this pull request Jul 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]: conv2d on CPU returns silently wrong / nondeterministic output with large width-padding

1 participant

Morty Proxy This is a proxified and sanitized view of the page, visit original site.