Skip to content

Keep DS-CNN example inputs portable-safe - #21619

Merged
shoumikhin merged 1 commit into
mainfrom
fix/ds-cnn-portable-memory-format
Aug 6, 2026
Merged

Keep DS-CNN example inputs portable-safe#21619
shoumikhin merged 1 commit into
mainfrom
fix/ds-cnn-portable-memory-format

Conversation

@JacobSzwejbka

@JacobSzwejbka JacobSzwejbka commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Summary

PR #21485 moved the shared DS-CNN example input to channels-last to satisfy the Cortex-M backend. That regressed the portable model test because portable convolution expects the input's standard NCHW dim order.

DS-CNN's input shape is (1, 1, 49, 10). With a singleton channel, its original NCHW tensor reports both contiguous and channels-last contiguous even though its strides remain (490, 490, 10, 1). The Cortex-M compiler's conditional conversion therefore did not materialize channels-last strides.

  • Restore the shared DS-CNN example input to standard contiguous NCHW layout.
  • Always request channels-last for 4D tensors inside the Cortex-M compiler.
  • Propagate the converted example inputs for downstream bundle/reference generation.

This ports the release/1.4 fix from #21563 to main.

Test plan

  • python3 -m py_compile examples/models/mlperf_tiny/ds_cnn.py backends/arm/scripts/aot_arm_compiler.py
  • git diff --check
  • Loaded the wrapper with PyTorch 2.13 and verified:
    • portable input shape (1, 1, 49, 10)
    • portable stride (490, 490, 10, 1)
    • Cortex-M channels-last stride (490, 1, 10, 1)
    • converted values are unchanged
  • Portable and Cortex-M end-to-end coverage delegated to CI.
  • The identical change is already running on release/1.4 and passed its full 20-wheel matrix.

Authored with Codex.

cc @digantdesai @freddan80 @per @zingo @oscarandersson8218 @mansnils @Sebastian-Larsson @robell @rascani

Restore the shared DS-CNN example input to standard contiguous layout so the portable convolution kernel receives NCHW dim order. Convert and propagate four-dimensional inputs as channels-last inside the Cortex-M compiler, including singleton-channel tensors that satisfy both contiguity checks.\n\nThis ports the release/1.4 fix from #21563 to main.\n\nAuthored with Codex assistance.
@pytorch-bot

pytorch-bot Bot commented Aug 6, 2026

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/executorch/21619

Note: Links to docs will display an error until the docs builds have been completed.

❌ 2 New Failures, 2 Cancelled Jobs, 26 Pending, 24 Unrelated Failures, 1 Unclassified Failure

As of commit aab62df with merge base 85ade9f (image):

NEW FAILURES - The following jobs have failed:

UNCLASSIFIED FAILURE - DrCI could not classify the following job because the workflow did not run on the merge base. The failure may be pre-existing on trunk or introduced by this PR:

CANCELLED JOBS - The following jobs were cancelled. Please retry:

FLAKY - The following jobs failed but were likely due to flakiness present on trunk:

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 6, 2026
@github-actions github-actions Bot added ciflow/trunk module: arm Issues related to arm backend labels Aug 6, 2026
@shoumikhin

Copy link
Copy Markdown
Contributor

does release/1.4 need this too? #21485 was cherry-picked there as #21498, so the ds_cnn portable break probably rode along with it.

return (
(torch.rand(1, 1, 49, 10) * 2 - 1).to(memory_format=torch.channels_last),
)
return (torch.rand(1, 1, 49, 10) * 2 - 1,)

@digantdesai digantdesai Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

this is based on the implicit, always-do-to-nhwc call for cortex-m, I guess? should we add a test for that?

@shoumikhin
shoumikhin merged commit 74f1ce8 into main Aug 6, 2026
363 of 511 checks passed
@shoumikhin
shoumikhin deleted the fix/ds-cnn-portable-memory-format branch August 6, 2026 19:38
@shoumikhin

Copy link
Copy Markdown
Contributor

merged to main.

two things are still open and would be lost here, so noting them:

  1. digant's question above about adding a test for the cortex-m nhwc conversion.
  2. does release/1.4 need this too? Fix DS-CNN Cortex-M input memory format #21485 went there as Fix DS-CNN Cortex-M input memory format #21498.

the ds_cnn portable job runs from the nightly workflow, not from pull or trunk, so the next scheduled run is the first place this shows up as green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ciflow/trunk CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. module: arm Issues related to arm backend

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants