Skip to content

Conversation

@Erik-Lundell
Copy link
Collaborator

@Erik-Lundell Erik-Lundell commented Jan 29, 2025

The pass differs from existing fuse passes since they
use the get_attr node which is not supported by ArmBackend.
Instead, we update the existing parameters.
Also adds tests.

To test the batchnorm pass, I extended RunPasses, adding a parameter,
passes_with_exported_program, that can be used just
like pass_list but are initiated with an
exported program before they are run.

The functionality is tested in new tests for the
CastInt64Pass and InsertTableOpsPass

cc @digantdesai @freddan80 @per @zingo @oscarandersson8218

ExportPasses that need to be initated with
an ExportedProgram can currently not be tested
in a convenient way.

This patch subclasses RunPasses, adding a parameter,
`passes_with_exported_program`, that can be used just
like `pass_list` but are initiated with an
exported program before they are run.

The functionality is tested in new tests for the
CastInt64Pass and InsertTableOpsPass

Signed-off-by: Erik Lundell <[email protected]>
Change-Id: I1712d86abe7cc3672c343db568df1264c0b9133e
The pass differs from existing fuse passes since they
use the get_attr node which is not supported by ArmBackend.
Instead, we update the existing parameters.
Also adds tests.

Change-Id: Iad6d70e632191d74d96df62b1837d37fe60e7d3a
@Erik-Lundell Erik-Lundell added partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm ciflow/trunk topic: not user facing labels Jan 29, 2025
@pytorch-bot
Copy link

pytorch-bot bot commented Jan 29, 2025

🔗 Helpful Links

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

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

❌ 1 New Failure, 2 Cancelled Jobs, 1 Pending

As of commit d3c6a60 with merge base c5fea7e (image):

NEW FAILURE - The following job has failed:

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

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

@facebook-github-bot facebook-github-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 Jan 29, 2025
@manuelcandales manuelcandales added the triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module label Jan 29, 2025
@zingo
Copy link
Collaborator

zingo commented Jan 31, 2025

macos unrealted

@zingo zingo merged commit 5f849ca into pytorch:main Jan 31, 2025
104 of 107 checks passed
Copy link
Contributor

@digantdesai digantdesai left a comment

Choose a reason for hiding this comment

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

Dang missed the train on this one :p

self.add_pass(ConvertMeanDimToAveragePoolPass())
self.add_pass(DecomposeDivPass())
self.add_pass(DecomposeSoftmaxesPass())
self.add_pass(FuseBatchnorm2DPass(exported_program))
Copy link
Contributor

Choose a reason for hiding this comment

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

Only for MI?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is already done in the BI case, in prepare_pt2e I believe

from executorch.backends.arm.test.tester.arm_tester import ArmTester, RunPasses


class Int64Model(torch.nn.Module):
Copy link
Contributor

Choose a reason for hiding this comment

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

what makes it int64?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A scalar always becomes int64

z = self.conv2d2(x)
a = self.batch_norm2d(
y
) # Can't be fused since paramters of conv2d2 have multiple users.
Copy link
Contributor

Choose a reason for hiding this comment

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

why is this a real constraint? i.e.

y = conv2d2(x1)
a = bn2d(y)
return a

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Since we update the weights of the convolution, it will produce different output. The new output will only be correct for the user that had the bn.

@Erik-Lundell Erik-Lundell deleted the fold_batch_norm branch July 31, 2025 07:51
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. partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm topic: not user facing triaged This issue has been looked at a team member, and triaged and prioritized into an appropriate module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants