Skip to content

Conversation

@tom-arm
Copy link
Collaborator

@tom-arm tom-arm commented Oct 2, 2024

  • Adds GenericModelEvaluator, which gathers metrics applicable to all models
  • Adds --evaluate option to enable gathering quantization metrics

Signed-off-by: Tom Allsop tom.allsop@arm.com

Change-Id: Ia9b591841f188870fa5e62d0568169498301393d

* Adds GenericModelEvaluator, which gathers metrics applicable to all models
* Adds --evaluate option to enable gathering quantization metrics

Signed-off-by: Tom Allsop <tom.allsop@arm.com>

Change-Id: Ia9b591841f188870fa5e62d0568169498301393d
@pytorch-bot
Copy link

pytorch-bot bot commented Oct 2, 2024

🔗 Helpful Links

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

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

❌ 1 New Failure

As of commit c50af6d with merge base da1d2ca (image):

NEW FAILURE - The following job has 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 Oct 2, 2024
@tom-arm tom-arm added partner: arm For backend delegation, kernels, demo, etc. from the 3rd-party partner, Arm ciflow/trunk labels Oct 2, 2024
Comment on lines 43 to 44
fp32_output = self.fp32_model(*self.example_input)
int8_output = self.int8_model(*self.example_input)
Copy link
Contributor

Choose a reason for hiding this comment

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

so run these in eager?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, they do, this appears to be the best option particularly as we will want to extend this to use larger datasets (other options such as FVP and reference model are not practical for this use case). I'd be interested to know if you have any concerns about using eager for this :)

random.seed(0)

# Create an input that is hard to compress
COMPRESSION_RATIO_TEST = bytearray(random.getrandbits(8) for _ in range(1000000))
Copy link
Contributor

Choose a reason for hiding this comment

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

torch.rand() fp32 dtype -> save it as bytesio?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

test_get_compression_ratio tests a file on a filesystem, so not sure if bytesio works here (I understand it's for writing in memory). Unless I'm missing something?

I can look into using torch.rand() instead.

import torch


class GenericModelEvaluator:
Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking out loud, how does it or should it belong in Tester? Or some other util rather than scripts?

Even better I can see this useful for any delegate not just arm, what do you think about that?

Copy link
Collaborator Author

@tom-arm tom-arm Oct 3, 2024

Choose a reason for hiding this comment

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

I agree this could be moved to a more appropriate folder in backends/arm. I will take your suggestion of util .

On moving this out entirely. I would personally like to add more functionality first to see how this shapes out before moving it out. In particular, add a model specific evaluator.

Change-Id: Idf8be511477ecf43262ee6e8d0f1b3fb95436339
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.

Stamping to unblock, COMPRESSION_RATIO_TEST generation seems inefficient let's fix it in the future?

Also once matures move it out to backends/

@facebook-github-bot
Copy link
Contributor

@digantdesai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@tom-arm
Copy link
Collaborator Author

tom-arm commented Oct 8, 2024

Stamping to unblock, COMPRESSION_RATIO_TEST generation seems inefficient let's fix it in the future?

Also once matures move it out to backends/

Thanks Digant! Yep this could be more efficient, I'll make sure this is addressed in the next patch for this feature

@digantdesai
Copy link
Contributor

rebase please? Landing a bunch of stuff tonight, so probably tomorrow? :)

Change-Id: Ib882873a35f139c4c0463b0ec9009d24015a12a2
@facebook-github-bot
Copy link
Contributor

@digantdesai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

1 similar comment
@facebook-github-bot
Copy link
Contributor

@digantdesai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@digantdesai merged this pull request in e91357d.

@tom-arm tom-arm deleted the add_aot_compiler_metrics branch September 18, 2025 09:26
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. Merged 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.

4 participants