Adding model stats to aot_arm_compiler #5816
Conversation
* Adds GenericModelEvaluator, which gathers metrics applicable to all models * Adds --evaluate option to enable gathering quantization metrics Signed-off-by: Tom Allsop <[email protected]> Change-Id: Ia9b591841f188870fa5e62d0568169498301393d
🔗 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 FailureAs of commit c50af6d with merge base da1d2ca ( NEW FAILURE - The following job has failed:
This comment was automatically generated by Dr. CI and updates every 15 minutes. |
facebook-github-bot
added
the
CLA Signed
tom-arm
added
partner: arm
| fp32_output = self.fp32_model(*self.example_input) | ||
| int8_output = self.int8_model(*self.example_input) |
There was a problem hiding this comment.
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)) |
There was a problem hiding this comment.
torch.rand() fp32 dtype -> save it as bytesio?
There was a problem hiding this comment.
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: |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
|
@digantdesai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Thanks Digant! Yep this could be more efficient, I'll make sure this is addressed in the next patch for this feature |
|
rebase please? Landing a bunch of stuff tonight, so probably tomorrow? :) |
Change-Id: Ib882873a35f139c4c0463b0ec9009d24015a12a2
|
@digantdesai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
1 similar comment
|
@digantdesai has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
@digantdesai merged this pull request in e91357d. |
Signed-off-by: Tom Allsop [email protected]
Change-Id: Ia9b591841f188870fa5e62d0568169498301393d