Skip to content

Conversation

@guangy10
Copy link
Contributor

@guangy10 guangy10 commented Jan 23, 2025

Some benchmark jobs have been failing for a long time, creating misleading signal about what expected and unexpected by looking at those jobs. For example:

The reason is that those configs are either not supported or having bugs to be fixed. There is no way to specify which config for which model should be disabled/bypassed in the past.

This PR adds the ability to do so. Going forward, the nightly benchmark jobs (android-perf and apple-perf) should always be green, if not, it clearly signals that some configs are failing/flaky and should be fixed.

This PR also set a process to track disabled model+config in the GitHub Issue, the test will catch it if trying to disable a config for a model but not providing a link to GitHub Issue. The enforcement is to make sure regressions is tracked properly.

Test:

python .ci/scripts/tests/test_gather_benchmark_configs.py
----------------------------------------------------------------------
Ran 14 tests in 0.139s

OK

@pytorch-bot
Copy link

pytorch-bot bot commented Jan 23, 2025

🔗 Helpful Links

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

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

⏳ 16 Pending, 2 Unrelated Failures

As of commit 1a4cf63 with merge base d68ca28 (image):

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

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 23, 2025
@guangy10 guangy10 requested a review from huydhn January 23, 2025 01:30
Copy link
Contributor

@huydhn huydhn left a comment

Choose a reason for hiding this comment

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

LGTM!

@guangy10 guangy10 temporarily deployed to upload-benchmark-results January 23, 2025 02:03 — with GitHub Actions Inactive
@guangy10 guangy10 temporarily deployed to upload-benchmark-results January 23, 2025 04:59 — with GitHub Actions Inactive
@guangy10 guangy10 force-pushed the benchmark_skip_disabled_configs branch from c4a2c5d to e0efaba Compare January 23, 2025 21:32
@guangy10
Copy link
Contributor Author

Subscribed @kimishpatel @digantdesai to DevX related work.

@facebook-github-bot
Copy link
Contributor

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

@guangy10 guangy10 force-pushed the benchmark_skip_disabled_configs branch from e0efaba to 1a4cf63 Compare January 23, 2025 22:35
@facebook-github-bot
Copy link
Contributor

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

@guangy10 guangy10 merged commit 453e9e2 into main Jan 24, 2025
72 of 76 checks passed
@guangy10 guangy10 deleted the benchmark_skip_disabled_configs branch January 24, 2025 00:01
YIWENX14 pushed a commit that referenced this pull request Jan 28, 2025
Skip disabled configs for benchmarking

Co-authored-by: Github Executorch <github_executorch@arm.com>
zonglinpeng pushed a commit to zonglinpeng/executorch that referenced this pull request Jan 30, 2025
Skip disabled configs for benchmarking

Co-authored-by: Github Executorch <github_executorch@arm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. topic: not user facing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants