Skip to content

Conversation

@Juanfi8
Copy link
Contributor

@Juanfi8 Juanfi8 commented May 7, 2025

Found an error when trying to use the aot_arm_compiler.py with a custom model defined in an external python file:

import torch

class ModelUnderTest(torch.nn.Module):
    def __init__(self):
        super().__init__()

    def forward(self, x):
        return x +x
    
ModelInputs = (torch.ones(5),)

Error message: TypeError: Module.eval() missing 1 required positional argument: 'self'

This patch fixes the problem.

cc @digantdesai @freddan80 @per @zingo @oscarandersson8218

Fixes: TypeError: Module.eval() missing 1 required positional argument: 'self'
@Juanfi8 Juanfi8 requested a review from digantdesai as a code owner May 7, 2025 15:12
@pytorch-bot
Copy link

pytorch-bot bot commented May 7, 2025

🔗 Helpful Links

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

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

❌ 5 New Failures

As of commit 6edf988 with merge base cebe051 (image):

NEW FAILURES - The following jobs have failed:

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 May 7, 2025
@github-actions
Copy link

github-actions bot commented May 7, 2025

This PR needs a release notes: label

If your changes are user facing and intended to be a part of release notes, please use a label starting with release notes:.

If not, please add the topic: not user facing label.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "topic: not user facing"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@zingo zingo added partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm ciflow/trunk labels May 8, 2025
@pytorch-bot
Copy link

pytorch-bot bot commented May 8, 2025

To add the ciflow label ciflow/trunk please first approve the workflows that are awaiting approval (scroll to the bottom of this page).

This helps ensure we don't trigger CI on this PR until it is actually authorized to do so. Please ping one of the reviewers if you do not have access to approve and run workflows.

@zingo
Copy link
Collaborator

zingo commented May 9, 2025

Hi thanks for your PR, this highlight a "bad" thing in our current setup and help texts, as we just used this method to test model internally I see we lack some sort of documentation/description on how file should look like. Down the line I was hoping to move out the small models in the aot compiler to small model file snipets like yours as a better example then how it works today when they are baked into it.

The idea was to set/use ModelUnderTest and ModelInputs as pre defined varable in the pyton file
e,g, you file could be changed to this:

import torch

class mymodel(torch.nn.Module):
    def __init__(self):
        super().__init__()

    def forward(self, x):
        return x +x

ModelUnderTest=mymodel()
ModelInputs = (torch.ones(5),)

If that is OK for you then the PR should not be needed :) But if you would like to submit a model like this as a new example maybe with a short text in the readm on how to run it we would appreciate it, if not we will hopefully (soon:ish) do it anyway down the line.

We are also open to other maybe better ways to handle this if you have any input that would be really nice, I feel that the ModelUnderTest and ModelInputs is a bit of a hack, kind of OK and solves the problem but maybe we could do it better :) It would be really cool if for some huggingface or torchaudion/video models you could just sue them and don't need to patch the files, but lets see what we manage to do :)

@Juanfi8
Copy link
Contributor Author

Juanfi8 commented May 12, 2025

Hi thanks for your PR, this highlight a "bad" think in our current setup and help texts, as we just used this method to test model internally I see we lack some sort of documentation/description on how file should look like. Down the line I was hoping to move out the small models in the aot compiler to small model file snipets like yours as a better example then how it works today when they are baked into it.

The idea was to set/use ModelUnderTest and ModelInputs as pre defined varable in the pyton file e,g, you file could be changed to this:

import torch

class mymodel(torch.nn.Module):
    def __init__(self):
        super().__init__()

    def forward(self, x):
        return x +x

ModelUnderTest=mymodel()
ModelInputs = (torch.ones(5),)

If that is OK for you then the PR should not be needed :) But if you would like to submit a model like this as a new example maybe with a short text in the readm on how to run it we would appreciate it, if not we will hopefully (soon:ish) do it anyway down the line.

Thanks for the explanation :) Now it is clear how this should work. I will submit another PR with the example model in the example/models directory, so it will be clearer for others.

We are also open to other maybe better ways to handle this if you have any input that would be really nice, I feel that the ModelUnderTest and ModelInputs is a bit of a hack, kind of OK and solves the problem but maybe we could do it better :) It would be really cool if for some huggingface or torchaudion/video models you could just sue them and don't need to patch the files, but lets see what we manage to do :)

I actually find that the hack works nicely, it is easy enough to test new custom models with this backend. I would only suggest a bit more documentation, but It guess it will come with time and project maturation :)

@Juanfi8 Juanfi8 closed this May 12, 2025
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants