-
Notifications
You must be signed in to change notification settings - Fork 584
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support device detection with new benchmark suite #13182
Conversation
ff54f00
to
8b90d01
Compare
Abbreviated Benchmark Summary@ commit a3449ae47ab17b896b99ab336441c486908b2050 (vs. base f2fc7c5bb95a5e59f540d201474e53563f109f96) No improved or regressed benchmarks 🏖️ No improved or regressed compilation metrics 🏖️ For more information: |
Abbreviated Android Benchmark Summary@ commit 177a3da54b9cfb17a1c0bdb489b443aa2204e28c (vs. base 5363ea3f0ae3af7f016ee4da4ee48f49f869dd99) Regressed Latencies 🚩
Improved Latencies 🎉
For more information: |
4e56e94
to
95c1b17
Compare
e4f506a
to
a024b3a
Compare
Reorg the commits and gave each commit a clear commit message, which should help the code review. |
306e283
to
069af70
Compare
Kindly ping |
Sorry for the delay here, I've been swamped and summiting all last week. I'm looking now, but just from reading the description, I think I'd prefer that fail be the default and auto-detect the special flag. That way it's always clear that autodetection is happening and we don't get silent failures/skips |
if cpu_target_arch is None: | ||
print("WARNING: Detected unsupported CPU architecture in " | ||
f'"{self.device_info}", CPU benchmarking is disabled.') | ||
cpu_target_arch = "unknown" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather than letting magic string filtering take care of this here, should we have an explicit case for running no cpu benchmarks?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I realized that we probably just want to provide a list of the included architectures to the filter_benchmarks_for_category
below. Refactored to avoid the magic string and simplify the interface.
069af70
to
9a50800
Compare
a76d51e
to
2b1920b
Compare
This field indicates the version of the benchmark suite and help the following control flow make decision.
Later each DeviceArchitecture has a name and implements __str__. Using that instead of crafting one here.
aced3be
to
eb885ed
Compare
def test_load_from_run_configs(self): | ||
model_tflite = common_definitions.Model( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is failing on macOS: https://github.com/openxla/iree/actions/runs/4868571511/jobs/8682175725#step:7:2529
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm looking into it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be fixed with #13390
IREE benchmark tool automatically detects the device info of benchmark device and filter the benchmarks. This change maps the detected info to device architecture enum of new benchmark suite.
IREE benchmark tool automatically detects the device info of benchmark device and filter the benchmarks. This change maps the detected info to device architecture enum of new benchmark suite.
For unknown architecture, instead of failing directly, print a warning and skip the benchmarks. This allows users to use the tools with partially unknown hardware (e.g. If users want to run GPU benchmarks but don't care about the CPU benchmarks, we shouldn't fail simply because their CPU is not in the supported list). This is actually the current intended behavior for x86_64 (if
uarch
is unknown, we don't fail but no CPU benchmarks will be run) but without any warning.In the case that we should fail if hardware is mismatched to the benchmarks (e.g. on CI), the force mode option will be added in a follow-up change (#13198).
As a side effect, some tests are updated to test with new benchmark suite as we change some code to use new path by default.
This enables the detection of mobile phones' CPU/GPU for Android benchmark tool with new benchmark suite #13176
First 3 commits are the major changes. Each commit message describes their goals.