Extend the upload script for other local use cases #36
Conversation
Signed-off-by: Huy Do <huydhn@gmail.com>
Signed-off-by: Huy Do <huydhn@gmail.com>
Signed-off-by: Huy Do <huydhn@gmail.com>
Courtesy of Claude code. This is the initial version of an API that I'm building to allow people to upload benchmark results. This works in conjunction with pytorch/pytorch-integration-testing#36 I will need to prepare a Terraform change for the lambda, but the API works as follows: ``` import requests with open("FILE_TO_BE_UPLOADED.json") as f: content = f.read() json_data = { "username": "REDACT", "password": "REDACT", "content": content, "path": "v3/foobar/debug.json", } headers = { "content-type": "application/json" } url = "https://qrr6jzjpvyyd77fkj6mqkes4mq0tpirr.lambda-url.us-east-1.on.aws" requests.post(url, json=json_data, headers=headers) ``` cc @zhe-thoughts @ZainRizvi --------- Signed-off-by: Huy Do <huydhn@gmail.com>
| if s3_bucket: | ||
| upload_s3(s3_bucket, s3_path, data) | ||
| elif upload_url: | ||
| upload_via_api(upload_url, s3_path, data) |
There was a problem hiding this comment.
Seems like you're expecting the user to always specify a unique s3 bucket to upload the metrics to.
- Is there a right/wrong format or path for them to be using, or will arbitrary paths work just fine as long as they're unique? Do we need path validation here?
- Can we generate this bucket path for the user? Ideally in the lambda so that there's no risk of a bad path being passed in
There was a problem hiding this comment.
While the S3 path is generated automatically here f"v3/{repo_name}/{head_branch}/{head_sha}/{device}/benchmark_results{model_suffix}.json", the lambda has a check to not overwrite existing file https://github.com/pytorch/test-infra/blob/main/aws/lambda/benchmark-results-uploader/lambda_function.py#L29. I will remove the option to set the bucket name because I want to write only to the benchmark bucket.
There was a problem hiding this comment.
How do you feel about sending the lambda the inputs used to generate the S3 path instead? Feels like a much more reliable approach over trusting input from a client to be in a specific format (especially if the backend takes a dependency on that format)
| uploader.add_argument( | ||
| "--upload-url", | ||
| type=str, | ||
| action=ValidateURL, | ||
| help="the URL to upload the benchmark results to", |
There was a problem hiding this comment.
is this expected to be the full s3 url, or just the s3 item key (the lambda seems to have the bucket name fixed)
There was a problem hiding this comment.
Let me rework this part. The URL here will be fixed after https://github.com/pytorch-labs/pytorch-gha-infra/pull/705 is deployed. For example, https://qrr6jzjpvyyd77fkj6mqkes4mq0tpirr.lambda-url.us-east-1.on.aws. I only want people to upload to the benchmark bucket
Signed-off-by: Huy Do <huydhn@gmail.com>
|
The failure on llama4 Maverick is expected. That model isn't working yet. |
|
Looking forward! |
Signed-off-by: Huy Do <huydhn@gmail.com>
|
cc @yangw-dev Once this is merged, we will need to finally migrate https://hud.pytorch.org/benchmark/compilers dashboard to the new benchmark table because this script will only upload the results there. Till date, the two tables are in sync, but this is not the case anymore. |
@zhe-thoughts This is finally landed, I'm writing the wiki on how to use this script locally and will send it your way in the next few hours so that we can try it out tomorrow. |
4 participants
This is the script I'm using to upload vLLM benchmark results. It can be retrofit to upload other benchmark results too, for example
torch.compilebenchmark results on GB200. I will write a wiki on how to use this script, but the simple usage is:This also handles JSONEachRow format correctly in
read_benchmark_resultsfunction as this is the format used by ClickHouse and DynamoBenchcc @zhe-thoughts @ZainRizvi