Upload Android test spec to ossci-android by huydhn · Pull Request #4676 · pytorch/executorch

huydhn · 2024-08-12T20:54:51Z

A follow-up PR of #4657 to automatically update the Android spec file on S3 if the file examples/demo-apps/android/LlamaDemo/android-llm-device-farm-test-spec.yml on GitHub is updated. This requires a role to upload to ossci-android S3 bucket from https://github.com/pytorch-labs/pytorch-gha-infra/pull/455

Testing

https://github.com/pytorch/executorch/actions/runs/10361242176 nows validate the test spec first by calling android-perf with the small stories110M model before trying to upload it to ossci-android

Another test to make sure that android-perf can still be trigger manually via workflow_dispatch https://github.com/pytorch/executorch/actions/runs/10361255713

pytorch-bot · 2024-08-12T20:54:53Z

🔗 Helpful Links

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

📄 Preview Python docs built from this PR

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

✅ No Failures

As of commit acc6482 with merge base 2654f59 ():
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

huydhn · 2024-08-12T21:31:55Z

~~Hmm, where is the import to fbsource button? It looks like facehub chrome extension is having issues~~ Never mind, it comes back now

facebook-github-bot · 2024-08-12T21:35:27Z

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

huydhn · 2024-08-12T23:09:24Z

@pytorchbot drci

kirklandsign

Thank you!

guangy10 · 2024-08-12T23:15:57Z

+      - name: Only push to S3 when running the workflow manually from main branch
+        # if: ${{ github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main' }}


Should it be uncommented?

What are validated in this job on PR if the spec is only uploaded once post-merge?

Why uploading when running the workflow manually?

Yeah, the testing is done, I will need to uncomment this before landing. The 3 point is just a copy/paste from iOS workflow (where we manually upload iOS built artifacts on release, but it's not needed here)

Aha I see, so we don't need to trigger this on PR. pull_request: is added for testing purpose only.

Is there a way to validate the test-spec on PR, instead of updating the S3 to a broken one and defer the issue discovering until the runtime. It will be nice if we could run some validation in the PR that modifies the test-spec

Ah, we can keep pull_request trigger here as it will use aws s3 cp --dry-run which doesn't copy the file and act as a form of testing on PR

Is there a way to validate the test-spec on PR, instead of updating the S3 to a broken one and defer the issue discovering until the runtime. It will be nice if we could run some validation in the PR that modifies the test-spec

Good point! Let me see if I can trigger the android-perf job with the new spec for testing as it's the only way to know if the spec works.

If we don't run any validation on PR, I don't see the value of keeping the pull_request: other than validating the job itself runnable.

Ideally some static analysis for the test-spec on PR would be good, just to ensure no syntax issue, mandatory things are provided in valid format, etc.

It may not be a good idea to add dependency to the android-perf, for the short-term to validate the test-spec, I'm okay with it, if we can make sure we only work on a tiny model with one backend when triggering the job (defaults for on-demand workflow would be good). FYI even today we coded periodic workflow and on-demand workflow with the same defaults, later the periodic defaults will be expanded.

I think it's ok to call android-perf with the small story model and the default set of parameters. To achieve this, I make the test spec configurable as an input and set its default value to the S3 source of truth at https://ossci-android.s3.amazonaws.com/executorch/android-llm-device-farm-test-spec.yml

facebook-github-bot · 2024-08-12T23:16:11Z

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

facebook-github-bot · 2024-08-13T00:15:06Z

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

guangy10 · 2024-08-13T00:43:45Z

+        description: The test spec to drive the test on AWS devices
+        required: false
+        type: string
+        default: https://ossci-android.s3.amazonaws.com/executorch/android-llm-device-farm-test-spec.yml


If the prefix is common for all, let's only put the file name android-llm-device-farm-test-spec.yml here

Oh, this is not a common prefix here though. For PR testing, it needs to stay as an artifact available only to the PR, i.e. https://gha-artifacts.s3.amazonaws.com/pytorch/executorch/10361242176/artifact/android-llm-device-farm-test-spec.yml. On the other hand, once validation is done and the spec file is committed, it will be uploaded to the source of truth at https://ossci-android.s3.amazonaws.com/executorch/android-llm-device-farm-test-spec.yml

@huydhn, fyi, we start seeing the failures due to empty test spec https://hud.pytorch.org/hud/pytorch/executorch/main/1?per_page=50&name_filter=android-perf%20%2F%20benchmark

Let me take a look

Oh I just spot your fix at #4734 while trying to draft mine #4736 :)

facebook-github-bot · 2024-08-13T00:54:31Z

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

huydhn · 2024-08-13T03:14:42Z

@pytorchbot drci

facebook-github-bot · 2024-08-13T08:36:26Z

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

Upload Android test spec to ossci-android

cb437da

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 Aug 12, 2024

huydhn added 3 commits August 12, 2024 14:01

Add the new workflow

13b144f

Use the role

a0384fa

Grant id-token permission

64bf7e7

huydhn requested review from guangy10, kirklandsign and kit1980 August 12, 2024 21:29

huydhn marked this pull request as ready for review August 12, 2024 21:29

huydhn mentioned this pull request Aug 12, 2024

Not hardcode llama2 model in perf test #4657

Merged

kirklandsign approved these changes Aug 12, 2024

View reviewed changes

guangy10 reviewed Aug 12, 2024

View reviewed changes

Comment thread .github/workflows/upload-test-specs.yml Outdated

huydhn added 8 commits August 12, 2024 16:22

Undo testing code

3b4d24d

Keep pull_request trigger as it's dry-run

b7ef68f

Fix typo

2d4802a

Add validation logic

998cc22

Set the correct permission

0c7be0e

Add the permission

d9f5f9e

Still permission error?

67cccde

Validate first and upload later

b549895

huydhn commented Aug 13, 2024

View reviewed changes

Comment thread .github/workflows/android-perf.yml

huydhn requested a review from guangy10 August 13, 2024 00:43

guangy10 reviewed Aug 13, 2024

View reviewed changes

Comment thread .github/workflows/android-perf.yml

guangy10 reviewed Aug 13, 2024

View reviewed changes

Comment thread .github/workflows/upload-test-specs.yml

Also set devices and delegates parameters

5dfedea

guangy10 approved these changes Aug 13, 2024

View reviewed changes

Merge branch 'main' into upload-test-spec-s3

acc6482

facebook-github-bot merged commit ccaaa46 into main Aug 13, 2024

huydhn mentioned this pull request Aug 15, 2024

Fix android perf periodic default spec #4736

Merged

		- name: Only push to S3 when running the workflow manually from main branch
		# if: ${{ github.event_name == 'workflow_dispatch' && github.ref == 'refs/heads/main' }}

Uh oh!

Conversation

huydhn commented Aug 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing

Uh oh!

pytorch-bot Bot commented Aug 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🔗 Helpful Links

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

✅ No Failures

Uh oh!

huydhn commented Aug 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

facebook-github-bot commented Aug 12, 2024

Uh oh!

huydhn commented Aug 12, 2024

Uh oh!

kirklandsign left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huydhn Aug 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huydhn Aug 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huydhn Aug 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Aug 12, 2024

Uh oh!

Uh oh!

Uh oh!

facebook-github-bot commented Aug 13, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

facebook-github-bot commented Aug 13, 2024

Uh oh!

huydhn commented Aug 13, 2024

Uh oh!

facebook-github-bot commented Aug 13, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

huydhn commented Aug 12, 2024 •

edited

Loading

pytorch-bot Bot commented Aug 12, 2024 •

edited

Loading

huydhn commented Aug 12, 2024 •

edited

Loading

huydhn Aug 12, 2024 •

edited

Loading

huydhn Aug 12, 2024 •

edited

Loading

huydhn Aug 13, 2024 •

edited

Loading